What compiler is this? Wondering why we don't see this on any other platform....

I think this #ifndef is supposed to be for macOS? If so, I don't think this is the right thing to do. I found the following code in another project (czmq):

#if defined (__UNIX__)
#   if defined (__UTYPE_OSX)
#include <crt_externs.h>
#define environ (*_NSGetEnviron ()) 
#   else
extern char **environ;              // should be declared as a part of unistd.h, but fail in some targets in Travis
                                    // declare it explicitly
#   endif
#endif

It's a matter of libc.

On alpine, it is musl instead gblic and in unistd.h we have:

#ifdef _GNU_SOURCE     
extern char **environ;
...
...
#endif

musl seems to rely on _GNU_SOURCE.

On glibc environment, __USE_GNU is implied by _GNU_SOURCE, see this in /usr/include/features.h in glibc environment:

#ifdef  _GNU_SOURCE
# define __USE_GNU      1
#endif

On my glibc machine, the unistd.h contains:

/* NULL-terminated array of "NAME=VALUE" environment variables.  */
extern char **__environ;
#ifdef __USE_GNU
extern char **environ;
#endif 

You can reproduce the failing compilation like this:

docker run -it alpine /bin/sh

Then inside running container:

cd /home
wget https://github.com/gearman/gearmand/releases/download/1.1.19.1/gearmand-1.1.19.1.tar.gz
tar xvzf gearmand-1.1.19.1.tar.gz
cd gearmand-1.1.19.1
apk add musl-dev gcc g++ autoconf automake m4 git make util-linux-dev libuuid libevent-dev gperf boost-dev
./configure
make

I pushed a new fix that compiles both on alpine with musl and ubuntu with glibc.

It uses _GNU_SOURCE for the ifdef because __USE_GNU seems to be "internal"

Thibault

I pushed a new fix that compiles both on alpine with musl and ubuntu with glibc.

It uses _GNU_SOURCE for the ifdef because __USE_GNU seems to be "internal"

I think _GNU_SOURCE is meant to be #defined by the developer in order to enable certain features in glibc. So that's not what we want here either, I think, but maybe I'm wrong.

We want a #define that detects whether or not environ is declared, and I think _USE_GNU is the correct one for that, as shown in the snippet from your unistd.h.

So apparently the problem is that musl doesn't #define __USE_GNU unless _GNU_SOURCE is #defined, which it doesn't do by default, right? If so, I think what we need is something like

#if __MUSL
# define _GNU_SOURCE
#endif

at the top of this file. The problem is that musl doesn't #define a __MUSL or the like, according to this:

https://stackoverflow.com/questions/7296963/gnu-source-and-use-gnu
EDIT: Pasted the wrong URL. I meant this one:
https://stackoverflow.com/questions/58177815/how-to-actually-detect-musl-libc

That StackOverflow question has an interesting comment: "... consider checking for GNU_SOURCE but NOT _GLIBC (or _GLIB_MINOR, or __GNU_LIBRARY)...." That doesn't seem correct, but it might be close. Maybe #if defined(_GLIBC) && !defined(_GNU_SOURCE)?

EDIT: No, that's obviously wrong.... Ugh. Maybe this?

#if !defined(_GLIBC) && !defined(__USE_GNU) /* detect musl */
# define _GNU_SOURCE
#endif

Maybe need to add some conditions there for BSD and Mac?

EDIT: Ugh, I think I'm just completely confused and need something to eat... Sorry.

I don't know 🤭

I had the feeling that _GNU_SOURCE was more "standard" between various libc:

glibc = both __USE_GNU and _GNU_SOURCE (in features.h if _GNU_SOURCE implies __USE_GNU)
uClibc = __USE_GNU and _GNU_SOURCE (same features.h mechanism where __USE_GNU is implied by _GNU_SOURCE)
dietlibc = almost only _GNU_SOURCE (only 1 occurrence of __USE_GNU)
musl = only _GNU_SOURCE

But honestly I repeat, I don't know 😁

I leave you the final word to decide 👍

Well, I think it should be clear by now that I don't know either. :)

As near as I can tell, nothing in the gearmand build apparatus #defines either _GNU_SOURCE or __USE_GNU, so I'm kind of befuddled as to how this code compiles at all.

Everything I've read online says that _GNU_SOURCE is supposed to be #defined by developers in order to enable GNU extensions in glibc. Source: https://lwn.net/Articles/590381/

As near as I can tell, nothing in the gearmand build apparatus #defines either _GNU_SOURCE or __USE_GNU, so I'm kind of befuddled as to how this code compiles at all.

I think this is wrong.

In gearman, _GNU_SOURCE (and GNU extensions in general) seems to be set by configure "on systems that have them."

It appears in m4/extensions.m4 (but this "undef is not defined" is weird) then later in the gear_config.in (same weird logic) in the release tarball then later in the gear_config.h after the ./configure (but this time it sets well the _GNU_SOURCE).

