* MinGW build on master broken by Gnulib update @ 2024-09-05 5:43 Eli Zaretskii 2024-09-05 16:28 ` Paul Eggert 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2024-09-05 5:43 UTC (permalink / raw) To: Po Lu, Paul Eggert; +Cc: emacs-devel 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: MinGW build on master broken by Gnulib update 2024-09-05 5:43 MinGW build on master broken by Gnulib update Eli Zaretskii @ 2024-09-05 16:28 ` Paul Eggert 2024-09-05 18:02 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Paul Eggert @ 2024-09-05 16:28 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel, Po Lu [-- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: MinGW build on master broken by Gnulib update 2024-09-05 16:28 ` Paul Eggert @ 2024-09-05 18:02 ` Eli Zaretskii 2024-09-05 18:49 ` Paul Eggert 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2024-09-05 18:02 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel, luangruo > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: MinGW build on master broken by Gnulib update 2024-09-05 18:02 ` Eli Zaretskii @ 2024-09-05 18:49 ` Paul Eggert 2024-09-06 6:48 ` Eli Zaretskii 0 siblings, 1 reply; 6+ messages in thread From: Paul Eggert @ 2024-09-05 18:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel, luangruo 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: MinGW build on master broken by Gnulib update 2024-09-05 18:49 ` Paul Eggert @ 2024-09-06 6:48 ` Eli Zaretskii 2024-09-06 9:52 ` Bruno Haible 0 siblings, 1 reply; 6+ messages in thread From: Eli Zaretskii @ 2024-09-06 6:48 UTC (permalink / raw) To: Paul Eggert; +Cc: emacs-devel, bug-gnulib > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: MinGW build on master broken by Gnulib update 2024-09-06 6:48 ` Eli Zaretskii @ 2024-09-06 9:52 ` Bruno Haible 0 siblings, 0 replies; 6+ messages in thread From: Bruno Haible @ 2024-09-06 9:52 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Paul Eggert, bug-gnulib, emacs-devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-06 9:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-05 5:43 MinGW build on master broken by Gnulib update Eli Zaretskii 2024-09-05 16:28 ` Paul Eggert 2024-09-05 18:02 ` Eli Zaretskii 2024-09-05 18:49 ` Paul Eggert 2024-09-06 6:48 ` Eli Zaretskii 2024-09-06 9:52 ` Bruno Haible
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.