[PATCH] Fix compilation on OS X.

[PATCH] Fix compilation on OS X.

[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

Re: [PATCH] Fix compilation on OS X.

[Ramkumar Ramachandra]

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?

Re: [PATCH] Fix compilation on OS X.

[tsuna]

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

Re: [PATCH] Fix compilation on OS X.

[Torsten Bögershausen]

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

[PATCH] Fix compilation on OS X.

[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

Re: [PATCH] Fix compilation on OS X.

[Junio C Hamano]

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?

Re: [PATCH] Fix compilation on OS X.

[tsuna]

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

[PATCH] Revert "compat/unsetenv.c: Fix a sparse warning"

[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

[PATCH] Revert "compat/unsetenv.c: Fix a sparse warning"

[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

Re: [PATCH] Revert "compat/unsetenv.c: Fix a sparse warning"

[Junio C Hamano]

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;