threads / patch / 37408

patchUndefine strlcpy if needed.

Subject: [PATCH] Undefine strlcpy if needed.

## tl;dr

10 messages between Aug 24, 2014 and Aug 25, 2014. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Benoit Sigoure· Aug 24, 2014, 04:32 UTC · lore
On OS X, strlcpy is already #define'd, which causes warnings
in all the files that include `git-compat-util.h'.  Note that
this only occurs when building without running ./configure.
---
 git-compat-util.h | 3 +++
 1 file changed, 3 insertions(+)
Show changes to git-compat-util.h +3 −0
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..8c001e2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -495,6 +495,9 @@ extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
 #ifdef NO_STRLCPY
+#ifdef strlcpy
+#undef strlcpy
+#endif
 #define strlcpy gitstrlcpy
 extern size_t gitstrlcpy(char *, const char *, size_t);
 #endif

-- 
1.9.2.460.gfb82504
Benoit Sigoure· Aug 24, 2014, 04:35 UTC · re: Benoit Sigoure · lore

On OS X, strlcpy is already #define'd, which causes warnings in all the files that include `git-compat-util.h'. Note that this only occurs when building without running ./configure.

Signed-off-by: Benoit Sigoure <tsunanet@gmail.com>
---
Resending with the SOB line I forgot.
 git-compat-util.h | 3 +++
 1 file changed, 3 insertions(+)
Show changes to git-compat-util.h +3 −0
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..8c001e2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -495,6 +495,9 @@ extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
 #ifdef NO_STRLCPY
+#ifdef strlcpy
+#undef strlcpy
+#endif
 #define strlcpy gitstrlcpy
 extern size_t gitstrlcpy(char *, const char *, size_t);
 #endif

-- 
1.9.2.460.gfb82504
Ramsay Jones· Aug 24, 2014, 11:10 UTC · re: Benoit Sigoure · lore

Re: [PATCH] Undefine strlcpy if needed.

