# [PATCH] Fix compilation on OS X. 10 messages from 2013-07-20 to 2013-07-21. Participants: Benoit Sigoure, Ramkumar Ramachandra, tsuna, Torsten Bögershausen, Junio C Hamano. Thread: https://gitlist.dev/t/34499 ## Benoit Sigoure, 2013-07-20 07:49 Subject: [PATCH] Fix compilation on OS X. Message-ID: <1374306567-16640-1-git-send-email-tsunanet@gmail.com> URL: https://gitlist.dev/e/1374306567-16640-1-git-send-email-tsunanet%40gmail.com ``` On OS X libc headers don't define `environ', and since ec535cc2 removed the redundant declaration this code no longer builds on OS X. --- compat/unsetenv.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compat/unsetenv.c b/compat/unsetenv.c index 4ea1856..addf3dc 100644 --- a/compat/unsetenv.c +++ b/compat/unsetenv.c @@ -1,5 +1,10 @@ #include "../git-compat-util.h" +#ifdef __APPLE__ +// On OS X libc headers don't define this symbol. +extern char **environ; +#endif + void gitunsetenv (const char *name) { int src, dst; -- 1.8.2.1.539.g4196a96 ``` ## Ramkumar Ramachandra, 2013-07-20 07:55 Subject: Re: [PATCH] Fix compilation on OS X. Message-ID: URL: https://gitlist.dev/e/CALkWK0m46SR3C6R_pjYouu7LkHjL_gPGb1A3S94d0nx8227-UA%40mail.gmail.com In-Reply-To: <1374306567-16640-1-git-send-email-tsunanet@gmail.com> ``` Benoit Sigoure wrote: > diff --git a/compat/unsetenv.c b/compat/unsetenv.c > index 4ea1856..addf3dc 100644 > --- a/compat/unsetenv.c > +++ b/compat/unsetenv.c > @@ -1,5 +1,10 @@ > #include "../git-compat-util.h" > > +#ifdef __APPLE__ > +// On OS X libc headers don't define this symbol. > +extern char **environ; > +#endif > + Shouldn't this go into git-compat-util.h, since there may be other files depending on this variable? ``` ## tsuna, 2013-07-20 07:56 Subject: Re: [PATCH] Fix compilation on OS X. Message-ID: URL: https://gitlist.dev/e/CAFKYj4fdYHKukM7kaxxTKYNqTfOXadmZynTxXO9824G6H6HiZA%40mail.gmail.com In-Reply-To: ``` On Sat, Jul 20, 2013 at 12:55 AM, Ramkumar Ramachandra wrote: > Benoit Sigoure wrote: >> diff --git a/compat/unsetenv.c b/compat/unsetenv.c >> index 4ea1856..addf3dc 100644 >> --- a/compat/unsetenv.c >> +++ b/compat/unsetenv.c >> @@ -1,5 +1,10 @@ >> #include "../git-compat-util.h" >> >> +#ifdef __APPLE__ >> +// On OS X libc headers don't define this symbol. >> +extern char **environ; >> +#endif >> + > > Shouldn't this go into git-compat-util.h, since there may be other > files depending on this variable? I thought about that but there are no other files that use `environ' so I opted for putting it here instead. -- Benoit "tsuna" Sigoure ``` ## Torsten Bögershausen, 2013-07-20 12:06 Subject: Re: [PATCH] Fix compilation on OS X. Message-ID: <51EA7D58.3050800@web.de> URL: https://gitlist.dev/e/51EA7D58.3050800%40web.de In-Reply-To: <1374306567-16640-1-git-send-email-tsunanet@gmail.com> ``` On 2013-07-20 09.49, Benoit Sigoure wrote: > +#ifdef __APPLE__ > +// On OS X libc headers don't define this symbol. > +extern char **environ; > +#endif > + A more generic approach could be: In the file "config.mak.uname": Define a variable in the Darwin section like this NO_EXT_ENVIRON = UnfortunatelyYes In "Makefile", pick it up, and convert it into a compiler option: ifdef NO_EXT_ENVIRON BASIC_CFLAGS += -DNO_EXT_ENVIRON endif And in "git-compat-util.h", add these lines "at a good place": #ifdef NO_EXT_ENVIRON extern char **environ; #endif This will allow other OS to use the NO_EXT_ENVIRON when needed,. Thanks for working on this. /Torsten ``` ## Benoit Sigoure, 2013-07-20 18:41 Subject: [PATCH] Fix compilation on OS X. Message-ID: <1374345691-21834-1-git-send-email-tsunanet@gmail.com> URL: https://gitlist.dev/e/1374345691-21834-1-git-send-email-tsunanet%40gmail.com In-Reply-To: <51EA7D58.3050800@web.de> ``` On OS X libc headers don't define `environ', and since ec535cc2 removed the redundant declaration this code no longer builds on OS X. --- Makefile | 5 +++++ config.mak.uname | 1 + git-compat-util.h | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 0600eb4..774db18 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,8 @@ all:: # # Define NO_UNSETENV if you don't have unsetenv in the C library. # +# Define NO_EXT_ENVIRON if your C library doesn't define `environ'. +# # Define NO_MKDTEMP if you don't have mkdtemp in the C library. # # Define MKDIR_WO_TRAILING_SLASH if your mkdir() can't deal with trailing slash. @@ -1307,6 +1309,9 @@ ifdef NO_UNSETENV COMPAT_CFLAGS += -DNO_UNSETENV COMPAT_OBJS += compat/unsetenv.o endif +ifdef NO_EXT_ENVIRON + COMPAT_CFLAGS += -DNO_EXT_ENVIRON +endif ifdef NO_SYS_SELECT_H BASIC_CFLAGS += -DNO_SYS_SELECT_H endif diff --git a/config.mak.uname b/config.mak.uname index 7ac541e..ebcfbfd 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -93,6 +93,7 @@ ifeq ($(uname_S),Darwin) NO_STRLCPY = YesPlease endif NO_MEMMEM = YesPlease + NO_EXT_ENVIRON = UnfortunatelyYes USE_ST_TIMESPEC = YesPlease HAVE_DEV_TTY = YesPlease NEEDS_CLIPPED_WRITE = YesPlease diff --git a/git-compat-util.h b/git-compat-util.h index ff193f4..3bac4e9 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -408,6 +408,10 @@ extern ssize_t git_pread(int fd, void *buf, size_t count, off_t offset); */ extern ssize_t read_in_full(int fd, void *buf, size_t count); +#ifdef NO_EXT_ENVIRON +extern char **environ; +#endif + #ifdef NO_SETENV #define setenv gitsetenv extern int gitsetenv(const char *, const char *, int); -- 1.8.2.1.539.g4196a96 ``` ## Junio C Hamano, 2013-07-21 05:53 Subject: Re: [PATCH] Fix compilation on OS X. Message-ID: <7vr4essbkv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr4essbkv.fsf%40alter.siamese.dyndns.org In-Reply-To: <51EA7D58.3050800@web.de> ``` Torsten Bögershausen writes: > On 2013-07-20 09.49, Benoit Sigoure wrote: >> +#ifdef __APPLE__ >> +// On OS X libc headers don't define this symbol. >> +extern char **environ; >> +#endif >> + > A more generic approach could be: > > In the file "config.mak.uname": Define a variable in the Darwin section like this > NO_EXT_ENVIRON = UnfortunatelyYes Actually, it is _wrong_ for us to rely on system header files to define this symbol for us. Declaring "extern char **environ" is responsibility of the user programs (like us). When _GNU_SOURCE is defined glibc header (I think it is unistd.h) seem to define it for us. Perhaps the correct fix is to revert ec535cc2 for everybody, and if MinGW needs such a workaround, do it inside #ifndef MINGW? ``` ## tsuna, 2013-07-21 06:10 Subject: Re: [PATCH] Fix compilation on OS X. Message-ID: URL: https://gitlist.dev/e/CAFKYj4dRFvTwM_x0mXsOLQCTuw9nAvh3b4bZ%2BOmg7M5DVNnVzA%40mail.gmail.com In-Reply-To: <7vr4essbkv.fsf@alter.siamese.dyndns.org> ``` On Sat, Jul 20, 2013 at 10:53 PM, Junio C Hamano wrote: > Actually, it is _wrong_ for us to rely on system header files to > define this symbol for us. Declaring "extern char **environ" is > responsibility of the user programs (like us). Actually, that's right. The C99 standard doesn't mention anything about `environ' (only 7.20.4.5 defines `getenv') and POSIX explicitly states "the [environ] variable, which must be declared by the user if it is to be used directly" (http://pubs.opengroup.org/onlinepubs/9699919799/functions/environ.html) > When _GNU_SOURCE is defined glibc header (I think it is unistd.h) > seem to define it for us. > > Perhaps the correct fix is to revert ec535cc2 for everybody, and if > MinGW needs such a workaround, do it inside #ifndef MINGW? That sounds right. -- Benoit "tsuna" Sigoure ``` ## Benoit Sigoure, 2013-07-21 06:17 Subject: [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Message-ID: <1374387454-27800-1-git-send-email-tsunanet@gmail.com> URL: https://gitlist.dev/e/1374387454-27800-1-git-send-email-tsunanet%40gmail.com In-Reply-To: <7vr4essbkv.fsf@alter.siamese.dyndns.org> ``` This reverts commit ec535cc27e6c4f5e0b1d157e04f5511f166ecd9d. POSIX explicitly states "the [environ] variable, which must be declared by the user if it is to be used directly". Not declaring it causes compilation to fail on OS X. Instead don't declare the variable on MinGW, as it causes a spurious warning there. --- compat/unsetenv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compat/unsetenv.c b/compat/unsetenv.c index 4ea1856..bf5fd70 100644 --- a/compat/unsetenv.c +++ b/compat/unsetenv.c @@ -2,6 +2,9 @@ void gitunsetenv (const char *name) { +#if !defined(__MINGW32__) + extern char **environ; +#endif int src, dst; size_t nmln; -- 1.8.2.1.539.g4196a96 ``` ## Benoit Sigoure, 2013-07-21 19:54 Subject: [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Message-ID: <1374436448-8199-1-git-send-email-tsunanet@gmail.com> URL: https://gitlist.dev/e/1374436448-8199-1-git-send-email-tsunanet%40gmail.com In-Reply-To: <1374387454-27800-1-git-send-email-tsunanet@gmail.com> ``` This reverts commit ec535cc27e6c4f5e0b1d157e04f5511f166ecd9d. POSIX explicitly states "the [environ] variable, which must be declared by the user if it is to be used directly". Not declaring it causes compilation to fail on OS X. Instead don't declare the variable on MinGW, as it causes a spurious warning there. Signed-off-by: Benoit Sigoure --- Resending as I forgot to Sign-off the previous patch. compat/unsetenv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compat/unsetenv.c b/compat/unsetenv.c index 4ea1856..bf5fd70 100644 --- a/compat/unsetenv.c +++ b/compat/unsetenv.c @@ -2,6 +2,9 @@ void gitunsetenv (const char *name) { +#if !defined(__MINGW32__) + extern char **environ; +#endif int src, dst; size_t nmln; -- 1.8.2.1.539.g4196a96 ``` ## Junio C Hamano, 2013-07-21 22:09 Subject: Re: [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Message-ID: <7v38r7sgyf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v38r7sgyf.fsf%40alter.siamese.dyndns.org In-Reply-To: <1374436448-8199-1-git-send-email-tsunanet@gmail.com> ``` Benoit Sigoure writes: > This reverts commit ec535cc27e6c4f5e0b1d157e04f5511f166ecd9d. > > POSIX explicitly states "the [environ] variable, which > must be declared by the user if it is to be used directly". > Not declaring it causes compilation to fail on OS X. > > Instead don't declare the variable on MinGW, as it causes > a spurious warning there. > > Signed-off-by: Benoit Sigoure Thanks, will queue. > --- > > Resending as I forgot to Sign-off the previous patch. > > compat/unsetenv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/compat/unsetenv.c b/compat/unsetenv.c > index 4ea1856..bf5fd70 100644 > --- a/compat/unsetenv.c > +++ b/compat/unsetenv.c > @@ -2,6 +2,9 @@ > > void gitunsetenv (const char *name) > { > +#if !defined(__MINGW32__) > + extern char **environ; > +#endif > int src, dst; > size_t nmln; ```