* [PATCH] Fix compilation on OS X. @ 2013-07-20 7:49 Benoit Sigoure 2013-07-20 7:55 ` Ramkumar Ramachandra 2013-07-20 12:06 ` Torsten Bögershausen 0 siblings, 2 replies; 10+ messages in thread From: Benoit Sigoure @ 2013-07-20 7:49 UTC (permalink / raw) To: git; +Cc: Benoit Sigoure 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix compilation on OS X. 2013-07-20 7:49 [PATCH] Fix compilation on OS X Benoit Sigoure @ 2013-07-20 7:55 ` Ramkumar Ramachandra 2013-07-20 7:56 ` tsuna 2013-07-20 12:06 ` Torsten Bögershausen 1 sibling, 1 reply; 10+ messages in thread From: Ramkumar Ramachandra @ 2013-07-20 7:55 UTC (permalink / raw) To: Benoit Sigoure; +Cc: git 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix compilation on OS X. 2013-07-20 7:55 ` Ramkumar Ramachandra @ 2013-07-20 7:56 ` tsuna 0 siblings, 0 replies; 10+ messages in thread From: tsuna @ 2013-07-20 7:56 UTC (permalink / raw) To: Ramkumar Ramachandra; +Cc: git On Sat, Jul 20, 2013 at 12:55 AM, Ramkumar Ramachandra <artagnon@gmail•com> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix compilation on OS X. 2013-07-20 7:49 [PATCH] Fix compilation on OS X Benoit Sigoure 2013-07-20 7:55 ` Ramkumar Ramachandra @ 2013-07-20 12:06 ` Torsten Bögershausen 2013-07-20 18:41 ` Benoit Sigoure 2013-07-21 5:53 ` Junio C Hamano 1 sibling, 2 replies; 10+ messages in thread From: Torsten Bögershausen @ 2013-07-20 12:06 UTC (permalink / raw) To: Benoit Sigoure; +Cc: git 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Fix compilation on OS X. 2013-07-20 12:06 ` Torsten Bögershausen @ 2013-07-20 18:41 ` Benoit Sigoure 2013-07-21 5:53 ` Junio C Hamano 1 sibling, 0 replies; 10+ messages in thread From: Benoit Sigoure @ 2013-07-20 18:41 UTC (permalink / raw) To: git; +Cc: Benoit Sigoure 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix compilation on OS X. 2013-07-20 12:06 ` Torsten Bögershausen 2013-07-20 18:41 ` Benoit Sigoure @ 2013-07-21 5:53 ` Junio C Hamano 2013-07-21 6:10 ` tsuna 2013-07-21 6:17 ` [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Benoit Sigoure 1 sibling, 2 replies; 10+ messages in thread From: Junio C Hamano @ 2013-07-21 5:53 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: Benoit Sigoure, git Torsten Bögershausen <tboegi@web•de> 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix compilation on OS X. 2013-07-21 5:53 ` Junio C Hamano @ 2013-07-21 6:10 ` tsuna 2013-07-21 6:17 ` [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Benoit Sigoure 1 sibling, 0 replies; 10+ messages in thread From: tsuna @ 2013-07-21 6:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: Torsten Bögershausen, git On Sat, Jul 20, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox•com> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" 2013-07-21 5:53 ` Junio C Hamano 2013-07-21 6:10 ` tsuna @ 2013-07-21 6:17 ` Benoit Sigoure 2013-07-21 19:54 ` Benoit Sigoure 1 sibling, 1 reply; 10+ messages in thread From: Benoit Sigoure @ 2013-07-21 6:17 UTC (permalink / raw) To: git; +Cc: tboegi, gitster, Benoit Sigoure 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" 2013-07-21 6:17 ` [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Benoit Sigoure @ 2013-07-21 19:54 ` Benoit Sigoure 2013-07-21 22:09 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Benoit Sigoure @ 2013-07-21 19:54 UTC (permalink / raw) To: git; +Cc: tboegi, gitster, Benoit Sigoure 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 <tsunanet@gmail•com> --- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" 2013-07-21 19:54 ` Benoit Sigoure @ 2013-07-21 22:09 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2013-07-21 22:09 UTC (permalink / raw) To: Benoit Sigoure; +Cc: git, tboegi, Ramsay Jones Benoit Sigoure <tsunanet@gmail•com> 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 <tsunanet@gmail•com> 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; ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-07-21 22:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-20 7:49 [PATCH] Fix compilation on OS X Benoit Sigoure 2013-07-20 7:55 ` Ramkumar Ramachandra 2013-07-20 7:56 ` tsuna 2013-07-20 12:06 ` Torsten Bögershausen 2013-07-20 18:41 ` Benoit Sigoure 2013-07-21 5:53 ` Junio C Hamano 2013-07-21 6:10 ` tsuna 2013-07-21 6:17 ` [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning" Benoit Sigoure 2013-07-21 19:54 ` Benoit Sigoure 2013-07-21 22:09 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox