[patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Maciej W. Rozycki]

Hello,

 This change:

2006-11-30  Christopher Faylor  <cgf@timesys.com>

	* win/tclWinPipe.c (TclpCreateProcess): Call cygwin-specific routine to
	set up windows environment.

broke building on versions of Cygwin predating 1.15.20.  The 
CW_SYNC_WINENV internal call was not previously available.  Here is a fix.

2007-05-10  Maciej W. Rozycki  <macro@mips.com>

	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
	for Cygwin 1.15.20+.

 OK to apply?

  Maciej

tcl_cw_sync_winenv.diff
Index: binutils-quilt/src/tcl/win/tclWinPipe.c
===================================================================
--- binutils-quilt.orig/src/tcl/win/tclWinPipe.c	2006-12-01 13:31:17.000000000 +0000
+++ binutils-quilt/src/tcl/win/tclWinPipe.c	2007-05-10 12:10:25.000000000 +0100
@@ -1228,7 +1228,8 @@
 
     BuildCommandLine(execPath, argc, argv, &cmdLine);
 
-#ifdef __CYGWIN__
+#if defined(__CYGWIN__) && (CYGWIN_VERSION_API_MAJOR > 1 || CYGWIN_VERSION_API_MINOR >= 154)
+    /* Only available in Cygwin 1.15.20+. */
     cygwin_internal (CW_SYNC_WINENV);
 #endif
 

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Christopher Faylor]

On Thu, May 10, 2007 at 12:42:39PM +0100, Maciej W. Rozycki wrote:
>Hello,
>
> This change:
>
>2006-11-30  Christopher Faylor  <cgf@timesys.com>
>
>	* win/tclWinPipe.c (TclpCreateProcess): Call cygwin-specific routine to
>	set up windows environment.
>
>broke building on versions of Cygwin predating 1.15.20.  The 
>CW_SYNC_WINENV internal call was not previously available.  Here is a fix.

There is no such thing as Cygwin 1.15.20.  I don't know exactly when this was
applied but I would just avoid referring to the exact version of Cygwin in the
ChangeLog.

>2007-05-10  Maciej W. Rozycki  <macro@mips.com>
>
>	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
>	for Cygwin 1.15.20+.
>
> OK to apply?

No, sorry.  If you are going to be testing versions like this then please
use CYGWIN_VERSION_USER_API_VERSION_COMBINED.

cgf

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Maciej W. Rozycki]

On Thu, 10 May 2007, Christopher Faylor wrote:

> There is no such thing as Cygwin 1.15.20.  I don't know exactly when this was
> applied but I would just avoid referring to the exact version of Cygwin in the
> ChangeLog.

 Hmm, your own words:

http://www.cygwin.com/ml/cygwin/2006-07/msg00037.html

Or do you mean the missing "-1" suffix?  If so, then sorry -- this can 
certainly be rectified.  I just copied the change including the comment 
from elsewhere as is.

> >2007-05-10  Maciej W. Rozycki  <macro@mips.com>
> >
> >	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
> >	for Cygwin 1.15.20+.
> >
> > OK to apply?
> 
> No, sorry.  If you are going to be testing versions like this then please
> use CYGWIN_VERSION_USER_API_VERSION_COMBINED.

 Well, no such macro as of 1.5.14, but it was introduced before 
CW_SYNC_WINENV (at CYGWIN_VERSION_API_MINOR == 147), so it would be OK.  
Except it's not going to guard against CW_SYNC_WINENV being undefined 
anyway (which is what I am addressing here) as it refers to a "user_data" 
structure variable, so it can only be used at the run time (the result is 
not a preprocessor constant).

 I could use something like:

#if CYGWIN_VERSION_DLL_MAKE_COMBINED(CYGWIN_VERSION_API_MAJOR, \
				     CYGWIN_VERSION_API_MINOR) >= 154

if you prefer -- does it look better to you?  It builds for me.  Either is 
fine by my standards.

 And BTW, where is "user_data" declared?  Certainly not in any header 
included from <cygwin/version.h>.

  Maciej

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Christopher Faylor]

On Mon, May 14, 2007 at 02:49:17PM +0100, Maciej W. Rozycki wrote:
>On Thu, 10 May 2007, Christopher Faylor wrote:
>
>> There is no such thing as Cygwin 1.15.20.  I don't know exactly when this was
>> applied but I would just avoid referring to the exact version of Cygwin in the
>> ChangeLog.
>
> Hmm, your own words:
>
>http://www.cygwin.com/ml/cygwin/2006-07/msg00037.html

You're quoting a message from me which does not mention 1.15.20.  It is
1.*5*.20.

There is no reason to argue about this when a simple removal of the
version is all that I'm asking for.  Changing the "for Cygwin
1.15.20(sic)+" to "when it is supported by the compilation environment"
would be sufficient.

>Or do you mean the missing "-1" suffix?  If so, then sorry -- this can 
>certainly be rectified.  I just copied the change including the comment 
>from elsewhere as is.
>
>> >2007-05-10  Maciej W. Rozycki  <macro@mips.com>
>> >
>> >	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
>> >	for Cygwin 1.15.20+.
>> >
>> > OK to apply?
>> 
>> No, sorry.  If you are going to be testing versions like this then please
>> use CYGWIN_VERSION_USER_API_VERSION_COMBINED.
>
> Well, no such macro as of 1.5.14, but it was introduced before 
>CW_SYNC_WINENV (at CYGWIN_VERSION_API_MINOR == 147), so it would be OK.  

