All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] CYGWIN: avoid implicit declaration warning
@ 2014-11-23 14:16 Torsten Bögershausen
  2014-11-23 17:12 ` Ramsay Jones
  2014-11-24 17:59 ` Jonathan Nieder
  0 siblings, 2 replies; 17+ messages in thread
From: Torsten Bögershausen @ 2014-11-23 14:16 UTC (permalink / raw)
  To: git; +Cc: ramsay, tboegi

gcc under cygwin reports several warnings like this:
 warning: implicit declaration of function 'memmem'
  [-Wimplicit-function-declaration]
This has been observed under CYGWIN-32 with GCC 4.7.3 as well
as CYGWIN-64 with gcc v4.8.3-5 x86-64

Do not #define _XOPEN_SOURCE 600 for CYGWIN.

Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
This may be a start for a patch, tested under CYGWIN-32,
both Windows7 and XP
 git-compat-util.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 400e921..cef2691 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -75,7 +75,8 @@
 # endif
 #elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
       !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
-      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__)
+      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__) && \
+      !defined(__CYGWIN__)
 #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
 #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
 #endif
-- 
1.9.1.dirty

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 14:16 [PATCH RFC] CYGWIN: avoid implicit declaration warning Torsten Bögershausen
@ 2014-11-23 17:12 ` Ramsay Jones
  2014-11-23 18:53   ` Junio C Hamano
  2014-11-24 17:59 ` Jonathan Nieder
  1 sibling, 1 reply; 17+ messages in thread
From: Ramsay Jones @ 2014-11-23 17:12 UTC (permalink / raw)
  To: Torsten Bögershausen, git

On 23/11/14 14:16, Torsten Bögershausen wrote:
> gcc under cygwin reports several warnings like this:
>  warning: implicit declaration of function 'memmem'
>   [-Wimplicit-function-declaration]
> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
> as CYGWIN-64 with gcc v4.8.3-5 x86-64

Heh, thanks for looking into this. Your email came at a good time,
since I was just about to boot my old laptop into windows XP to
test my patch on 32-bit cygwin! (If I had not been watching the
F1 Grand Prix on TV, I would already have done so! ;-) ).

It's been a while since I updated my 32-bit cygwin installation
(about 6 months) but I'm a little surprised you found this issue
with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
just to see what versions of software it is running).

Just for the reccord, my patch follows.

ATB,
Ramsay Jones

> 
> Do not #define _XOPEN_SOURCE 600 for CYGWIN.
> 
> Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> This may be a start for a patch, tested under CYGWIN-32,
> both Windows7 and XP
>  git-compat-util.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 400e921..cef2691 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -75,7 +75,8 @@
>  # endif
>  #elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
>        !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
> -      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__)
> +      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__) && \
> +      !defined(__CYGWIN__)
>  #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
>  #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
>  #endif
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 17:12 ` Ramsay Jones
@ 2014-11-23 18:53   ` Junio C Hamano
  2014-11-23 23:15     ` Ramsay Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2014-11-23 18:53 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Torsten Bögershausen, git

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> On 23/11/14 14:16, Torsten Bögershausen wrote:
>> gcc under cygwin reports several warnings like this:
>>  warning: implicit declaration of function 'memmem'
>>   [-Wimplicit-function-declaration]
>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>
> Heh, thanks for looking into this. Your email came at a good time,
> since I was just about to boot my old laptop into windows XP to
> test my patch on 32-bit cygwin! (If I had not been watching the
> F1 Grand Prix on TV, I would already have done so! ;-) ).
>
> It's been a while since I updated my 32-bit cygwin installation
> (about 6 months) but I'm a little surprised you found this issue
> with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
> just to see what versions of software it is running).

So you have an old installation to check how well the patched
version is accepted by the old set of header files?

>
> Just for the reccord, my patch follows.
>
> ATB,
> Ramsay Jones
>
>> 
>> Do not #define _XOPEN_SOURCE 600 for CYGWIN.
>> 
>> Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>> This may be a start for a patch, tested under CYGWIN-32,
>> both Windows7 and XP
>>  git-compat-util.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 400e921..cef2691 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -75,7 +75,8 @@
>>  # endif
>>  #elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
>>        !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
>> -      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__)
>> +      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__) && \
>> +      !defined(__CYGWIN__)
>>  #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
>>  #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
>>  #endif
>> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 18:53   ` Junio C Hamano
@ 2014-11-23 23:15     ` Ramsay Jones
  2014-11-24  7:20       ` Torsten Bögershausen
  2014-11-24 22:27       ` Ramsay Jones
  0 siblings, 2 replies; 17+ messages in thread
