After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 788476 - Cannot compiler vte 0.50.x on macOS
Cannot compiler vte 0.50.x on macOS
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.50.x
Other Mac OS
: Normal major
: ---
Assigned To: VTE Maintainers
VTE Maintainers
: 789022 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-10-03 15:11 UTC by Tom Schoonjans
Modified: 2017-10-15 22:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
C approach (2.76 KB, patch)
2017-10-03 19:53 UTC, Egmont Koblinger
none Details | Review
C++ approach (2.99 KB, patch)
2017-10-03 19:53 UTC, Egmont Koblinger
none Details | Review
Fix v2 (2.58 KB, patch)
2017-10-06 13:12 UTC, Egmont Koblinger
none Details | Review
Fix v3 (2.23 KB, patch)
2017-10-06 19:40 UTC, Egmont Koblinger
accepted-commit_now Details | Review

Description Tom Schoonjans 2017-10-03 15:11:57 UTC
The error I observed was:

  CXX      libvte_2_91_la-vtestream.lo
  CXX      libvte_2_91_la-vtetree.lo
  CXX      libvte_2_91_la-vtetypes.lo
vteseq.cc:2537:18: error: use of undeclared identifier 'strchrnul'; did you mean 'strchr'?
                *strchrnul(id, ':') = '\0';
                 ^~~~~~~~~
                 strchr
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/string.h:78:13: note: 'strchr' declared here
      char* strchr(      char* __s, int __c) {return __libcpp_strchr(__s, __c);}
            ^
1 error generated.
make[2]: *** [libvte_2_91_la-vteseq.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [install] Error 2
make: *** [install-recursive] Error 1

It turns out that strchrnul is a GNU extension, unavailable on macOS...
Comment 1 Egmont Koblinger 2017-10-03 15:15:37 UTC
Thanks for the report!

I've introduced it so I'll take care of fixing. There's a my_strchrnul() in vtespawn.cc, I guess I'll move that to vteutils.

There's also a strchrnul in gnome-terminal, needs fixing too.
Comment 2 Tom Schoonjans 2017-10-03 15:19:58 UTC
Thanks for the quick reply!

I appreciate you looking into this, but don't bother with fixing this in gnome-terminal as that one cannot be built on macOS due to its Linux only dependencies.
Comment 3 Egmont Koblinger 2017-10-03 15:28:40 UTC
Hmmm, really? @chpe do you know anything more about it?

Is it one of the prerequisites of gnome-terminal that cannot be compiled, or gnome-terminal itself? In the latter case I'd be happy to look at it.

I don't have access to a mac to test, though.
Comment 4 Christian Persch 2017-10-03 17:31:05 UTC
I don't see any linux-specific dependencies in gnome-terminal...
Comment 5 Egmont Koblinger 2017-10-03 19:18:08 UTC
Ok.

Tom, if can please help me here, we could work together to track down and fix all the compilation issues of g-t.
Comment 6 Egmont Koblinger 2017-10-03 19:53:29 UTC
Created attachment 360867 [details] [review]
C approach
Comment 7 Egmont Koblinger 2017-10-03 19:53:45 UTC
Created attachment 360868 [details] [review]
C++ approach
Comment 8 Egmont Koblinger 2017-10-03 19:57:01 UTC
Tom, could you please test which of the two attached versions work on mac?

The C approach defines one function that takes "const char*" and returns "char*" and in order to do this it needs to have an ugly cast.

The C++ approach defines two functions (as it happens in /usr/include/string.h if both __USE_GNU and __CORRECT_ISO_CPP_STRING_H_PROTO are set, which seems to be the case for me, but pretty sure it's totally different on mac). One has no const at all, the other one takes and also returns a const. Hence no need for a cast. [vte uses this method at 2 places. As lucky as we are, of course one of them needs the const version and the other needs the non-const one.]

@chpe If both patches work, which one to go for? :)

I definitely wanted to fall back to the libc implementation if available, since that one might be better optimized (or maybe even gcc has it built-in). Not that it matters, but still.
Comment 9 Egmont Koblinger 2017-10-03 19:59:35 UTC
> but pretty sure it's totally different on mac

Silly me, of course it's completely missing on mac, that's what this bugreport is about.
Comment 10 Tom Schoonjans 2017-10-04 07:21:49 UTC
Thanks for coming up with patches so fast. I tested the C approach, which appears to work fine.

I apologise for my ignorant comment regarding the Linux only dependencies: I only had a quick look at the configure.ac file and saw the reference to nautilus, not realising that was an optional dependency.

Either way, there is no real need to investigate supporting gnome-terminal on macOS. I maintain the Gtk+/GNOME related formulas on Homebrew and have never been asked by anyone to come up with a formula for gnome-terminal.

Thanks again!
Comment 11 Egmont Koblinger 2017-10-04 15:11:47 UTC
Let's go with the C version then. That's what we had so far anyways.

Christian, please take a quick look. I never feel comfortable touching autoconf stuff :)

