MinGW build on master broken by Gnulib update

MinGW build on master broken by Gnulib update

[Eli Zaretskii]

The last update from Gnulib broke the MinGW build on master:

  sig2str.c:321:1: warning: no previous prototype for 'str2sig' [-Wmissing-prototypes]
    321 | str2sig (char const *signame, int *signum)
	| ^~~~~~~
  sig2str.c:332:1: warning: no previous prototype for 'sig2str' [-Wmissing-prototypes]
    332 | sig2str (int signum, char *signame)
	| ^~~~~~~
  [...]
  process.c: In function 'abbr_to_signal':
  process.c:7265:9: warning: implicit declaration of function 'str2sig' [-Wimplicit-function-declaration]
   7265 |  return str2sig (sigbuf, &signo) == 0 ? signo : -1;
	|         ^~~~~~~
  process.c:7265:9: warning: nested extern declaration of 'str2sig' [-Wnested-externs]
  process.c: In function 'Fsignal_names':
  process.c:8539:13: error: 'SIG2STR_MAX' undeclared (first use in this function)
   8539 |   char name[SIG2STR_MAX];
	|             ^~~~~~~~~~~
  process.c:8539:13: note: each undeclared identifier is reported only once for each function it appears in
    CC       intervals.o
  process.c:8544:12: warning: implicit declaration of function 'sig2str' [-Wimplicit-function-declaration]
   8544 |       if (!sig2str (i, name))
	|            ^~~~~~~
  process.c:8544:12: warning: nested extern declaration of 'sig2str' [-Wnested-externs]
  process.c:8539:8: warning: unused variable 'name' [-Wunused-variable]
   8539 |   char name[SIG2STR_MAX];
	|        ^~~~
    CC       textprop.o
  Makefile:457: recipe for target `process.o' failed
  make[2]: *** [process.o] Error 1

Gnulib moved the prototypes of sig2str and str2sig from sig2str.h to
Gnulib's signal.h, evidently assuming that a build which uses the
Gnulib sig2str will also use the Gnulib signal.h header, but that
assumption is false for the MinGW build of Emacs, which omits
lib/signal.h (because it clashes with some w32 code in Emacs).

I fixed that temporarily by modifying lib/sig2str.h to include the
missing stuff for MinGW, but this is really a Gnulib issue, and should
be fixed in Gnulib, IMO.

Re: MinGW build on master broken by Gnulib update

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On 2024-09-04 22:43, Eli Zaretskii wrote:

> Gnulib moved the prototypes of sig2str and str2sig from sig2str.h to
> Gnulib's signal.h, evidently assuming that a build which uses the
> Gnulib sig2str will also use the Gnulib signal.h header, but that
> assumption is false for the MinGW build of Emacs, which omits
> lib/signal.h (because it clashes with some w32 code in Emacs).
> 
> I fixed that temporarily by modifying lib/sig2str.h to include the
> missing stuff for MinGW, but this is really a Gnulib issue, and should
> be fixed in Gnulib, IMO.

The change to Gnulib was to align with POSIX.1-2024, which declares 
sig2str and str2sig in <signal.h>; see:

https://pubs.opengroup.org/onlinepubs/9799919799/functions/sig2str.html

We don't want sig2str callers to depart from the POSIX API, any more 
than we'd want to require (say) fdopen callers to include a 
Gnulib-specific fdopen.h rather than including <stdio.h>.

Instead, how about adjusting Emacs's MinGW shims to supply the missing 
declarations? Something like the attached patch, say. (I don't use MinGW 
so can't easily test this.)

[-- Attachment #2: 0001-Move-MinGW-sig2str-workaround-into-ms-w32.h.patch --]
[-- Type: text/x-patch, Size: 2001 bytes --]

From 0c17a34cdfbab86b4b44ea2b123f6d072e00ab79 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 5 Sep 2024 09:22:41 -0700
Subject: [PATCH] Move MinGW sig2str workaround into ms-w32.h

* lib/sig2str.h: Go back to syncing exactly with Gnulib.
* nt/inc/ms-w32.h (SIG2STR_MAX): New macro.
(sig2str, str2sig): New decls.
---
 lib/sig2str.h   | 20 --------------------
 nt/inc/ms-w32.h |  7 +++++++
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/lib/sig2str.h b/lib/sig2str.h
index 62b6d628f12..1abdb140e5a 100644
--- a/lib/sig2str.h
+++ b/lib/sig2str.h
@@ -17,28 +17,8 @@
 
 /* Written by Paul Eggert.  */
 
-/* This file uses HAVE_* macros.  */
-# if !_GL_CONFIG_H_INCLUDED
-#  error "Please include config.h first."
-# endif
-
 #include <signal.h>
 
-/* Maximum size of a signal name returned by sig2str(), including the
-   terminating NUL byte.  */
-#ifndef SIG2STR_MAX
-/* The longest one: "RTMAX", then "+" or "-", then up to 10 digits, then NUL.
-   Add + 2 as a reserve for the future.  */
-# define SIG2STR_MAX (5 + 1 + 10 + 1 + 2)
-#endif
-
-#ifdef __MINGW32__
-int sig2str (int, char *);
-#endif
-#ifdef __MINGW32__
-int str2sig (char const *, int *);
-#endif
-
 /* An upper bound on signal numbers allowed by the system.  */
 
 #if defined _sys_nsig
diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h
index 7212e4d2984..55273cbce79 100644
--- a/nt/inc/ms-w32.h
+++ b/nt/inc/ms-w32.h
@@ -427,6 +427,13 @@ #define SIG_BLOCK       1
 #define SIG_SETMASK     2
 #define SIG_UNBLOCK     3
 
+/* Maximum size of a signal name returned by sig2str(), including the
+   terminating NUL byte.  The longest one: "RTMAX", then "+" or "-",
+   then up to 10 digits, then NUL.  Add + 2 as a reserve for the future.  */
+#define SIG2STR_MAX (5 + 1 + 10 + 1 + 2)
+
+extern int sig2str (int, char *);
+extern int str2sig (char const *, int *);
 extern int sigemptyset (sigset_t *);
 extern int sigaddset (sigset_t *, int);
 extern int sigfillset (sigset_t *);
-- 
2.43.0

Re: MinGW build on master broken by Gnulib update

[Eli Zaretskii]

> Date: Thu, 5 Sep 2024 09:28:15 -0700
> Cc: emacs-devel@gnu.org, Po Lu <luangruo@yahoo.com>
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2024-09-04 22:43, Eli Zaretskii wrote:
> 
> > Gnulib moved the prototypes of sig2str and str2sig from sig2str.h to
> > Gnulib's signal.h, evidently assuming that a build which uses the
> > Gnulib sig2str will also use the Gnulib signal.h header, but that
> > assumption is false for the MinGW build of Emacs, which omits
> > lib/signal.h (because it clashes with some w32 code in Emacs).
> > 
> > I fixed that temporarily by modifying lib/sig2str.h to include the
> > missing stuff for MinGW, but this is really a Gnulib issue, and should
> > be fixed in Gnulib, IMO.
> 
> The change to Gnulib was to align with POSIX.1-2024, which declares 
> sig2str and str2sig in <signal.h>; see:
> 
> https://pubs.opengroup.org/onlinepubs/9799919799/functions/sig2str.html
> 
> We don't want sig2str callers to depart from the POSIX API, any more 
> than we'd want to require (say) fdopen callers to include a 
> Gnulib-specific fdopen.h rather than including <stdio.h>.

I'm talking about what's in the sig2str.h header.  The above URL says
nothing about it.  Why cannot Gnulib arrange for that header to
declare these functions and that macro, if signal.h didn't?

> Instead, how about adjusting Emacs's MinGW shims to supply the missing 
> declarations? Something like the attached patch, say. (I don't use MinGW 
> so can't easily test this.)

This might solve the Emacs problem (and is not very clean even in that
case), but will not help other projects that use Gnulib.

Once again, assuming every program that needs sig2str should also use
the Gnulib signal.h header is IMO wrong.  The Gnulib signal.h pulls in
too much stuff, including pthreads (which are a major source of
trouble on MS-Windows), and a lot of other signal-related stuff which
will get in the way of any project that wants decent emulation of
Posix signals that are not available on Windows (and thus require
custom code to emulate, similar to what Emacs does with SIGKILL and
SIGPROF).  By forcing projects to use the Gnulib signal.h header when
all they need is sig2str, you are making porting of GNU software to
Windows much harder, in an area where it is hard enough already.  I
urge you to reconsider and find a way of allowing projects to have
sig2str without also using the Gnulib signal.h.  I don't think the
changes in Gnulib should be too complicated (there are already ways of
testing in Gnulib headers whether a given function is replaced by
Gnulib, and the declarations are needed only in that case), so I hope
the Gnulib will agree to such changes and not make porting programs to
Windows any harder in this area.

TIA

Re: MinGW build on master broken by Gnulib update

[Paul Eggert]

On 2024-09-05 11:02, Eli Zaretskii wrote:

> I'm talking about what's in the sig2str.h header.  The above URL says
> nothing about it.  Why cannot Gnulib arrange for that header to
> declare these functions and that macro, if signal.h didn't?

Because then callers might easily make the mistake of thinking that they 
should include <sig2str.h> to declare sig2str. Callers shouldn't do 
that. They are supposed to include <signal.h> to declare sig2str. 
sig2str.h is for something else: it's for defining SIGNUM_BOUND, and it 
includes signal.h only to be backwards-compatible with older Gnulib 
versions.

In hindsight I should have given sig2str.h a different name. I wrote 
sig2str.h decades before POSIX standardized sig2str, so I have some 
excuse for this infelicity. We can change sig2str.h's name now, if the 
name is so confusing that it's worth the hassle of changing.


>> Instead, how about adjusting Emacs's MinGW shims to supply the missing
>> declarations? Something like the attached patch, say. (I don't use MinGW
>> so can't easily test this.)
> 
> This might solve the Emacs problem (and is not very clean even in that
> case), but will not help other projects that use Gnulib.

That's not something we need to worry about.

Gnulib's sig2str module depends on its signal-h module, so other 
projects that use sig2str get the POSIX-compatible API with no fuss. The 
MSVC port of Emacs is a special case, because it deliberately suppresses 
the Gnulib replacement for signal.h and supplies its own declarations 
for signal-related functions like sigaddset, sigprocmask and sigaction. 
sig2str is simply another function to add to that longstanding list.

I doubt whether any other project does anything similar. If there is 
one, it will have its own special reasons and will need to accommodate 
them. If that ever happens, we can then worry about whether its 
accommodations can be shared via Gnulib with Emacs's; this will surely 
affect many functions other than sig2str. In the meantime, as you 
mentioned the Emacs patch I proposed is good enough to solve this 
particular problem.


> Once again, assuming every program that needs sig2str should also use
> the Gnulib signal.h header is IMO wrong.

The issue is not just sig2str; it's also sigaction and many other 
functions. And the issue is not limited to signal.h; it's also present 
in stdio.h and many other standard headers. Gnulib's design principle is 
to make the API resemble the GNU API as best it can. Of course that 
principle could be changed if needed, but any such change should be 
discussed on bug-gnulib@gnu.org, as it's a much bigger issue than just 
sig2str and Emacs.

Re: MinGW build on master broken by Gnulib update

[Eli Zaretskii]

> Date: Thu, 5 Sep 2024 11:49:19 -0700
> Cc: emacs-devel@gnu.org, luangruo@yahoo.com
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2024-09-05 11:02, Eli Zaretskii wrote:
> 
> > I'm talking about what's in the sig2str.h header.  The above URL says
> > nothing about it.  Why cannot Gnulib arrange for that header to
> > declare these functions and that macro, if signal.h didn't?
> 
> Because then callers might easily make the mistake of thinking that they 
> should include <sig2str.h> to declare sig2str. Callers shouldn't do 
> that. They are supposed to include <signal.h> to declare sig2str. 
> sig2str.h is for something else: it's for defining SIGNUM_BOUND, and it 
> includes signal.h only to be backwards-compatible with older Gnulib 
> versions.

Then document this and be done.  I don't see how is this a big
problem.

> >> Instead, how about adjusting Emacs's MinGW shims to supply the missing
> >> declarations? Something like the attached patch, say. (I don't use MinGW
> >> so can't easily test this.)
> > 
> > This might solve the Emacs problem (and is not very clean even in that
> > case), but will not help other projects that use Gnulib.
> 
> That's not something we need to worry about.

That is strange to hear, since Gnulib is supposed to care about all
GNU projects, not just about Emacs.

> Gnulib's sig2str module depends on its signal-h module, so other 
> projects that use sig2str get the POSIX-compatible API with no fuss. The 
> MSVC port of Emacs is a special case

There's no MSVC port of Emacs, not for many years.

> I doubt whether any other project does anything similar. If there is 
> one, it will have its own special reasons and will need to accommodate 
> them. If that ever happens, we can then worry about whether its 
> accommodations can be shared via Gnulib with Emacs's; this will surely 
> affect many functions other than sig2str. In the meantime, as you 
> mentioned the Emacs patch I proposed is good enough to solve this 
> particular problem.
> 
> 
> > Once again, assuming every program that needs sig2str should also use
> > the Gnulib signal.h header is IMO wrong.
> 
> The issue is not just sig2str; it's also sigaction and many other 
> functions. And the issue is not limited to signal.h; it's also present 
> in stdio.h and many other standard headers. Gnulib's design principle is 
> to make the API resemble the GNU API as best it can. Of course that 
> principle could be changed if needed, but any such change should be 
> discussed on bug-gnulib@gnu.org, as it's a much bigger issue than just 
> sig2str and Emacs.

I have neither time nor energy for this Gnulib politics, so I went
ahead and installed a workaround for this.  (If I had more free time,
which I don't, I'd stopped using the Gnulib sig2str.c altogether to
avoid the issue in the first place, but things being as they are, I
installed an easier cop-out.)  If the Gnulib folks (CC'ed) are ready
to reconsider, I will be happy to remove the workaround.

Re: MinGW build on master broken by Gnulib update

[Bruno Haible]

Replying to the thread that started at
<https://lists.gnu.org/archive/html/emacs-devel/2024-09/msg00131.html>.

I agree with everything that Paul wrote in this thread, except for the
typo where we wrote "MSVC port of Emacs" but meant "mingw port of Emacs".

Eli Zaretskii replied to Paul Eggert:
> > ... Callers shouldn't do 
> > that. They are supposed to include <signal.h> to declare sig2str. 
> > sig2str.h is for something else: it's for defining SIGNUM_BOUND, and it 
> > includes signal.h only to be backwards-compatible with older Gnulib 
> > versions.
> 
> Then document this and be done.  I don't see how is this a big
> problem.

We already document this, in the module description of the 'sig2str' module:

  Include:
  <signal.h>
  "sig2str.h" /* for SIGNUM_BOUND */

It is a bit terse, but it means:
  - Callers should use '#include <signal.h>' for the general part.
  - Callers should additionally use '#include "sig2str.h"' if they need the
    SIGNUM_BOUND macro.

> > >> Instead, how about adjusting Emacs's MinGW shims to supply the missing
> > >> declarations? Something like the attached patch, say. (I don't use MinGW
> > >> so can't easily test this.)
> > > 
> > > This might solve the Emacs problem (and is not very clean even in that
> > > case), but will not help other projects that use Gnulib.
> > 
> > That's not something we need to worry about.
> 
> That is strange to hear, since Gnulib is supposed to care about all
> GNU projects, not just about Emacs.

Gnulib cares about all GNU packages. Emacs is one of them, which is why
we are discussing here. You are arguing that there are or might be other
GNU packages that have the same problem as Emacs. This is not the case.
Emacs uses the --avoid option far more extensively than other packages
that use Gnulib. And when a packages uses the --avoid option, the documentation
<https://www.gnu.org/software/gnulib/manual/html_node/Initial-import.html>
says:

  "If you want to cut a dependency, i.e., not add a module although one
   of your requested modules depends on it, you may use the option
   ‘--avoid=module’ to do so. Multiple uses of this option are possible.
   Of course, you will then need to implement the same interface as the
   removed module."

The patch that Paul suggested in
<https://lists.gnu.org/archive/html/emacs-devel/2024-09/msg00169.html>
is exactly the right thing: Since you decided that you don't want the
signal-h module, but sig2str needs this module for providing the declaration
of the two functions, you need to provide the declaration of these two
functions elsewhere.

> I have neither time nor energy for this Gnulib politics, so I went
> ahead and installed a workaround for this.  (If I had more free time,
> which I don't, I'd stopped using the Gnulib sig2str.c altogether to
> avoid the issue in the first place, but things being as they are, I
> installed an easier cop-out.)  If the Gnulib folks (CC'ed) are ready
> to reconsider, I will be happy to remove the workaround.

Adapting Gnulib to one's package can be done in multiple ways:
  - through the --avoid option for selected modules,
  - through the --local-dir option and *.diff files, which is
    roughly equivalent to applying patches after running gnulib-tool,
  - by upstreaming specific requirements to Gnulib.

Among these possibilities, use of the --local-dir option has the highest
maintenance cost, especially for a file such as signal.in.h. It would
break several times a year. Patching a rarely-modified file such as
sig2str.h, like you did, is going to work for some time, but will break
when on the Gnulib side we decide to add more declarations to this .h file.

In other words, adapting Gnulib to one's package is an art; it's a series
of continuing compromises with maintainability in mind. Paul Eggert masters
this art perfectly. You can 100% trust him in these matters.

Bruno