* [PATCH} define _GNU_SOURCE on Cygwin @ 2016-10-26 10:04 Jun T. 2016-10-26 10:56 ` Daniel Shahaf 0 siblings, 1 reply; 8+ messages in thread From: Jun T. @ 2016-10-26 10:04 UTC (permalink / raw) To: zsh-workers Cygwin now started to require _XOPEN_SOURCE also for wcwidth() (in addition to strptime(); cf. worker/38862). Since wcwidth() is used in may files, I think it would be better to define _GNU_SOURCE in zsh_system.h. (defining _XOPEN_SOURCE alone hides may other function) diff --git a/Src/Modules/datetime.c b/Src/Modules/datetime.c index b924392..bb82c54 100644 --- a/Src/Modules/datetime.c +++ b/Src/Modules/datetime.c @@ -27,9 +27,6 @@ * */ -#ifdef __CYGWIN__ -#define _XOPEN_SOURCE -#endif #include "datetime.mdh" #include "datetime.pro" #include <time.h> diff --git a/Src/zsh_system.h b/Src/zsh_system.h index a9cbf02..5339b49 100644 --- a/Src/zsh_system.h +++ b/Src/zsh_system.h @@ -37,7 +37,7 @@ #endif #endif -#if defined(__linux) || defined(__GNU__) || defined(__GLIBC__) || defined(LIBC_MUSL) +#if defined(__linux) || defined(__GNU__) || defined(__GLIBC__) || defined(LIBC_MUSL) || defined(__CYGWIN__) /* * Turn on numerous extensions. * This is in order to get the functions for manipulating /dev/ptmx. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-26 10:04 [PATCH} define _GNU_SOURCE on Cygwin Jun T. @ 2016-10-26 10:56 ` Daniel Shahaf 2016-10-26 12:30 ` Jun T. 0 siblings, 1 reply; 8+ messages in thread From: Daniel Shahaf @ 2016-10-26 10:56 UTC (permalink / raw) To: Jun T.; +Cc: zsh-workers Jun T. wrote on Wed, Oct 26, 2016 at 19:04:08 +0900: > Cygwin now started to require _XOPEN_SOURCE also for > wcwidth() (in addition to strptime(); cf. worker/38862). > > Since wcwidth() is used in may files, I think it would be > better to define _GNU_SOURCE in zsh_system.h. > > (defining _XOPEN_SOURCE alone hides may other function) The patch causes _XOPEN_SOURCE to no longer be defined, ever. Is this okay for both old and new cygwin? I.e., does this patch work for people compiling new zsh on old cygwin? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-26 10:56 ` Daniel Shahaf @ 2016-10-26 12:30 ` Jun T. 2016-10-26 19:55 ` Peter A. Castro 0 siblings, 1 reply; 8+ messages in thread From: Jun T. @ 2016-10-26 12:30 UTC (permalink / raw) To: zsh-workers 2016/10/26 19:56, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > The patch causes _XOPEN_SOURCE to no longer be defined, ever. _GNU_SOURCE is a superset of _XOPEN_SOURCE; it is virtually equivalent to defining both _BSD_SOURCE and _XOPEN_SOURCE. > Is this okay for both old and new cygwin? I.e., does this patch work > for people compiling new zsh on old cygwin? Sorry, I can't test on older cygwin. I guess installing older cygwin is not trivial. Unless someone still has older cygwin can test the patch, what we can do would be to either go with _GNU_SOURCE (and to see whether incompatibility with older cygwin comes out or not), or just continue using the current code, which generates some warnings but still seems to work fine (the return value of wcwidth() is int, which is what compiler assumes when no prototype is available). ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-26 12:30 ` Jun T. @ 2016-10-26 19:55 ` Peter A. Castro 2016-10-27 1:34 ` Daniel Shahaf 0 siblings, 1 reply; 8+ messages in thread From: Peter A. Castro @ 2016-10-26 19:55 UTC (permalink / raw) To: Jun T.; +Cc: zsh-workers On Wed, 26 Oct 2016, Jun T. wrote: > Date: Wed, 26 Oct 2016 21:30:26 +0900 > From: Jun T. <takimoto-j@kba.biglobe.ne.jp> > To: zsh-workers@zsh.org > Subject: Re: [PATCH} define _GNU_SOURCE on Cygwin > > > 2016/10/26 19:56, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: >> >> The patch causes _XOPEN_SOURCE to no longer be defined, ever. > > _GNU_SOURCE is a superset of _XOPEN_SOURCE; it is virtually > equivalent to defining both _BSD_SOURCE and _XOPEN_SOURCE. > >> Is this okay for both old and new cygwin? I.e., does this patch work >> for people compiling new zsh on old cygwin? > > Sorry, I can't test on older cygwin. > I guess installing older cygwin is not trivial. It is fairly trivial, actually. You just need to know where to get it. :-) > Unless someone still has older cygwin can test the patch, what we > can do would be to either go with _GNU_SOURCE (and to see whether > incompatibility with older cygwin comes out or not), or just > continue using the current code, which generates some warnings > but still seems to work fine (the return value of wcwidth() is int, > which is what compiler assumes when no prototype is available). I'll see if I can give it a quick build and let you know. -- --=> Peter A. Castro Email: doctor at fruitbat dot org / Peter dot Castro at oracle dot com "Cats are just autistic Dogs" -- Dr. Tony Attwood ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-26 19:55 ` Peter A. Castro @ 2016-10-27 1:34 ` Daniel Shahaf 2016-10-31 6:15 ` Peter A. Castro 0 siblings, 1 reply; 8+ messages in thread From: Daniel Shahaf @ 2016-10-27 1:34 UTC (permalink / raw) To: Peter A. Castro; +Cc: Jun T., zsh-workers Peter A. Castro wrote on Wed, Oct 26, 2016 at 12:55:36 -0700: > On Wed, 26 Oct 2016, Jun T. wrote: > >Unless someone still has older cygwin can test the patch, what we > >can do would be to either go with _GNU_SOURCE (and to see whether > >incompatibility with older cygwin comes out or not), or just > >continue using the current code, which generates some warnings > >but still seems to work fine (the return value of wcwidth() is int, > >which is what compiler assumes when no prototype is available). > > I'll see if I can give it a quick build and let you know. Thanks, Peter. I wasn't meaning to cause the two of you extra work; I had assumed defining both _XOPEN_SOURCE and _GNU_SOURCE would work on both old and new cygwin. I.e., just adding _GNU_SOURCE without removing _XOPEN_SOURCE. Cheers, Daniel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-27 1:34 ` Daniel Shahaf @ 2016-10-31 6:15 ` Peter A. Castro 2016-11-01 12:11 ` Jun T. 0 siblings, 1 reply; 8+ messages in thread From: Peter A. Castro @ 2016-10-31 6:15 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Jun T., zsh-workers On Thu, 27 Oct 2016, Daniel Shahaf wrote: Greetings, Daniel and Jun, > Peter A. Castro wrote on Wed, Oct 26, 2016 at 12:55:36 -0700: >> On Wed, 26 Oct 2016, Jun T. wrote: >>> Unless someone still has older cygwin can test the patch, what we >>> can do would be to either go with _GNU_SOURCE (and to see whether >>> incompatibility with older cygwin comes out or not), or just >>> continue using the current code, which generates some warnings >>> but still seems to work fine (the return value of wcwidth() is int, >>> which is what compiler assumes when no prototype is available). >> >> I'll see if I can give it a quick build and let you know. > > Thanks, Peter. > > I wasn't meaning to cause the two of you extra work; I had assumed > defining both _XOPEN_SOURCE and _GNU_SOURCE would work on both old and > new cygwin. I.e., just adding _GNU_SOURCE without removing > _XOPEN_SOURCE. I keep an "older" Cygwin build environment for just such an occasion. :) Now, I have some questions and a report. First, the report: I built with Cygwin 1.7.35, which is before the major change to 2.0, but after the "legacy" release (v1.5). I could try building with the "legacy" release, but there is almost no reason to do so as the version of Windows supported by "legacy" is quickly dropping off the map of active Windows hosts (as well as being desupported by MS), and there is a version of zsh already built for it. With the proposed patch from Jun, zsh compiles, links and runs the check tests "adequately". I say "adequately" because there were 2 test failures, 5 skipped and 40 successful. But that has always been the case for the check tests for me on Cygwin, so at the very least there are no new failures. :) Can you perhaps suggest some other tests that this change might influence? I haven't look at all the possibly dependencies involed, but if _GNU_SOURCE is really a superset, then there really should be no effective difference. Now, about the patch itself: Jun's patch changes two files: datetime.c and zsh_system.h. However, I did my build testing with the stock zsh-5.2 source and the datetime.c change doesn't apply because the lines indicated are not present in the source. I'm presuming Jun was working from latest git source. As a result, I only applied the change to zsh_system.h, which was successful. So, in lue of other testing, I'd call it a success. If there are other tests you'd like me to perform, please let me know. I'll keep this build of a while longer. > Cheers, > Daniel -- --=> Peter A. Castro Email: doctor at fruitbat dot org / Peter dot Castro at oracle dot com "Cats are just autistic Dogs" -- Dr. Tony Attwood ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-10-31 6:15 ` Peter A. Castro @ 2016-11-01 12:11 ` Jun T. 2016-11-01 16:31 ` Peter A. Castro 0 siblings, 1 reply; 8+ messages in thread From: Jun T. @ 2016-11-01 12:11 UTC (permalink / raw) To: zsh-workers On 2016/10/31, at 15:15, Peter A. Castro <doctor@fruitbat.org> wrote: > I built with Cygwin 1.7.35 Thanks. > With the proposed patch from Jun, zsh compiles, links and runs the check tests "adequately". Did you get any "new" warnings when building with the patch? (new = no warning without the patch) > I say "adequately" because there were 2 test failures, 5 skipped and 40 successful. Do the same 2 tests fail if you do not apply the patch? > As a result, I only applied the change to zsh_system.h, which was successful. Yes, that is sufficient to test the patch. Jun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH} define _GNU_SOURCE on Cygwin 2016-11-01 12:11 ` Jun T. @ 2016-11-01 16:31 ` Peter A. Castro 0 siblings, 0 replies; 8+ messages in thread From: Peter A. Castro @ 2016-11-01 16:31 UTC (permalink / raw) To: Jun T.; +Cc: zsh-workers On Tue, 1 Nov 2016, Jun T. wrote: > Date: Tue, 1 Nov 2016 21:11:22 +0900 > From: Jun T. Greetings, Jun, > On 2016/10/31, at 15:15, Peter A. Castro wrote: > >> I built with Cygwin 1.7.35 > > Thanks. You are welcome. :) >> With the proposed patch from Jun, zsh compiles, links and runs the check tests "adequately". > > Did you get any "new" warnings when building with the patch? > (new = no warning without the patch) No, no "new" warnings. >> I say "adequately" because there were 2 test failures, 5 skipped and 40 successful. > > Do the same 2 tests fail if you do not apply the patch? As I said, I have always gotten 2 test failures (essentially the same tests failing). Your patch does not change this result. >> As a result, I only applied the change to zsh_system.h, which was successful. > > Yes, that is sufficient to test the patch. Fair enough. > Jun -- --=> Peter A. Castro Email: doctor at fruitbat dot org / Peter dot Castro at oracle dot com "Cats are just autistic Dogs" -- Dr. Tony Attwood ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-11-01 16:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-26 10:04 [PATCH} define _GNU_SOURCE on Cygwin Jun T. 2016-10-26 10:56 ` Daniel Shahaf 2016-10-26 12:30 ` Jun T. 2016-10-26 19:55 ` Peter A. Castro 2016-10-27 1:34 ` Daniel Shahaf 2016-10-31 6:15 ` Peter A. Castro 2016-11-01 12:11 ` Jun T. 2016-11-01 16:31 ` Peter A. Castro
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).