Re: [PATCH 1/1] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn
On Tue, 8 Dec 2020, Jon Turney wrote:
> On 07/12/2020 09:43, Corinna Vinschen wrote:
> > On Dec 4 10:35, Jeremy Drake via Cygwin-patches wrote:
> > > On Fri, 4 Dec 2020, Corinna Vinschen via Cygwin-patches wrote:
> > >
> > > > I'm not happy about a new CYGWIN option.
> > > >
> > > > Wouldn't it make sense, perhaps, to switch to CREATE_DEFAULT_ERROR_MODE
> > > > for all non-Cygwin processes by default instead?
>
> I agree.
>
> Cygwin calls SetErrorMode(), so we need to use this flag to prevent that
> non-default behaviour being inherited by processes started with
> CreateProcess().
>
In that case, here's my initial, much simpler patch
-- >8 --
Subject: [PATCH] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn
This allows native processes to get Windows-default error handling
behavior (such as invoking the registered JIT debugger), while cygwin
processes would quickly set their error mode back to what they expect.
---
winsup/cygwin/spawn.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index c77d62984..f5952d53b 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -429,7 +429,8 @@ child_info_spawn::worker (const char *prog_arg, const char
*const *argv,
c_flags = GetPriorityClass (GetCurrentProcess ());
sigproc_printf ("priority class %d", c_flags);
- c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT;
+ c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT
+ | CREATE_DEFAULT_ERROR_MODE;
/* We're adding the CREATE_BREAKAWAY_FROM_JOB flag here to workaround
issues with the "Program Compatibility Assistant (PCA) Service".
Re: [PATCH] Use automake (v3)
On 02/12/2020 19:03, Corinna Vinschen via Cygwin-patches wrote: On Dec 2 13:33, Yaakov Selkowitz via Cygwin-patches wrote: On Wed, 2020-12-02 at 18:03 +, Jon Turney wrote: On 02/12/2020 17:05, Corinna Vinschen via Cygwin-patches wrote: On Dec 2 15:36, Jon Turney wrote: On 01/12/2020 09:18, Corinna Vinschen wrote: What bugs me is that the mingw executables are built in utils/mingw, but the object files are still in utils. Any problem generating the object files in utils/mingw, too? Not easily. This behaviour can be turned off by not using the 'subdir- objects' automake option. But then automake warns that option is disabled (since it's going to be the default in future). So why not just move the mingw source files to utils/mingw, too? There's probably some scope for doing that, but not in all cases, as some files are built multiple times with different compilers and/or flags. e.g. path.cc is built with a cygwin compiler and -DFSTAB as part of mount, with a MinGW compiler as part of cygcheck, and with a MinGW compiler and -DTESTSUITE as part of path-testsuite. Then something like: $ cat > winsup/utils/mingw/path.cc <<_EOF #define MINGW // whatever is needed here... #include "../path.cc" _EOF ?? +1 Sure, there are plenty of ways of rearranging the code to address this. I'm not sure I see what the benefit of that additional complexity is.
Re: [PATCH] Cygwin: Launch cygmagic with bash, not sh
On 07/12/2020 06:17, Mark Geisert wrote: On some systems /bin/sh is not /bin/bash and cygmagic has bash-isms in it. So even though cygmagic has a /bin/bash shebang, it also needs to be launched with bash from within Makefile.in. Thanks. Since cygmagic is executable, I don't think we actually need to mention the shell here at all, but this is fine.
Re: [PATCH 1/1] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn
On 07/12/2020 09:43, Corinna Vinschen wrote: On Dec 4 10:35, Jeremy Drake via Cygwin-patches wrote: On Fri, 4 Dec 2020, Corinna Vinschen via Cygwin-patches wrote: I'm not happy about a new CYGWIN option. Wouldn't it make sense, perhaps, to switch to CREATE_DEFAULT_ERROR_MODE for all non-Cygwin processes by default instead? I agree. Cygwin calls SetErrorMode(), so we need to use this flag to prevent that non-default behaviour being inherited by processes started with CreateProcess(). In fact, my first iteration was to set that flag unconditionally (relying on the fact that SetErrorMode is called extremely early in Cygwin process startup rather than only setting it for non-Cygwin processes), but I received feedback that it would be better to put it behind an option: https://github.com/msys2/msys2-runtime/pull/18#issuecomment-723683606 I don't really understand what that comment is saying. If Git for Windows doesn't want the default SetErrorMode(), it should change it itself. (The same executable not started by Cygwin will get the default error mode in any case). Jon, thoughts on that as GDB maintainer? I don't think this actually interacts with gdb. gdb either CreateProcess() with DEBUG_PROCESS (which presumably overrides that flag) or attaches to an existing process (possibly due to being configured as a JIT debugger).
Re: [PATCH] Cygwin: Allow to set SO_PEERCRED zero (v2)
On Dec 8 10:47, Corinna Vinschen via Cygwin-patches wrote: > Hi Mark, > > On Dec 7 19:25, Mark Geisert wrote: > > Hi Corinna, > > > > Corinna Vinschen via Cygwin-patches wrote: > > > On Dec 7 16:30, Corinna Vinschen via Cygwin-patches wrote: > > > > On Dec 7 02:29, Mark Geisert wrote: > > > > > The existing code errors as EINVAL any attempt to set a value for > > > > > SO_PEERCRED via setsockopt() on an AF_UNIX/AF_LOCAL socket. But to > > > > > enable the workaround set_no_getpeereid behavior for Python one has > > > > > to be able to set SO_PEERCRED to zero. Ergo, this patch. Python has > > > > > no way to specify a NULL pointer for 'optval'. > > > > > > > > > > This v2 of patch allows the original working (i.e., allow NULL,0 for > > > > > optval,optlen to mean turn off SO_PEERCRED) in addition to the new > > > > > working described above. The sense of the 'if' stmt is reversed for > > > > > readability. > > > > > > > > > > --- > > [...] > > > > > -- > > > > > 2.29.2 > > > > > > > > Pushed > > > > > > I created new developer snapshots for testing. > > > > I didn't phrase my comment somewhere about "future snapshot TBA" as I had > > intended. I just meant some future snapshot, not that I was requesting one > > for this patch. But thank you very much anyway. > > I freely admit I didn't actually read your comment :} ...the part talking about a future snapshot, that is... > > I just created this snapshot because it seemed useful, so all is well :) > > > Corinna
Re: [PATCH] Cygwin: Allow to set SO_PEERCRED zero (v2)
Hi Mark, On Dec 7 19:25, Mark Geisert wrote: > Hi Corinna, > > Corinna Vinschen via Cygwin-patches wrote: > > On Dec 7 16:30, Corinna Vinschen via Cygwin-patches wrote: > > > On Dec 7 02:29, Mark Geisert wrote: > > > > The existing code errors as EINVAL any attempt to set a value for > > > > SO_PEERCRED via setsockopt() on an AF_UNIX/AF_LOCAL socket. But to > > > > enable the workaround set_no_getpeereid behavior for Python one has > > > > to be able to set SO_PEERCRED to zero. Ergo, this patch. Python has > > > > no way to specify a NULL pointer for 'optval'. > > > > > > > > This v2 of patch allows the original working (i.e., allow NULL,0 for > > > > optval,optlen to mean turn off SO_PEERCRED) in addition to the new > > > > working described above. The sense of the 'if' stmt is reversed for > > > > readability. > > > > > > > > --- > [...] > > > > -- > > > > 2.29.2 > > > > > > Pushed > > > > I created new developer snapshots for testing. > > I didn't phrase my comment somewhere about "future snapshot TBA" as I had > intended. I just meant some future snapshot, not that I was requesting one > for this patch. But thank you very much anyway. I freely admit I didn't actually read your comment :} I just created this snapshot because it seemed useful, so all is well :) Corinna