From: Ramsay Jones @ 2014-11-23 23:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git

On 23/11/14 18:53, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> On 23/11/14 14:16, Torsten Bögershausen wrote:
>>> gcc under cygwin reports several warnings like this:
>>>  warning: implicit declaration of function 'memmem'
>>>   [-Wimplicit-function-declaration]
>>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>>
>> Heh, thanks for looking into this. Your email came at a good time,
>> since I was just about to boot my old laptop into windows XP to
>> test my patch on 32-bit cygwin! (If I had not been watching the
>> F1 Grand Prix on TV, I would already have done so! ;-) ).
>>
>> It's been a while since I updated my 32-bit cygwin installation
>> (about 6 months) but I'm a little surprised you found this issue
>> with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
>> just to see what versions of software it is running).
> 
> So you have an old installation to check how well the patched
> version is accepted by the old set of header files?

... I can, indeed, use this old installation to test this on
32-bit cygwin. sigh, I thought I had dodged that bullet! ;-)

[I can't do this tonight and I'm pretty busy tomorrow, so it
may have to wait until tomorrow evening at the earliest. sorry
about that. :( ]

This does not help much with 64-bit cygwin. They are effectively
two different 'distributions'.

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 23:15     ` Ramsay Jones
@ 2014-11-24  7:20       ` Torsten Bögershausen
  2014-11-24 16:00         ` Torsten Bögershausen
  2014-11-24 21:27         ` [PATCH RFC] CYGWIN: avoid implicit declaration warning Ramsay Jones
  2014-11-24 22:27       ` Ramsay Jones
  1 sibling, 2 replies; 17+ messages in thread
From: Torsten Bögershausen @ 2014-11-24  7:20 UTC (permalink / raw)
  To: Ramsay Jones, Junio C Hamano; +Cc: Torsten Bögershausen, git

On 2014-11-24 00.15, Ramsay Jones wrote:
> On 23/11/14 18:53, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> On 23/11/14 14:16, Torsten Bögershausen wrote:
>>>> gcc under cygwin reports several warnings like this:
>>>>  warning: implicit declaration of function 'memmem'
>>>>   [-Wimplicit-function-declaration]
>>>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>>>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>>>
>>> Heh, thanks for looking into this. Your email came at a good time,
>>> since I was just about to boot my old laptop into windows XP to
>>> test my patch on 32-bit cygwin! (If I had not been watching the
>>> F1 Grand Prix on TV, I would already have done so! ;-) ).
>>>
>>> It's been a while since I updated my 32-bit cygwin installation
>>> (about 6 months) but I'm a little surprised you found this issue
>>> with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
>>> just to see what versions of software it is running).
>>
>> So you have an old installation to check how well the patched
>> version is accepted by the old set of header files?
> 
> ... I can, indeed, use this old installation to test this on
> 32-bit cygwin. sigh, I thought I had dodged that bullet! ;-)
> 

It depends what we mean with "old":
cygwin 1.5 is old, and I lost my test installation this summer:
One netbook was converted from XP to Linux, the other machine needs to be
re-installed and CYGWIN 1.5 is no longer available for download.

I can confirm that Ramsays patch works with CYGWIN 1.7 32 Bit.

 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24  7:20       ` Torsten Bögershausen
@ 2014-11-24 16:00         ` Torsten Bögershausen
  2014-11-24 18:29           ` [PATCH] t5000 on Windows: do not mistake "sh.exe" as "sh" Johannes Sixt
  2014-11-24 21:27         ` [PATCH RFC] CYGWIN: avoid implicit declaration warning Ramsay Jones
  1 sibling, 1 reply; 17+ messages in thread
From: Torsten Bögershausen @ 2014-11-24 16:00 UTC (permalink / raw)
  To: Torsten Bögershausen, Ramsay Jones, Junio C Hamano; +Cc: git

> 
> It depends what we mean with "old":
> cygwin 1.5 is old, and I lost my test installation this summer:
> One netbook was converted from XP to Linux, the other machine needs to be
> re-installed and CYGWIN 1.5 is no longer available for download.
> 
> I can confirm that Ramsays patch works with CYGWIN 1.7 32 Bit.
Another update:
The test suite passes, except
t5000 - not ok 48 - archive and :(glob)

fatal: pathspec '*.abc' did not match any files

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 14:16 [PATCH RFC] CYGWIN: avoid implicit declaration warning Torsten Bögershausen
  2014-11-23 17:12 ` Ramsay Jones
@ 2014-11-24 17:59 ` Jonathan Nieder
  2014-11-24 19:17   ` Torsten Bögershausen
  2014-11-24 21:41   ` Ramsay Jones
  1 sibling, 2 replies; 17+ messages in thread
From: Jonathan Nieder @ 2014-11-24 17:59 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, ramsay

Torsten Bögershausen wrote:

> gcc under cygwin reports several warnings like this:
>
>  warning: implicit declaration of function 'memmem'
>   [-Wimplicit-function-declaration]
>
> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>
> Do not #define _XOPEN_SOURCE 600 for CYGWIN.
>
> Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> This may be a start for a patch, tested under CYGWIN-32,
> both Windows7 and XP

The "tested under" part would also be a good addition to the commit
message.

>  git-compat-util.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Patch looks good to me.  Do you know if this has been reported to the
Cygwin maintainers?  The behavior seems counterintuitive --- I would
expect _GNU_SOURCE to override everything else (since I thought that
was the point of _GNU_SOURCE).

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] t5000 on Windows: do not mistake "sh.exe" as "sh"
  2014-11-24 16:00         ` Torsten Bögershausen
@ 2014-11-24 18:29           ` Johannes Sixt
  2014-11-24 20:02             ` Torsten Bögershausen
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Sixt @ 2014-11-24 18:29 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Ramsay Jones, Junio C Hamano, git

In their effort to emulate POSIX as close as possible, the MSYS tools
and Cygwin treat the file name "foo.exe" as "foo" when the latter is
asked for, but not present, but the former is present.

Following this rule, 'cp /bin/sh a/bin' actually copies the file
/bin/sh.exe, so that we now have a/bin/sh.exe in the repository. This
difference did not matter in the tests in the past because we were only
interested in the equality of contents generated in various ways. But
recently added tests check file names, in particular, the presence of
"a/bin/sh". This test fails on Windows, as we do not have a file by this
name, but "a/bin/sh.exe".

Use test-genrandom to generate the large binary file in the repository
under the expected name.

We could change the guilty line to 'cat /bin/sh >a/bin/sh', but it is
better for test reproducibility to ensure that the test data is the same
across platforms, which test-genrandom can guarantee.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 24.11.2014 um 17:00 schrieb Torsten Bögershausen:
> The test suite passes, except
> t5000 - not ok 48 - archive and :(glob)
> 
> fatal: pathspec '*.abc' did not match any files

I've this patch in my tree for a while.

 t/t5000-tar-tree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index d01bbdc..4b68bba 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -101,7 +101,7 @@ test_expect_success \
      ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
      echo long filename >a/four$hundred &&
      mkdir a/bin &&
-     cp /bin/sh a/bin &&
+     test-genrandom "frotz" 500000 >a/bin/sh &&
      printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
      printf "A not substituted O" >a/substfile2 &&
      if test_have_prereq SYMLINKS; then
-- 
2.0.0.12.gbcf935e

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 17:59 ` Jonathan Nieder
@ 2014-11-24 19:17   ` Torsten Bögershausen
  2014-11-24 21:41   ` Ramsay Jones
  1 sibling, 0 replies; 17+ messages in thread
From: Torsten Bögershausen @ 2014-11-24 19:17 UTC (permalink / raw)
  To: Jonathan Nieder, Torsten Bögershausen; +Cc: git, ramsay

> 
> Patch looks good to me.  Do you know if this has been reported to the
> Cygwin maintainers?  The behavior seems counterintuitive --- I would
> expect _GNU_SOURCE to override everything else (since I thought that
> was the point of _GNU_SOURCE).
I don't know, it seems that CYGWIN is now in class with some BSD and Mac OS X.
Actually Ramsay did send a patch, so that this is obsolete now.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] t5000 on Windows: do not mistake "sh.exe" as "sh"
  2014-11-24 18:29           ` [PATCH] t5000 on Windows: do not mistake "sh.exe" as "sh" Johannes Sixt
@ 2014-11-24 20:02             ` Torsten Bögershausen
  0 siblings, 0 replies; 17+ messages in thread
From: Torsten Bögershausen @ 2014-11-24 20:02 UTC (permalink / raw)
  To: Johannes Sixt, Torsten Bögershausen
  Cc: Ramsay Jones, Junio C Hamano, git

> 
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index d01bbdc..4b68bba 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -101,7 +101,7 @@ test_expect_success \
>       ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
>       echo long filename >a/four$hundred &&
>       mkdir a/bin &&
> -     cp /bin/sh a/bin &&
> +     test-genrandom "frotz" 500000 >a/bin/sh &&

That's a fast respose :-)
The patch works for me, Thanks

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24  7:20       ` Torsten Bögershausen
  2014-11-24 16:00         ` Torsten Bögershausen
@ 2014-11-24 21:27         ` Ramsay Jones
  2014-11-24 21:44           ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Ramsay Jones @ 2014-11-24 21:27 UTC (permalink / raw)
  To: Torsten Bögershausen, Junio C Hamano; +Cc: git

On 24/11/14 07:20, Torsten Bögershausen wrote:
> On 2014-11-24 00.15, Ramsay Jones wrote:
>> On 23/11/14 18:53, Junio C Hamano wrote:
>>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>>
>>>> On 23/11/14 14:16, Torsten Bögershausen wrote:
>>>>> gcc under cygwin reports several warnings like this:
>>>>>  warning: implicit declaration of function 'memmem'
>>>>>   [-Wimplicit-function-declaration]
>>>>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>>>>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>>>>
>>>> Heh, thanks for looking into this. Your email came at a good time,
>>>> since I was just about to boot my old laptop into windows XP to
>>>> test my patch on 32-bit cygwin! (If I had not been watching the
>>>> F1 Grand Prix on TV, I would already have done so! ;-) ).
>>>>
>>>> It's been a while since I updated my 32-bit cygwin installation
>>>> (about 6 months) but I'm a little surprised you found this issue
>>>> with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
>>>> just to see what versions of software it is running).
>>>
>>> So you have an old installation to check how well the patched
>>> version is accepted by the old set of header files?
>>
>> ... I can, indeed, use this old installation to test this on
>> 32-bit cygwin. sigh, I thought I had dodged that bullet! ;-)
>>
> 
> It depends what we mean with "old":
> cygwin 1.5 is old, and I lost my test installation this summer:

