(I kind of cut up bits of quoting to try to address my thoughts/motivations better)
On Mon, 30 Jun 2025, Corinna Vinschen wrote: > On Jun 27 11:44, Jeremy Drake via Cygwin-patches wrote: > > On Fri, 27 Jun 2025, Corinna Vinschen wrote: > > > > > Hmmmm. So we only may dup2/open/close stdin/out/err? That's not > > > exactly what POSIX requires. > > > > In this fast-path. Otherwise, it will use the existing fork/exec > > implementation in newlib. Also, close can work for other fds (by setting > > them to cloexec for the duration). Note this is done holding > > lock_process, which seems to be the same lock around dtable, so it should > > be safe to temporarily muck about with file descriptors in this way. > > Probably something else that needs a comment ;) > > Indeed. Still, I'm not that happy with this code. It seems to cater > for native child processes in the first place, but Cygwin children are > more important and the code should not go out of its way to handle > native processes while neglecting cygwin processes. It should *at > least* already point into the direction the code is going to support > Cygwin children in the first place. Does that make sense? > Yeah. I don't have problems to use the fork/exec fallback for stuff > which just isn't implemented yet. I'm just reluctant if the code > implements only the border case for native children. > What exactly were you trying to accomplish with this patch? My original idea was to implement the "least common denominator" functionality that would apply to all child processes (cygwin and non-cygwin). This also seems to be what most users of posix_spawn use. That's mostly redirecting stdin/out/err. I wasn't particularly interested in getting into the startup code for Cygwin processes, but it turned out I had to hook up stderr, and then fchdir (because Cygwin processes will ignore the Win32 cwd if the cwdstuff is already populated in the cygheap). Looking at what llvm, rust, make, ninja do they don't seem to have file actions for non-stdin/out/err, and only use some pretty easily implemented parent-side flags (sigmask, sigdef, pgroup). I imagine the scheduler/schedparam are relatively easy too, but I don't think I've seen any of those build tools try to use them so I haven't looked into them. I was looking at hooking up (f)chdir because I knew there was a parameter to CreateProcess for it, but now looking back it may not be all that important. Perhaps I should drop it for this first cut and come back to it in a future patch. It kind of sounds like what you are envisioning is pushing this to a lower level, potentially even re-architecting child_info_spawn and related startup code (I don't know if handle_spawn would necessarily encapsulate everything) in terms of posix_spawn's parameters. I was not looking to get that deep into things. I probably won't be able to get back to really working on this for at least a week, but I'm hoping to at least get some comments written today.