On 24/08/14 05:35, Benoit Sigoure wrote:
Show 23 quoted lines
> On OS X, strlcpy is already #define'd, which causes warnings
> in all the files that include `git-compat-util.h'.  Note that
> this only occurs when building without running ./configure.
> 
> Signed-off-by: Benoit Sigoure <tsunanet@gmail.com>
> ---
> 
> Resending with the SOB line I forgot.
> 
>  git-compat-util.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f587749..8c001e2 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -495,6 +495,9 @@ extern char *gitstrcasestr(const char *haystack, const char *needle);
>  #endif
>  
>  #ifdef NO_STRLCPY
> +#ifdef strlcpy
> +#undef strlcpy
> +#endif
If strlcpy is #defined, then presumably NO_STRLCPY should not be set, no?
>  #define strlcpy gitstrlcpy
>  extern size_t gitstrlcpy(char *, const char *, size_t);
>  #endif
> 
Hmm, which version of OS X are we talking about?
config.mak.uname contains this:
        ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
                NO_STRLCPY = YesPlease
What does ./configure put in config.mak.autogen for NO_STRLCPY?

(sorry, I don't have access to any version of OS X, so I can't offer much help on this).

ATB, Ramsay Jones

tsuna· Aug 24, 2014, 11:13 UTC · re: Ramsay Jones · lore

Re: [PATCH] Undefine strlcpy if needed.

On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:

> Hmm, which version of OS X are we talking about?
OS X 10.9.4:

$ uname -a Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun 3 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64

Show 6 quoted lines
> config.mak.uname contains this:
>
>         ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
>                 NO_STRLCPY = YesPlease
>
> What does ./configure put in config.mak.autogen for NO_STRLCPY?
NO_STRLCPY=

I guess I saw all the warnings because I did just a “git pull —rebase && make -j8” without running “make configure && ./configure”.

-- 
Benoit "tsuna" Sigoure
Ramsay Jones· Aug 24, 2014, 16:18 UTC · re: tsuna · lore

Re: [PATCH] Undefine strlcpy if needed.

On 24/08/14 12:13, tsuna wrote:
Show 9 quoted lines
> On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones
> <ramsay@ramsay1.demon.co.uk> wrote:
>> Hmm, which version of OS X are we talking about?
> 
> OS X 10.9.4:
> 
> $ uname -a
> Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun  3
> 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64
Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!)
Show 8 quoted lines
>> config.mak.uname contains this:
>>
>>         ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
>>                 NO_STRLCPY = YesPlease
>>
>> What does ./configure put in config.mak.autogen for NO_STRLCPY?
> 
> NO_STRLCPY=

OK, so I've got to my limit here! ;-) The conditional shown above (from config.mak.uname) should not have set NO_STRLCPY (assuming that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY is being set somewhere else (command-line, environment), this should just work. puzzled. :(

> 
> I guess I saw all the warnings because I did just a “git pull —rebase
> && make -j8” without running “make configure && ./configure”.
> 
Yes, but use of configure is supposed to be optional ...
Hopefully, someone who actually knows OS X can solve the mystery.

ATB, Ramsay Jones

Torsten Bögershausen· Aug 24, 2014, 19:49 UTC · re: Ramsay Jones · lore

Re: [PATCH] Undefine strlcpy if needed.

On 2014-08-24 18.18, Ramsay Jones wrote:
Show 39 quoted lines
> On 24/08/14 12:13, tsuna wrote:
>> On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones
>> <ramsay@ramsay1.demon.co.uk> wrote:
>>> Hmm, which version of OS X are we talking about?
>>
>> OS X 10.9.4:
>>
>> $ uname -a
>> Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun  3
>> 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64
> 
> Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!)
> 
>>> config.mak.uname contains this:
>>>
>>>         ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
>>>                 NO_STRLCPY = YesPlease
>>>
>>> What does ./configure put in config.mak.autogen for NO_STRLCPY?
>>
>> NO_STRLCPY=
> 
> OK, so I've got to my limit here! ;-) The conditional shown above
> (from config.mak.uname) should not have set NO_STRLCPY (assuming
> that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY
> is being set somewhere else (command-line, environment), this should
> just work. puzzled. :(
> 
>>
>> I guess I saw all the warnings because I did just a “git pull —rebase
>> && make -j8” without running “make configure && ./configure”.
>>
> 
> Yes, but use of configure is supposed to be optional ...
> 
> Hopefully, someone who actually knows OS X can solve the mystery.
> 
> ATB,
> Ramsay Jones

I need to admit that I can not reproduce the warning here, uname -r gives "13.3.0"

Could it be that something is special on your machine ? Something in the environment ? Does a fresh clone help ?

tsuna· Aug 24, 2014, 21:09 UTC · re: Torsten Bögershausen · lore

Re: [PATCH] Undefine strlcpy if needed.

On Sun, Aug 24, 2014 at 12:49 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 13 quoted lines
> On 2014-08-24 18.18, Ramsay Jones wrote:
>> On 24/08/14 12:13, tsuna wrote:
>>> On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones
>>> <ramsay@ramsay1.demon.co.uk> wrote:
>>>> Hmm, which version of OS X are we talking about?
>>>
>>> OS X 10.9.4:
>>>
>>> $ uname -a
>>> Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun  3
>>> 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64
>>
>> Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!)

$ uname -r 13.3.0

Show 32 quoted lines
>>>> config.mak.uname contains this:
>>>>
>>>>         ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
>>>>                 NO_STRLCPY = YesPlease
>>>>
>>>> What does ./configure put in config.mak.autogen for NO_STRLCPY?
>>>
>>> NO_STRLCPY=
>>
>> OK, so I've got to my limit here! ;-) The conditional shown above
>> (from config.mak.uname) should not have set NO_STRLCPY (assuming
>> that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY
>> is being set somewhere else (command-line, environment), this should
>> just work. puzzled. :(
>>
>>>
>>> I guess I saw all the warnings because I did just a “git pull —rebase
>>> && make -j8” without running “make configure && ./configure”.
>>>
>>
>> Yes, but use of configure is supposed to be optional ...
>>
>> Hopefully, someone who actually knows OS X can solve the mystery.
>>
>> ATB,
>> Ramsay Jones
>
> I need to admit that I can not reproduce the warning here,
> uname -r gives "13.3.0"
>
> Could it be that something is special on your machine ?
> Something in the environment  ?

Not that I can think of, the only "non-standard” thing I have installed is Homebrew (http://brew.sh/), but otherwise it’s all the standard OS X stuff and Developer tools. I write code on this machine on a daily basis.

> Does a fresh clone help ?
A fresh clone doesn’t even build :-/

$ git clone git://github.com/git/git.git Cloning into 'git'... remote: Counting objects: 176423, done. remote: Compressing objects: 100% (47201/47201), done. remote: Total 176423 (delta 127349), reused 176233 (delta 127209) Receiving objects: 100% (176423/176423), 64.05 MiB | 6.13 MiB/s, done. Resolving deltas: 100% (127349/127349), done. Checking connectivity... done. $ cd git

                                                                   $
make
GIT_VERSION = 2.1.0
    * new build flags
    CC credential-store.o
In file included from credential-store.c:1:
In file included from ./cache.h:8:
./gettext.h:17:11: fatal error: 'libintl.h' file not found
#       include <libintl.h>
                ^
1 error generated.
make: *** [credential-store.o] Error 1
I need to run configure first:
$ make configure
    GEN configure
$ ./configure
configure: Setting lib to 'lib' (the default)
[…]
$ make
tsuna@damogran /tmp/git $ make
    * new build flags
    CC credential-store.o
    * new link flags
    CC abspath.o
[…]
Then the build succeeds.

$ grep NO_STRLCPY config.mak.autogen NO_STRLCPY=

$ which cc /usr/bin/cc

$ cc --version
Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)
Target: x86_64-apple-darwin13.3.0
Thread model: posix
-- 
Benoit "tsuna” Sigoure
Ramsay Jones· Aug 25, 2014, 00:32 UTC · re: tsuna · lore

Re: [PATCH] Undefine strlcpy if needed.

On 24/08/14 22:09, tsuna wrote:
Show 82 quoted lines
> On Sun, Aug 24, 2014 at 12:49 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 2014-08-24 18.18, Ramsay Jones wrote:
>>> On 24/08/14 12:13, tsuna wrote:
>>>> On Sun, Aug 24, 2014 at 4:10 AM, Ramsay Jones
>>>> <ramsay@ramsay1.demon.co.uk> wrote:
>>>>> Hmm, which version of OS X are we talking about?
>>>>
>>>> OS X 10.9.4:
>>>>
>>>> $ uname -a
>>>> Darwin damogran.local 13.3.0 Darwin Kernel Version 13.3.0: Tue Jun  3
>>>> 21:27:35 PDT 2014; root:xnu-2422.110.17~1/RELEASE_X86_64 x86_64
>>>
>>> Hmm, does 'uname -r' return 13.3.0 or 13.4.0? (or something else!)
> 
> $ uname -r
> 13.3.0
> 
>>>>> config.mak.uname contains this:
>>>>>
>>>>>         ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2)
>>>>>                 NO_STRLCPY = YesPlease
>>>>>
>>>>> What does ./configure put in config.mak.autogen for NO_STRLCPY?
>>>>
>>>> NO_STRLCPY=
>>>
>>> OK, so I've got to my limit here! ;-) The conditional shown above
>>> (from config.mak.uname) should not have set NO_STRLCPY (assuming
>>> that 'uname -r' is returning 13.3.0 or 13.4.0). So, unless NO_STRLCPY
>>> is being set somewhere else (command-line, environment), this should
>>> just work. puzzled. :(
>>>
>>>>
>>>> I guess I saw all the warnings because I did just a “git pull —rebase
>>>> && make -j8” without running “make configure && ./configure”.
>>>>
>>>
>>> Yes, but use of configure is supposed to be optional ...
>>>
>>> Hopefully, someone who actually knows OS X can solve the mystery.
>>>
>>> ATB,
>>> Ramsay Jones
>>
>> I need to admit that I can not reproduce the warning here,
>> uname -r gives "13.3.0"
>>
>> Could it be that something is special on your machine ?
>> Something in the environment  ?
> 
> Not that I can think of, the only "non-standard” thing I have
> installed is Homebrew (http://brew.sh/), but otherwise it’s all the
> standard OS X stuff and Developer tools.  I write code on this machine
> on a daily basis.
> 
>> Does a fresh clone help ?
> 
> A fresh clone doesn’t even build :-/
> 
> $ git clone git://github.com/git/git.git
> Cloning into 'git'...
> remote: Counting objects: 176423, done.
> remote: Compressing objects: 100% (47201/47201), done.
> remote: Total 176423 (delta 127349), reused 176233 (delta 127209)
> Receiving objects: 100% (176423/176423), 64.05 MiB | 6.13 MiB/s, done.
> Resolving deltas: 100% (127349/127349), done.
> Checking connectivity... done.
> $ cd git
> 
>                                                                    $
> make
> GIT_VERSION = 2.1.0
>     * new build flags
>     CC credential-store.o
> In file included from credential-store.c:1:
> In file included from ./cache.h:8:
> ./gettext.h:17:11: fatal error: 'libintl.h' file not found
> #       include <libintl.h>
>                 ^
> 1 error generated.
> make: *** [credential-store.o] Error 1

Again, I don't have access to an OS X system, so I don't know which package provides libintl/gettext, but it seems to be missing on your system.

You can avoid the build failure, without running configure, by setting NO_GETTEXT=YesPlease in your config.mak file.

Show 9 quoted lines
> 
> 
> I need to run configure first:
> 
> $ make configure
>     GEN configure
> $ ./configure
> configure: Setting lib to 'lib' (the default)
> […]

So, presumably, configure has set NO_GETEXT=YesPlease in your config.mak.autogen file.

ATB, Ramsay Jones

tsuna· Aug 25, 2014, 01:54 UTC · re: Ramsay Jones · lore

Re: [PATCH] Undefine strlcpy if needed.

On Sun, Aug 24, 2014 at 5:32 PM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:

> Again, I don't have access to an OS X system, so I don't know
> which package provides libintl/gettext, but it seems to be missing
> on your system.

Probably yeah, those libraries don’t seem to be provided in standard with OS X or OS X’s development tools, so maybe the Makefile should also default to having NO_GETTEXT=YesPlease when on OS X.

Show 15 quoted lines
> You can avoid the build failure, without running configure, by
> setting NO_GETTEXT=YesPlease in your config.mak file.
>
>>
>>
>> I need to run configure first:
>>
>> $ make configure
>>     GEN configure
>> $ ./configure
>> configure: Setting lib to 'lib' (the default)
>> […]
>
> So, presumably, configure has set NO_GETEXT=YesPlease in your
> config.mak.autogen file.
Yes it did.

I don’t mind running configure, but so far Git has compiled fine without doing it. Should we fix the default values of NO_STRLCPY / NO_GETEXT on OS X?

-- 
Benoit "tsuna" Sigoure
Ramsay Jones· Aug 25, 2014, 11:16 UTC · re: tsuna · lore

Re: [PATCH] Undefine strlcpy if needed.

On 25/08/14 02:54, tsuna wrote:
Show 32 quoted lines
> On Sun, Aug 24, 2014 at 5:32 PM, Ramsay Jones
> <ramsay@ramsay1.demon.co.uk> wrote:
>> Again, I don't have access to an OS X system, so I don't know
>> which package provides libintl/gettext, but it seems to be missing
>> on your system.
> 
> Probably yeah, those libraries don’t seem to be provided in standard
> with OS X or OS X’s development tools, so maybe the Makefile should
> also default to having NO_GETTEXT=YesPlease when on OS X.
> 
>> You can avoid the build failure, without running configure, by
>> setting NO_GETTEXT=YesPlease in your config.mak file.
>>
>>>
>>>
>>> I need to run configure first:
>>>
>>> $ make configure
>>>     GEN configure
>>> $ ./configure
>>> configure: Setting lib to 'lib' (the default)
>>> […]
>>
>> So, presumably, configure has set NO_GETEXT=YesPlease in your
>> config.mak.autogen file.
> 
> Yes it did.
> 
> I don’t mind running configure, but so far Git has compiled fine
> without doing it.  Should we fix the default values of NO_STRLCPY /
> NO_GETEXT on OS X?
>

Is NO_STRLCPY still a problem with a fresh clone (and putting NO_GETEXT=YesPlease in your config.mak)? I still do not understand why you were getting those warnings; AFAICT it should not be happening! Also, Torsten could not reproduce.

As far as NO_GETTEXT is concerned, I have to defer to someone who has experience on that platform (I have _zero_ experience on OS X).

ATB, Ramsay Jones

← back to recent threads