I updated from cygwin 1.5 to cygwin 1.7 at the beginning of the year.
Since it is no longer supported, I don't think we need to worry about
version 1.5. When I said 'old installation' I meant my old version 1.7
32-bit installation.

> One netbook was converted from XP to Linux, the other machine needs to be
> re-installed and CYGWIN 1.5 is no longer available for download.
> 
> I can confirm that Ramsays patch works with CYGWIN 1.7 32 Bit.

Thanks!

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 17:59 ` Jonathan Nieder
  2014-11-24 19:17   ` Torsten Bögershausen
@ 2014-11-24 21:41   ` Ramsay Jones
  1 sibling, 0 replies; 17+ messages in thread
From: Ramsay Jones @ 2014-11-24 21:41 UTC (permalink / raw)
  To: Jonathan Nieder, Torsten Bögershausen; +Cc: git

On 24/11/14 17:59, Jonathan Nieder wrote:
> Torsten Bögershausen wrote:
> 
>> gcc under cygwin reports several warnings like this:
>>
>>  warning: implicit declaration of function 'memmem'
>>   [-Wimplicit-function-declaration]
>>
>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>>
>> Do not #define _XOPEN_SOURCE 600 for CYGWIN.
>>
>> Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>> This may be a start for a patch, tested under CYGWIN-32,
>> both Windows7 and XP
> 
> The "tested under" part would also be a good addition to the commit
> message.
> 
>>  git-compat-util.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Patch looks good to me.  Do you know if this has been reported to the
> Cygwin maintainers?

As I said in an earlier email, I have searched the cygwin mailing list
(a few days ago now), but this issue had not been mentioned (modulo my
poor list searching fu!).

However, since I don't want to subscribe to (yet another) busy mailing
list, I won't be reporting this issue there. (If someone on this list
is already subscribed to that list, then ... ;-) ).

>                       The behavior seems counterintuitive --- I would
> expect _GNU_SOURCE to override everything else (since I thought that
> was the point of _GNU_SOURCE).

I had that impression too, but I need to read some more first.

Also, my theory about the cause of the problem has changed slightly
this evening, after booting up my old laptop and looking at an old
32-bit cygwin installation.

Rather than a change to '..../sys/cdefs.h' which changed the priority
of _XOPEN_SOURCE, it seems that it was the <string.h> header that
changed; those functions used to be declared unconditionally, whereas
the new headers have them within preprocessor conditionals.

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 21:27         ` [PATCH RFC] CYGWIN: avoid implicit declaration warning Ramsay Jones
@ 2014-11-24 21:44           ` Junio C Hamano
  2014-11-24 22:59             ` Ramsay Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2014-11-24 21:44 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Torsten Bögershausen, git

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> I updated from cygwin 1.5 to cygwin 1.7 at the beginning of the year.
> Since it is no longer supported, I don't think we need to worry about
> version 1.5. When I said 'old installation' I meant my old version 1.7
> 32-bit installation.
>
>> One netbook was converted from XP to Linux, the other machine needs to be
>> re-installed and CYGWIN 1.5 is no longer available for download.
>> 
>> I can confirm that Ramsays patch works with CYGWIN 1.7 32 Bit.
>
> Thanks!