So I think it is set "when it's possible" and I just checked with musl and it is also well set.

Then this fix seems to be valid (see edit below), but you can should also target more specifically MacOS (as we believe it is the original goal of this ifdef).

EDIT: it appears that on MacOS, the configure also sets _GNU_SOURCE, I tested in a github action see the logs.
I don't know what is doing the libc on MacOS, maybe it does not set __USE_GNU from _GNU_SOURCE and that's why the current code (before my fix) was valid.

EDIT: on my side I used -fpermissive and it seems to fix some CPANTesters reports running on alpine

Thibault

I edited several times the previous message, sorry for that, as we agreed both, this problem and fix not cristal clear for us 😆

I tested some variation on mac os github actions:

✔️ Vanilla 1.1.19.1:
OK: https://github.com/thibaultduponchelle/messy-perl-ci-workflows/runs/902922433?check_suite_focus=true

✔️ No ifndef (but keep static char **environ= NULL;) :
OK: https://github.com/thibaultduponchelle/messy-perl-ci-workflows/runs/902923039?check_suite_focus=true

❌ ifndef _GNU_SOURCE:
KO: https://github.com/thibaultduponchelle/messy-perl-ci-workflows/runs/902795129?check_suite_focus=true
libtest/cmdline.cc:264:78: error: use of undeclared identifier 'environ'

❌ No ifndef and no environ declaration:
KO: https://github.com/thibaultduponchelle/messy-perl-ci-workflows/runs/902865330?check_suite_focus=true
libtest/cmdline.cc:260:78: error: use of undeclared identifier 'environ'

Means:

• Mac OS needs this line (static char **environ= NULL;) to build
• Mac OS has _GNU_SOURCE defined but __USE_GNU not defined (I imagine the Mac OS libc does not set __USE_GNU)

Then #ifndef __USE_GNU will impact both musl and macOS which is bad ❌
And #ifndef _USE_SOURCE will protect almost everybody which is bad ❌

My fix is wrong for mac os and current ifndef is wrong for musl.

The tarball used :

• The one from the release 1.1.19.1 from your github here
• ifndef __GNU_SOURCE
• No ifndef
• No ifndef no declaration

The github actions:

• Vanilla gearman 1.1.19.1
• ifndef _GNU_SOURCE
• No ifndef
• No ifndef no declaration

I hope this will help the debates 😄

Maybe I can try also extern char **environ= NULL; instead of static char **environ= NULL; not tested yet.

Back again, replacing static char **environ= NULL; per extern char **environ= NULL; compiles on Mac OS, Alpine and Ubuntu.

See Mac OS build here from this github action using this modified gearman 1.1.19.1

I updated the PR, I remind you the original error was to redefine with static instead extern.

I'm not sure what other kind of impact it could have.

If you have better idea or simply prefer another solution, I let you decide, no problem 😄

Thank you for extensive testing! You rock, @thibaultduponchelle. :)

Could you test something like this on macOS?

#if defined(__APPLE__) && __APPLE__
# include <crt_externs.h>
# define environ (*_NSGetEnviron ()) 
#elif !defined(_GNU_SOURCE)
extern char **environ= NULL;
#endif

There’s also FreeBSD. I don’t think we officially support FreeBSD, but I know we’ve made some efforts in that arena to get it to compile at least. Not expecting you to test this on FreeBSD, but it might be worth researching how FreeBSD defines environ in unistd.h, assuming it does.

EDIT: Hmmm, I don't see any mention of environ in FreeBSD's unistd.h, so I think the above should work. Anyone? I presume _GNU_SOURCE isn't defined on FreeBSD, right?

As near as I can tell, nothing in the gearmand build apparatus #defines either _GNU_SOURCE or __USE_GNU, so I'm kind of befuddled as to how this code compiles at all.

I think this is wrong.

Oh, I knew it was wrong, but I just couldn’t find where it happened. Thanks for giving me some clarity. :)

So I think it is set "when it's possible" and I just checked with musl and it is also well set.

Ok, that’s good to know!

✔️ Your code snippet compiles OK on Mac OS, Ubuntu and Alpine 🕺

See github ci build using this tarball

Concerning freebsd, I don't know, but I think you could add a freebsd ci with cirrus-ci. And IMHO you can as well add a mac os in your travis for non-reg purposes.

Seems like we have a fix, right ? 😃

Wait @p-alik thank you for approval but this PR still not contains the final fix, will add it right now

Thank you, @thibaultduponchelle, for seeing it through and doing so much testing. Cheers!

Can somebody check and make sure the libgearman produced by this is ABI compatible with 1.1.19.1? I worry that we may need to bump soname if we have changed this variable's declaration.

Hi, Clint. Is libtest code in libgearman.so? I assumed libtest was only used in the test suite.

This variable declaration has changed on macOS and other non-glibc platforms, like FreeBSD and alpine.