Re: [PATCH] regex: trim module dependencies
Hi Paul, > * modules/regex (Depends-on): Remove gettext-h and lock, > since the regex code should work OK without these modules, > and Emacs uses it that way. The removal of the gettext-h dependency is fine; regex_internal.h tests ENABLE_NLS and includes . But I object against the removal of the 'lock' dependency. This change transforms a module that is by default multithread-safe into a module that by default will crash in multithreaded situations. There is logic in regex_internal.h (lines 61..97): #elif defined GNULIB_LOCK && !defined USE_UNLOCKED_IO # include "glthread/lock.h" # define lock_define(name) # ... #elif defined GNULIB_PTHREAD && !defined USE_UNLOCKED_IO # include # define lock_define(name) pthread_mutex_t name; # ... #else # define lock_define(name) # ... #endif It implements the following (desired and perfectly correct) behaviour: - If the 'lock' module is present, use it to achieve MT-safety on all platforms. - Otherwise, if the 'pthread' module is present, use it to achieve MT-safety on POSIX-like platforms. - Otherwise, give up on MT-safety. Gnulib users should *not* need to know "regex is not MT-safe by default, therefore I need to include the 'lock' module". Rather, Gnulib users should get an MT-safe module by default, and if they don't like the complexities of 'lock' or 'pthread' modules, they can use the --avoid option IF THEY KNOW that their application is single-threaded. The coreutils maintainers should not have to check whether 'sort' and other multithreaded programs use regex. Likewise, the gettext maintainers should not have to check whether 'msgmerge' and libgettextpo (and possibly other multithreaded parts of gettext) use regex. And so on. That's easy for you to achieve in Emacs (assuming you know that Emacs uses regex only from a single thread): diff --git a/admin/merge-gnulib b/admin/merge-gnulib index 9a5ad54..f55908c 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -47,8 +47,9 @@ GNULIB_MODULES=' AVOIDED_MODULES=' close dup fchdir fstat + lock malloc-posix msvc-inval msvc-nothrow - openat-die opendir raise + openat-die opendir pthread raise save-cwd select setenv sigprocmask stat stdarg stdbool threadlib tzset unsetenv utime utime-h ' > Also remove memcmp, memmove, > and wctype, as these modules are obsolete and should not be > needed any more. It is not needed to avoid dependencies to obsolete modules. See the Gnulib documentation [1]: "Depends-on This field contains a newline separated list of the modules that are required for the proper working of this module. gnulib-tool includes each required module automatically, unless it is specified with option --avoid or it is marked as obsolete and the option --with-obsolete is not given." Maybe this sentence is too long to be understood, and I should reword it? Bruno [1] https://www.gnu.org/software/gnulib/manual/html_node/Module-description.html
Re: [PATCH] regex: trim module dependencies
Thanks for the detailed review, and you're right on all counts. I installed the attached to do the fixes you suggest. I'm still having trouble with the regex code under Emacs, and will try to follow up shortly. >From e09e64c7e0d3c9d0cca82cb80713f8aea195d493 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 1 Jul 2018 06:37:38 -0700 Subject: [PATCH] regex: revert most trimming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problems reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2018-07/msg1.html * modules/regex (Depends-on): Add lock, memcmp, memmove, and wctype back in. lock because regex users shouldnât need to know that regex needs locking, and the rest because gnulib-tool should ordinarily ignore them anyway. --- ChangeLog | 10 ++ modules/regex | 4 2 files changed, 14 insertions(+) diff --git a/ChangeLog b/ChangeLog index e25f5a7..4e9f441 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2018-07-01 Paul Eggert + + regex: revert most trimming + Problems reported by Bruno Haible in: + https://lists.gnu.org/r/bug-gnulib/2018-07/msg1.html + * modules/regex (Depends-on): Add lock, memcmp, memmove, + and wctype back in. lock because regex users shouldnât + need to know that regex needs locking, and the rest because + gnulib-tool should ordinarily ignore them anyway. + 2018-06-30 Paul Eggert regex: trim module dependencies diff --git a/modules/regex b/modules/regex index 8a6afcb..8863a8e 100644 --- a/modules/regex +++ b/modules/regex @@ -20,6 +20,9 @@ alloca-opt [test $ac_use_included_regex = yes] btowc [test $ac_use_included_regex = yes] builtin-expect [test $ac_use_included_regex = yes] intprops[test $ac_use_included_regex = yes] +lock [test "$ac_cv_gnu_library_2_1:$ac_use_included_regex" = no:yes] +memcmp [test $ac_use_included_regex = yes] +memmove [test $ac_use_included_regex = yes] mbrtowc [test $ac_use_included_regex = yes] mbsinit [test $ac_use_included_regex = yes] nl_langinfo [test $ac_use_included_regex = yes] @@ -28,6 +31,7 @@ stdint [test $ac_use_included_regex = yes] wchar [test $ac_use_included_regex = yes] wcrtomb [test $ac_use_included_regex = yes] wctype-h[test $ac_use_included_regex = yes] +wctype [test $ac_use_included_regex = yes] configure.ac: gl_REGEX -- 2.7.4
Re: avoid FP syntax-check failure due to bootstrap
Jim Meyering wrote: Subject: [PATCH] bootstrap: s/--option val/--option=val/ Thanks, that looks good to me. (I had run into the same problem but was hoping someone else would have time to look into it)
Re: gcc/g++-warning.spec: remove -Wswitch-enum?
Reuben Thomas wrote: I find them useful and switch them on, but given that others disagree, I like your suggestion as it makes things consistent, and less likely to confuse those who share my taste. OK, thanks, no further comment so I installed the attached. From f24fbe9d9297f57b632fbefd87770b4f39a611ef Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 1 Jul 2018 07:08:32 -0700 Subject: [PATCH] manywarnings: omit -Wswitch-default This should make things more consistent, as we already ignore -Wswitch-enum. Problem reported by Reuben Thomas; see: https://lists.gnu.org/r/bug-gnulib/2018-05/msg00179.html * build-aux/g++-warning.spec, build-aux/gcc-warning.spec: Add -Wswitch-default. * m4/manywarnings-c++.m4 (gl_MANYWARN_ALL_GCC_CXX_IMPL): * m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): Remove -Wswitch-default. --- ChangeLog | 10 ++ build-aux/g++-warning.spec | 1 + build-aux/gcc-warning.spec | 1 + m4/manywarnings-c++.m4 | 3 +-- m4/manywarnings.m4 | 3 +-- 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4e9f441..5f3ab00 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2018-07-01 Paul Eggert + manywarnings: omit -Wswitch-default + This should make things more consistent, as we already ignore + -Wswitch-enum. Problem reported by Reuben Thomas; see: + https://lists.gnu.org/r/bug-gnulib/2018-05/msg00179.html + * build-aux/g++-warning.spec, build-aux/gcc-warning.spec: + Add -Wswitch-default. + * m4/manywarnings-c++.m4 (gl_MANYWARN_ALL_GCC_CXX_IMPL): + * m4/manywarnings.m4 (gl_MANYWARN_ALL_GCC): + Remove -Wswitch-default. + regex: revert most trimming Problems reported by Bruno Haible in: https://lists.gnu.org/r/bug-gnulib/2018-07/msg1.html diff --git a/build-aux/g++-warning.spec b/build-aux/g++-warning.spec index ae3e866..99ef3c8 100644 --- a/build-aux/g++-warning.spec +++ b/build-aux/g++-warning.spec @@ -82,6 +82,7 @@ -Wstrict-prototypes c -Wstrict-selector-match objc and objc++ -Wsurprisingfortran +-Wswitch-default https://lists.gnu.org/r/bug-gnulib/2018-05/msg00179.html -Wswitch-enumFIXME maybe? borderline. some will want this -Wsynth deprecated -Wtabs fortran diff --git a/build-aux/gcc-warning.spec b/build-aux/gcc-warning.spec index c47298e..8c0a3dc 100644 --- a/build-aux/gcc-warning.spec +++ b/build-aux/gcc-warning.spec @@ -121,6 +121,7 @@ -Wsubobject-linkage c++ and objc++ -Wsuggest-override c++ and objc++ -Wsurprisingfortran +-Wswitch-default https://lists.gnu.org/r/bug-gnulib/2018-05/msg00179.html -Wswitch-enumFIXME maybe? borderline. some will want this -Wsynth deprecated -Wtabs fortran diff --git a/m4/manywarnings-c++.m4 b/m4/manywarnings-c++.m4 index 3a4797d..28d9aa0 100644 --- a/m4/manywarnings-c++.m4 +++ b/m4/manywarnings-c++.m4 @@ -1,4 +1,4 @@ -# manywarnings-c++.m4 serial 1 +# manywarnings-c++.m4 serial 2 dnl Copyright (C) 2008-2018 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -180,7 +180,6 @@ AC_DEFUN([gl_MANYWARN_ALL_GCC_CXX_IMPL], -Wsuggest-override \ -Wswitch \ -Wswitch-bool \ --Wswitch-default \ -Wsync-nand \ -Wsystem-headers \ -Wtrampolines \ diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4 index 925c40e..516c587 100644 --- a/m4/manywarnings.m4 +++ b/m4/manywarnings.m4 @@ -1,4 +1,4 @@ -# manywarnings.m4 serial 15 +# manywarnings.m4 serial 16 dnl Copyright (C) 2008-2018 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -239,7 +239,6 @@ m4_defun([gl_MANYWARN_ALL_GCC(C)], -Wsuggest-final-types \ -Wswitch \ -Wswitch-bool \ --Wswitch-default \ -Wswitch-unreachable \ -Wsync-nand \ -Wsystem-headers \ -- 2.7.4
Re: avoid FP syntax-check failure due to bootstrap
On Sun, Jul 1, 2018 at 6:42 AM, Paul Eggert wrote: > Jim Meyering wrote: > >> Subject: [PATCH] bootstrap: s/--option val/--option=val/ > > Thanks, that looks good to me. (I had run into the same problem but was > hoping someone else would have time to look into it) Thanks. Pushed.
[PATCH] getloadavg: Don't redefine WINDOWS32.
* lib/getloadavg.c: Only define WINDOWS32 if it's not already defined. --- lib/getloadavg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/getloadavg.c b/lib/getloadavg.c index 435d10a..578316e 100644 --- a/lib/getloadavg.c +++ b/lib/getloadavg.c @@ -97,7 +97,7 @@ # include "intprops.h" -# if defined _WIN32 && ! defined __CYGWIN__ +# if defined _WIN32 && ! defined __CYGWIN__ && ! defined WINDOWS32 # define WINDOWS32 # endif -- 2.9.2
Re: [PATCH] getloadavg: Don't redefine WINDOWS32.
Thanks. I installed that.
[PATCH] wchar: fix bug when checking for ‘inline’
I discovered this when looking into using the regex module
with Emacs.
* m4/wchar_h.m4 (gl_WCHAR_H_INLINE_OK): Fix bug introduced in
2016-08-17T23:09:38Z!sk...@iskunk.org; the code compiled
conftest1.c and conftest2.c but these files were not created.
As far as I can see, this check never worked and nobody reported
it until now, which is a bit worrisome.
---
ChangeLog | 11 +++
m4/wchar_h.m4 | 10 ++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f556f04fb..a220b9c41 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2018-07-01 Paul Eggert
+
+ wchar: fix bug when checking for ‘inline’
+ I discovered this when looking into using the regex module
+ with Emacs.
+ * m4/wchar_h.m4 (gl_WCHAR_H_INLINE_OK): Fix bug introduced in
+ 2016-08-17T23:09:38Z!sk...@iskunk.org; the code compiled
+ conftest1.c and conftest2.c but these files were not created.
+ As far as I can see, this check never worked and nobody reported
+ it until now, which is a bit worrisome.
+
2018-06-30 Jim Meyering
bootstrap: s/--option val/--option=val/
diff --git a/m4/wchar_h.m4 b/m4/wchar_h.m4
index 416e0a1f8..a062ca981 100644
--- a/m4/wchar_h.m4
+++ b/m4/wchar_h.m4
@@ -7,7 +7,7 @@ dnl with or without modifications, as long as this notice is
preserved.
dnl Written by Eric Blake.
-# wchar_h.m4 serial 42
+# wchar_h.m4 serial 43
AC_DEFUN([gl_WCHAR_H],
[
@@ -90,7 +90,8 @@ int main () { return zero(); }
dnl that the object file has the latter name from the start.
save_ac_compile="$ac_compile"
ac_compile=`echo "$save_ac_compile" | sed s/conftest/conftest1/`
- if AC_TRY_EVAL([ac_compile]); then
+ if echo '#include "conftest.c"' >conftest1.c &&
+AC_TRY_EVAL([ac_compile]); then
AC_LANG_CONFTEST([
AC_LANG_SOURCE([[#define wcstod renamed_wcstod
/* Tru64 with Desktop Toolkit C has a bug: must be included before
@@ -105,7 +106,8 @@ int zero (void) { return 0; }
]])])
dnl See note above about renaming object files.
ac_compile=`echo "$save_ac_compile" | sed s/conftest/conftest2/`
- if AC_TRY_EVAL([ac_compile]); then
+ if echo '#include "conftest.c"' >conftest2.c &&
+ AC_TRY_EVAL([ac_compile]); then
if $CC -o conftest$ac_exeext $CFLAGS $LDFLAGS conftest1.$ac_objext
conftest2.$ac_objext $LIBS >&AS_MESSAGE_LOG_FD 2>&1; then
:
else
@@ -114,7 +116,7 @@ int zero (void) { return 0; }
fi
fi
ac_compile="$save_ac_compile"
- rm -f conftest1.$ac_objext conftest2.$ac_objext conftest$ac_exeext
+ rm -f conftest[12].c conftest[12].$ac_objext conftest$ac_exeext
])
if test $gl_cv_header_wchar_h_correct_inline = no; then
AC_MSG_ERROR([ cannot be used with this compiler ($CC $CFLAGS
$CPPFLAGS).
--
2.17.1