Thanks.  So the unconditional version of the patch is good to go, I
take?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-23 23:15     ` Ramsay Jones
  2014-11-24  7:20       ` Torsten Bögershausen
@ 2014-11-24 22:27       ` Ramsay Jones
  2014-11-24 22:50         ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: Ramsay Jones @ 2014-11-24 22:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git, Jonathan Nieder

On 23/11/14 23:15, Ramsay Jones wrote:
> On 23/11/14 18:53, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> On 23/11/14 14:16, Torsten Bögershausen wrote:
>>>> gcc under cygwin reports several warnings like this:
>>>>  warning: implicit declaration of function 'memmem'
>>>>   [-Wimplicit-function-declaration]
>>>> This has been observed under CYGWIN-32 with GCC 4.7.3 as well
>>>> as CYGWIN-64 with gcc v4.8.3-5 x86-64
>>>
>>> Heh, thanks for looking into this. Your email came at a good time,
>>> since I was just about to boot my old laptop into windows XP to
>>> test my patch on 32-bit cygwin! (If I had not been watching the
>>> F1 Grand Prix on TV, I would already have done so! ;-) ).
>>>
>>> It's been a while since I updated my 32-bit cygwin installation
>>> (about 6 months) but I'm a little surprised you found this issue
>>> with gcc 4.7.3 (I'm _almost_ tempted to boot that laptop anyway
>>> just to see what versions of software it is running).
>>
>> So you have an old installation to check how well the patched
>> version is accepted by the old set of header files?
> 
> ... I can, indeed, use this old installation to test this on
> 32-bit cygwin. sigh, I thought I had dodged that bullet! ;-)
> 
> [I can't do this tonight and I'm pretty busy tomorrow, so it
> may have to wait until tomorrow evening at the earliest. sorry
> about that. :( ]

OK, so I had a look tonight and, as I told Jonathan earlier, my
theory for the cause of the issue has changed slightly.

First, I simply re-built git to make sure this issue wasn't present
(I would have been shocked if it was, but ...) and it was fine.
The master branch was at commit 7b69fcb (aka Git 2.1.0-rc1).
I applied my patch to this commit and re-built, again with no
problem. I then fetched the current version of git (i.e. commit
652e759 aka Git 2.2.0-rc3) and re-built; no problem. Finally, after
applying my patch to this commit, once again the build was free
from problems.

Note that I have not run any tests (it takes far too long), simply
a full build (make clean; make).

Some other things to note:

  $ uname -a
  CYGWIN_NT-5.1 toshiba 1.7.30(0.272/5/3) 2014-05-23 10:36 i686 Cygwin

  $ gcc --version
  gcc (GCC) 4.8.3
  Copyright (C) 2013 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions.  There is NO
  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

  $ ls -l /cygdrive/c/Documents\ and\ Settings/ramsay/Downloads/http%3a     %2f%2fmirrors.kernel.org%2fsourceware%2fcygwin%2f/x86/release/gcc/gcc-core/
  total 38M
  -rwx------+ 1 ramsay None 13M Nov 10  2013 gcc-core-4.8.2-1.tar.xz*
  -rwx------+ 1 ramsay None 13M Dec 22  2013 gcc-core-4.8.2-2.tar.xz*
  -rw-r--r--  1 ramsay None 13M Jun  3 13:06 gcc-core-4.8.3-1.tar.xz
  $ # ie this is the first build of the compiler package for v4.8.3

  $ cygcheck -f /usr/include/string.h
  cygwin-1.7.30-1
  $ cygcheck -f /usr/include/sys/cdefs.h
  cygwin-1.7.30-1

  $ ls /usr/lib/gcc/i686-pc-cygwin/4.8.3/include-fixed/
  limits.h  README  syslimits.h

While on my (new) 64-bit installation:

  $ uname -a
  CYGWIN_NT-6.3 satellite 1.7.33-2(0.280/5/3) 2014-11-13 15:47 x86_64 Cygwin
  $ gcc --version
  gcc (GCC) 4.8.3
  Copyright (C) 2013 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions.  There is NO
  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

  $ ls -l /cygdrive/c/Users/ramsay/Downloads/http%3a%2f%2fmirrors.kernel.org%2fsourceware%2fcygwin%2f/x86_64/release/gcc/gcc-core/
  total 66M
  -rw-r--r-- 1 ramsay None 13M May 18  2014 gcc-core-4.8.2-3.tar.xz
  -rw-r--r-- 1 ramsay None 13M Jun  7 00:06 gcc-core-4.8.3-2.tar.xz
  -rw-r--r-- 1 ramsay None 14M Aug 30 22:48 gcc-core-4.8.3-3.tar.xz
  -rw-r--r-- 1 ramsay None 14M Nov 12 20:19 gcc-core-4.8.3-4.tar.xz
  -rw-r--r-- 1 ramsay None 14M Nov 20 13:07 gcc-core-4.8.3-5.tar.xz
  $ # ie this is the fifth build of the compiler package for v4.8.3

  $ cygcheck -f /usr/include/string.h
  cygwin-devel-1.7.33-1
  $ cygcheck -f /usr/include/sys/cdefs.h
  cygwin-devel-1.7.33-1
  $ cygcheck -f /usr/lib/gcc/x86_64-pc-cygwin/4.8.3/include-fixed/sys/cdefs.h
  gcc-core-4.8.3-5
  $ 

Although I have not done an actual diff of the various cdef.h files, they
do appear to be more or less the same. In other words, I no longer think
that the change results from a 'change in priority of _XOPEN_SOURCE'. The
issue is simply that in the old <string.h> header these functions were
declared unconditionally; in the new headers the are contained within
preprocessor conditionals using the __GNU_VISIBLE and __BSD_VISIBLE macros
which are not set when _XOPEN_SOURCE is set (despite _GNU_SOURCE and
_BSD_SOURCE being set).

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 22:27       ` Ramsay Jones
@ 2014-11-24 22:50         ` Junio C Hamano
  2014-11-24 23:04           ` Ramsay Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2014-11-24 22:50 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Torsten Bögershausen, git, Jonathan Nieder

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> ...
> Although I have not done an actual diff of the various cdef.h files, they
> do appear to be more or less the same. In other words, I no longer think
> that the change results from a 'change in priority of _XOPEN_SOURCE'. The
> issue is simply that in the old <string.h> header these functions were
> declared unconditionally; in the new headers the are contained within
> preprocessor conditionals using the __GNU_VISIBLE and __BSD_VISIBLE macros
> which are not set when _XOPEN_SOURCE is set (despite _GNU_SOURCE and
> _BSD_SOURCE being set).