Ok then I withdraw my suggestion.  Your code is fine as is.

cgf

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Maciej W. Rozycki]

On Mon, 14 May 2007, Christopher Faylor wrote:

> You're quoting a message from me which does not mention 1.15.20.  It is
> 1.*5*.20.
> 
> There is no reason to argue about this when a simple removal of the 
> version is all that I'm asking for.  Changing the "for Cygwin 
> 1.15.20(sic)+" to "when it is supported by the compilation environment" 
> would be sufficient.

 Well, that's a typo that I've kept missing -- sorry for the confusion.  
Here's an updated version.

2007-05-14  Maciej W. Rozycki  <macro@mips.com>

	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
	if supported by the compilation environment.

 OK to apply?

  Maciej

tcl_cw_sync_winenv.diff
Index: binutils-quilt/src/tcl/win/tclWinPipe.c
===================================================================
--- binutils-quilt.orig/src/tcl/win/tclWinPipe.c	2007-05-14 16:16:44.000000000 +0100
+++ binutils-quilt/src/tcl/win/tclWinPipe.c	2007-05-14 16:18:41.000000000 +0100
@@ -1228,7 +1228,9 @@
 
     BuildCommandLine(execPath, argc, argv, &cmdLine);
 
-#ifdef __CYGWIN__
+#if defined(__CYGWIN__) && \
+    (CYGWIN_VERSION_API_MAJOR > 0 || CYGWIN_VERSION_API_MINOR >= 154)
+    /* Only available in Cygwin 1.5.20+. */
     cygwin_internal (CW_SYNC_WINENV);
 #endif
 

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Christopher Faylor]

On Mon, May 14, 2007 at 05:29:00PM +0100, Maciej W. Rozycki wrote:
>On Mon, 14 May 2007, Christopher Faylor wrote:
>
>> You're quoting a message from me which does not mention 1.15.20.  It is
>> 1.*5*.20.
>> 
>> There is no reason to argue about this when a simple removal of the 
>> version is all that I'm asking for.  Changing the "for Cygwin 
>> 1.15.20(sic)+" to "when it is supported by the compilation environment" 
>> would be sufficient.
>
> Well, that's a typo that I've kept missing -- sorry for the confusion.  
>Here's an updated version.
>
>2007-05-14  Maciej W. Rozycki  <macro@mips.com>
>
>	* win/tclWinPipe.c (TclpCreateProcess): Only use CW_SYNC_WINENV
>	if supported by the compilation environment.
>
> OK to apply?

It's ok with me.  Keith, I assume you're ok with it, right?

cgf

>tcl_cw_sync_winenv.diff
>Index: binutils-quilt/src/tcl/win/tclWinPipe.c
>===================================================================
>--- binutils-quilt.orig/src/tcl/win/tclWinPipe.c	2007-05-14 16:16:44.000000000 +0100
>+++ binutils-quilt/src/tcl/win/tclWinPipe.c	2007-05-14 16:18:41.000000000 +0100
>@@ -1228,7 +1228,9 @@
> 
>     BuildCommandLine(execPath, argc, argv, &cmdLine);
> 
>-#ifdef __CYGWIN__
>+#if defined(__CYGWIN__) && \
>+    (CYGWIN_VERSION_API_MAJOR > 0 || CYGWIN_VERSION_API_MINOR >= 154)
>+    /* Only available in Cygwin 1.5.20+. */
>     cygwin_internal (CW_SYNC_WINENV);
> #endif
> 
>

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Keith Seitz]

Christopher Faylor wrote:
>>
>> OK to apply?
> 
> It's ok with me.  Keith, I assume you're ok with it, right?

If you are, I am: absolutely.

Keith

Re: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Christopher Faylor]

On Mon, May 14, 2007 at 09:58:18AM -0700, Keith Seitz wrote:
>Christopher Faylor wrote:
>>>
>>>OK to apply?
>>
>>It's ok with me.  Keith, I assume you're ok with it, right?
>
>If you are, I am: absolutely.

Ok.  It's checked in.  Thanks for the patch Maciej.

cgf

RE: [patch] Only use CW_SYNC_WINENV for Cygwin 1.15.20+

[Dave Korn]

On 14 May 2007 16:08, Christopher Faylor wrote:

> On Mon, May 14, 2007 at 02:49:17PM +0100, Maciej W. Rozycki wrote:
>> On Thu, 10 May 2007, Christopher Faylor wrote:
>> 
>>> There is no such thing as Cygwin 1.15.20.  I don't know exactly when this
>>> was applied but I would just avoid referring to the exact version of
>>> Cygwin in the ChangeLog.
>> 
>> Hmm, your own words:
>> 
>> http://www.cygwin.com/ml/cygwin/2006-07/msg00037.html
> 
> You're quoting a message from me which does not mention 1.15.20.  It is
> 1.*5*.20.


  Well, that regex /matches/ 1.15.20 .......   <gd'n'r>


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....