bug#45198: 28.0.50; Sandbox mode

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 29 Dec 2020 17:05:33 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Bastien <bzg@gnu.org>, 
> 45198@debbugs.gnu.org, 
>       João Távora <joaotavora@gmail.com>
> 
> Am Di., 29. Dez. 2020 um 16:44 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
> >
> > > It would be great if someone could test whether the Windows build
> > > still works after that, thanks!
> >
> > I only skimmed the changes, but I'm already quite sure they will break
> > the Windows build.
> 
> Why? Everything should be behind ifdefs/conditional compilation,
> otherwise compilation on macOS would also break.

Because some/many Gnulib replacements are incompatible with how the
w32 port of Emacs works.  In particular, functions which operate on
file names don't support UTF-8 encoded file names; Gnulib's 'stat' and
'fstat' don't support security-related features from which we glean
owner and group of files; network-related functions need special
handling in Emacs to support subprocess functionality; etc.

> >   . Gnulib modules pulled to support seccomp should be disabled in the
> >     MS-Windows build, by suitable changes to nt/mingw-cfg.site and/or
> >     nt/gnulib-cfg.mk; and
> 
> This shouldn't be necessary, as Gnulib is compatible with Windows (I
> guess, since we use it elsewhere)

Not when Emacs is concerned, see above.  We use only small parts of
Gnulib on Windows, for those reasons.  And we don't use any Gnulib
functions that accept file names.

> (That's also what gnulib-cfg.mk says: "In general, do NOT omit modules
> that don't need to be omitted, to minimize the differences from
> upstream gnulib.mk and thus make the maintenance easier.")

I know; I wrote that.  However, this talks about code that will be
actually _used_ in the Windows build, whereas here we are talking
about "ballast": the related code will not be used at all.  So it
makes sense not to make our job harder by adding unnecessary code,
which then will need to be audited for compatibility.  Your Gnulib
import adds quite a lot of modules, which makes the audit a large job.

> >   . header files used to support seccomp code should be #ifdef'ed by
> >     HAVE_LINUX_SECCOMP_H or similar, and likewise with any code needed
> >     for seccomp and unnecessary otherwise.
> 
> That should already be the case.

It isn't, at least not in general.  Just one example:

  diff --git a/src/emacs.c b/src/emacs.c
  index 2a32083..a044535 100644
  --- a/src/emacs.c
  +++ b/src/emacs.c
  @@ -33,6 +33,8 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
   #include "lisp.h"
   #include "sysstdio.h"

  +#include <read-file.h>  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

This header is included unconditionally, although it shouldn't be
needed on Windows.

> Turning the question around: is building the branch on Windows
> actually broken? If so, what are the error messages?

I didn't have time to build it, and probably won't have enough time in
the following days, sorry.  I have the co-maintainer job to do, and
there's a pretest of Emacs 27.2 under way on top of that.  So I cannot
show the error messages.  Others are encouraged to try the branch and
report the actual problems; I just wanted to give a heads-up, in the
hope that it will help adapting and merging the branch sooner.

> > Btw, I wonder why you needed to import the read-file module from
> > Gnulib -- does it provide any features that we couldn't implement on
> > our own?  I'm asking because that module caused you to pull in quite a
> > few dependency modules from Gnulib, and I'm asking myself whether that
> > is really justified.
> 
> We could implement it ourselves, if we wanted, and in an earlier
> version of the code I did that. But it's easier and less error-prone
> to reuse an existing library.

I question the wisdom of that, FWIW.  Every unnecessary dependency on
Gnulib is IMO an addition to the maintenance burden.  It also makes
the job of adapting the Windows build harder, because for example
Gnulib's fopen cannot be used in Emacs on Windows, whereas we have
emacs_fopen for the same purpose, which doesn't have that problem.