So I can take your version [*1*], drop this bit from the log:

    This seems to be caused by a change to the system headers which
    results in the _XOPEN_SOURCE macro now having prioity over the
    _GNU_SOURCE and _BSD_SOURCE macros (they are simply ignored).
    This in turn leads to the declarations of the above functions
    to be suppressed.

and replace it with something like:

    <string.h> on Cygwin used to always declare the above functions,
    but a recent version of it no longer make them visible when
    _XOPEN_SOURCE is set (even if _GNU_SOURCE and _BSD_SOURCE is
    set).

and keep the rest, I think.

Thanks for digging this thoroughly.


[Reference]

*1* http://article.gmane.org/gmane.comp.version-control.git/260091

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 21:44           ` Junio C Hamano
@ 2014-11-24 22:59             ` Ramsay Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Ramsay Jones @ 2014-11-24 22:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git, Jonathan Nieder

On 24/11/14 21:44, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> I updated from cygwin 1.5 to cygwin 1.7 at the beginning of the year.
>> Since it is no longer supported, I don't think we need to worry about
>> version 1.5. When I said 'old installation' I meant my old version 1.7
>> 32-bit installation.
>>
>>> One netbook was converted from XP to Linux, the other machine needs to be
>>> re-installed and CYGWIN 1.5 is no longer available for download.
>>>
>>> I can confirm that Ramsays patch works with CYGWIN 1.7 32 Bit.
>>
>> Thanks!
> 
> Thanks.  So the unconditional version of the patch is good to go, I
> take?
> 

