Re: master 2c79a8f 2/2: Use posix_spawn if possible.

[Philipp Stephani <p.stephani2@gmail.com>]

(I'm still trying to set up some emulated Windows installation, but
haven't really been successful so far.)

Am Sa., 26. Dez. 2020 um 13:08 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> But many of the breaking changes can be found by simple inspection.
> For example, in the case in point, if we are not going to use
> posix_spawn on MS-Windows, we need to augment nt/mingw-cfg.site and/or
> nt/gnulib-cfg.mk to avoid compiling the respective Gnulib modules,
> since doing that just risks causing trouble (as happened in this
> case).  Also, since you deduced (correctly) that the Gnulib
> posix_spawn module cannot be used on MS-Windows, it isn't enough to
> have a run-time condition for bypassing that, you need to #ifdef away
> the relevant code, because otherwise Emacs might fail to link for no
> good reason.

Gnulib provides a stub definition for posix_spawn on Windows as well
(in fact, as of a few days ago, it provides a non-stub
implementation), so linking shouldn't fail. Also compiling the stub
implementation on Windows shouldn't hurt; if it's never called the
linker will likely remove the stub.
What error messages do you see on Windows when trying to build the branch?

> > > Btw, regarding use of posix_spawn, I'd expect a discussion before we
> > > make such a change.  AFAIU it is not a trivial decision, as
> > > posix_spawn has its down sides, and therefore is not necessarily the
> > > best API for running sub-processes on every supported platform, even
> > > if you consider only the Posix ones.  We should consider the
> > > advantages and disadvantages before we make the decision.
> >
> > Sure, I'm happy to have that discussion. I briefly reviewed the
> > posix_spawn implementation of GNU libc and Gnulib, and found that it
> > uses vfork/clone + execve like our hand-rolled code, so I wouldn't
> > expect any significant change. The primary advantage is to offload
> > complexity into a library that can properly deal with system-specific
> > issues and can improve over time. For example, on Linux, posix_spawn
> > can use clone instead of vfork.
>
> See Savannah bug #59093 for one subtle issue:
>
>   https://savannah.gnu.org/bugs/?59093
>
> Since Emacs also sets its stack limit in some cases, this could be
> directly relevant to us.  (But I didn't look into it close enough to
> tell whether it actually is relevant.)

I think that specific problem isn't relevant (we don't change the
stack size between fork and exec), but the fix to
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24869 (ironically
reported by me) is.
As indicated in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24869#8,
it might be better to solve the underlying problem in a different way
that doesn't involve changing resource limits (changing a
process-global setting is somewhat fishy anyway, especially with
modules). Essentially we just need to make sure to not add file
descriptors larger than FD_SETSIZE to an fd_set.

>
> > On Windows, posix_spawn could directly call CreateProcess (though
> > Gnulib doesn't implement that yet).
>
> FYI: there's a general problem with using Gnulib code on MS-Windows
> for anything in Emacs that involves file names.  Emacs on MS-Windows
> uses UTF-8 encoded file names, which allows us to support 'char *'
> strings as file names outside of the domain of the current system
> codepage.  This works by basically wrapping any API that accepts file
> names with our own code that converts file names to UTF-16, and then
> invokes Windows APIs which accept 'wchar_t *' "wide" strings.  Gnulib
> doesn't have this feature, it just calls Windows APIs assuming the
> 'char *' strings as file names will be correctly interpreted -- which
> doesn't work with UTF-8 encoded file names.
>
> Running subprocesses definitely manipulates file names, so the above
> issue is very relevant here.

Agreed. It would be really nice if Gnulib adopted the Emacs approach
(probably hidden behind some flag).