Tom, consider me asking for g-t, haha :) No, but seriously, I'd appreciate if you could help us locate the exact problems and fix our code.
Comment 12 Tom Schoonjans 2017-10-04 15:18:29 UTC
Ok, will give it a try at some point. I have never actually tried compiling gnome-terminal on macOS simply because macOS already ships with a really nice terminal app :-)

I do remember seeing some trouble with vte when using the gtk3 quartz backend, but that was long time ago and nobody has complained about in on Homebrew...
Comment 13 Christian Persch 2017-10-05 20:08:10 UTC
The "C version" is the one to go for.  (The replacement already exists in vtespawn.cc because that's copypasted code from glib.) I'd like however to keep the strchrnul name, so just provide the replacement function from vteutil on condition of #ifndef HAVE_STRCHRNUL, instead of writing a wrapper function.
Comment 14 Egmont Koblinger 2017-10-05 20:18:14 UTC
That has the disadvantage that we don't catch if we add a strchrnul() somewhere without #including <vteutils.h> first.

Mind you, even with my current patch, we won't catch if we add a non-prefixed strchrnul() somewhere, and that's about the same.

So okay, I'll do it :)
Comment 15 Egmont Koblinger 2017-10-06 13:12:01 UTC
Created attachment 361044 [details] [review]
Fix v2

Please take another look, thanks :)
Comment 16 Christian Persch 2017-10-06 17:16:48 UTC
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE /* for strchrnul */
+#endif

This is unnecessary, since we use AC_USE_SYSTEM_EXTENSIONS in configure, so _GNU_SOURCE will always be set on glibc-based systems.

With that removed, ok to commit (0-50 too if you want to). Thanks!
Comment 17 Egmont Koblinger 2017-10-06 19:40:13 UTC
Created attachment 361062 [details] [review]
Fix v3

Tom, to be on the safe side, could you please confirm that this patch (instead of the first one) still works?

(In reply to Christian Persch from comment #16)

> This is unnecessary, [...]

I copied it from vteutils.cc (O_TMPFILE). I guess we could remove it from there too, right? :)
Comment 18 Christian Persch 2017-10-10 08:59:31 UTC
Comment on attachment 361062 [details] [review]
Fix v3

Yes, you can remove any manual defines of _GNU_SOURCE.

+/* Copeid from glib */

Typo :-)
Comment 19 Christian Persch 2017-10-15 18:38:53 UTC
*** Bug 789022 has been marked as a duplicate of this bug. ***
Comment 20 Egmont Koblinger 2017-10-15 20:44:19 UTC
Submitted (master & 0-50).

Tom or Jiri, could you please help fix gnome-terminal as well? It must also fail on strchrnul, but I don't feel like fixing that as long as I don't know whether that's the only problem or there's more to fix.
Comment 21 Jiri Techet 2017-10-15 22:29:14 UTC
I have just tried but I don't have the dependencies installed and I'm using gtk-osx which uses jhbuild and installing dependencies is rather painful. I'm afraid I don't have time to get all the dependencies built. I'm maintaining a macOS port of Geany for which I needed just VTE.
Comment 22 Egmont Koblinger 2017-10-15 22:33:51 UTC
OK, nevermind, closing then.

If anyone can help us out with gnome-terminal on Mac, feel free to reopen this one or file a new bug.