Hmm, I don't know what you mean by 'unconditional version'. ;-)

Anyway, the commit message of my patch needs some edits to reflect
my new 'theory' of the cause. I suppose I should try to track down
the changes to the cygwin headers to be more confident that I have
actually identified the correct cause. (Like Jonathan, I'm still a
bit surprised that _GNU_SOURCE doesn't trump _XOPEN_SOURCE, but I
can't say that it is a bug).

However, I suspect that the patch as is, modulo commit message edits,
should be sufficient to fix this up (at least for now).

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC] CYGWIN: avoid implicit declaration warning
  2014-11-24 22:50         ` Junio C Hamano
@ 2014-11-24 23:04           ` Ramsay Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Ramsay Jones @ 2014-11-24 23:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git, Jonathan Nieder

On 24/11/14 22:50, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> ...
>> Although I have not done an actual diff of the various cdef.h files, they
>> do appear to be more or less the same. In other words, I no longer think
>> that the change results from a 'change in priority of _XOPEN_SOURCE'. The
>> issue is simply that in the old <string.h> header these functions were
>> declared unconditionally; in the new headers the are contained within
>> preprocessor conditionals using the __GNU_VISIBLE and __BSD_VISIBLE macros
>> which are not set when _XOPEN_SOURCE is set (despite _GNU_SOURCE and
>> _BSD_SOURCE being set).
> 
> So I can take your version [*1*], drop this bit from the log:
> 
>     This seems to be caused by a change to the system headers which
>     results in the _XOPEN_SOURCE macro now having prioity over the
>     _GNU_SOURCE and _BSD_SOURCE macros (they are simply ignored).
>     This in turn leads to the declarations of the above functions
>     to be suppressed.
> 
> and replace it with something like:
> 
>     <string.h> on Cygwin used to always declare the above functions,
>     but a recent version of it no longer make them visible when
>     _XOPEN_SOURCE is set (even if _GNU_SOURCE and _BSD_SOURCE is
>     set).
> 
> and keep the rest, I think.

Perfect! Thanks! (actually, another minor typo: the second warning
message, indent the ^ by two spaces - it should be pointing to the
= symbol).

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-11-24 23:05 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-23 14:16 [PATCH RFC] CYGWIN: avoid implicit declaration warning Torsten Bögershausen
2014-11-23 17:12 ` Ramsay Jones
2014-11-23 18:53   ` Junio C Hamano
2014-11-23 23:15     ` Ramsay Jones
2014-11-24  7:20       ` Torsten Bögershausen
2014-11-24 16:00         ` Torsten Bögershausen
2014-11-24 18:29           ` [PATCH] t5000 on Windows: do not mistake "sh.exe" as "sh" Johannes Sixt
2014-11-24 20:02             ` Torsten Bögershausen
2014-11-24 21:27         ` [PATCH RFC] CYGWIN: avoid implicit declaration warning Ramsay Jones
2014-11-24 21:44           ` Junio C Hamano
2014-11-24 22:59             ` Ramsay Jones
2014-11-24 22:27       ` Ramsay Jones
2014-11-24 22:50         ` Junio C Hamano
2014-11-24 23:04           ` Ramsay Jones
2014-11-24 17:59 ` Jonathan Nieder
2014-11-24 19:17   ` Torsten Bögershausen
2014-11-24 21:41   ` Ramsay Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.