Messed up gl_COMPILER_PREPARE_CHECK_DECL

Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Mats Erik Andersson]

Hello there,

running autoreconf on OpenIndiana and on Debian GNU/Linux 8.8,
with the intent of rebuilding Inetutils, shatters with the message:

  error: AC_REQUIRE(gl_COMPILER_PREPARE_CHECK_DECL):
	  cannot be used outside of an AC_DEFUN'd macro

Bruno Haible created this new macro on January 4th,
and Simon Josefsson was able to recreate Inetutils
on January 1st, so please check the text of this
new feature for any incompleteness or incompatibilities.

We are working on release candidates and need valid Gnulib!

Best regards,

  Mats Erik Andersson

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Hi Mats,

> running autoreconf on OpenIndiana and on Debian GNU/Linux 8.8,
> with the intent of rebuilding Inetutils, shatters with the message:
> 
>   error: AC_REQUIRE(gl_COMPILER_PREPARE_CHECK_DECL):
> 	  cannot be used outside of an AC_DEFUN'd macro

To let me reproduce it, please
  - tell your autoconf version,
  - tell your automake version,
  - tell your m4 version,
  - which is the source you're using? inetutils master branch?

Bruno

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Mats Erik Andersson]

Bootstrapping, as well as repetition of autoreconf, with
Inetutils master at HEAD fails with both of

  OpenIndiana: autoconf-2.69, automake-1.11.6, m4-1.4.16

  Debian 8.8:  autoconf-2.69, automake-1.14.1, m4-1.4.17

  Gnulib:      master HEAD

> >   error: AC_REQUIRE(gl_COMPILER_PREPARE_CHECK_DECL):
> > 	  cannot be used outside of an AC_DEFUN'd macro

When changing to commit c5c965^ of Gnulib, autoreconf is
successful on both systems, allowing and leading to
complete builds of Inetutils.

Observe that the backstep is to the commit just before
the modification of m4/00gnulib.m4 which introduced
gl_COMPILER_PREPARE_CHECK_DECL.

Simon Josefsson produced a release candidate with an even
earlier gnulib snapshot than I can use to reach the success
as given above.

Best regards,
  Mats E A

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Paul Eggert]

On 1/10/20 12:14 PM, Bruno Haible wrote:
> To let me reproduce it, please
>    - tell your autoconf version,
>    - tell your automake version,
>    - tell your m4 version,
>    - which is the source you're using? inetutils master branch?

I can reproduce the problem when building GNU Emacs on Fedora 31 
(Autoconf 2.69, Automake 1.16.1, m4 1.4.18) by running the following recipe:

mkdir d
cd d
git clone https://git.savannah.gnu.org/git/emacs.git
cd emacs
admin/merge-gnulib
./autogen.sh

This used the current versions of Emacs and Gnulib, which are Emacs 
commit 17cfd708575c351d030f8b05c5921d1867028d79 and Gnulib commit 
f5eb8cea72469348b423ae22068eeb9e1399011b. The last command attempts and 
fails to upgrade Emacs's copies of Gnulib to the current version of 
Gnulib, and its output ends like this:

Running 'autoreconf -fi -I m4' ...
configure.ac:1564: error: AC_REQUIRE(gl_COMPILER_PREPARE_CHECK_DECL): 
cannot be used outside of an AC_DEFUN'd macro
configure.ac:1564: the top level
autom4te: /usr/bin/m4 failed with exit status: 1
configure.ac:1564: error: AC_REQUIRE(gl_COMPILER_PREPARE_CHECK_DECL): 
cannot be used outside of an AC_DEFUN'd macro
configure.ac:1564: the top level
autom4te: /usr/bin/m4 failed with exit status: 1
autoreconf: /usr/bin/autoconf failed with exit status: 1

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Paul Eggert wrote:
> I can reproduce the problem when building GNU Emacs on Fedora 31 
> (Autoconf 2.69, Automake 1.16.1, m4 1.4.18)

Good, that means the problem does not come from an older version of Automake.

> by running the following recipe:
> 
> mkdir d
> cd d
> git clone https://git.savannah.gnu.org/git/emacs.git
> cd emacs
> admin/merge-gnulib
> ./autogen.sh

Thanks for the simple-to-reproduce recipe. I've pushed this fix:


2020-01-10  Bruno Haible  <bruno@clisp.org>

	Fix major regression from 2020-01-04.
	Reported by Mats Erik Andersson <mats.andersson@gisladisker.se> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00072.html>.
	* m4/00gnulib.m4 (gl_COMPILER_PREPARE_CHECK_DECL): Define through
	AC_DEFUN_ONCE.
	(AC_CHECK_DECL): Invoke, not require, it.

diff --git a/m4/00gnulib.m4 b/m4/00gnulib.m4
index 77503ea..58bc4ef 100644
--- a/m4/00gnulib.m4
+++ b/m4/00gnulib.m4
@@ -1,4 +1,4 @@
-# 00gnulib.m4 serial 4
+# 00gnulib.m4 serial 5
 dnl Copyright (C) 2009-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -62,7 +62,7 @@ barfbarf
        [gl_cv_compiler_clang=no])
     ])
 ])
-AC_DEFUN([gl_COMPILER_PREPARE_CHECK_DECL],
+AC_DEFUN_ONCE([gl_COMPILER_PREPARE_CHECK_DECL],
 [
   AC_REQUIRE([AC_PROG_CC])
   AC_REQUIRE([gl_COMPILER_CLANG])
@@ -95,10 +95,11 @@ m4_define([_AC_CHECK_DECL_BODY],
 m4_defn([_AC_CHECK_DECL_BODY])[  ac_compile="$ac_save_ac_compile"
 ])
   ])
-dnl Redefine AC_CHECK_DECL so that it starts with
-dnl AC_REQUIRE([gl_COMPILER_PREPARE_CHECK_DECL]).
+dnl Redefine AC_CHECK_DECL so that it starts with an invocation of
+dnl gl_COMPILER_PREPARE_CHECK_DECL.
 m4_define([AC_CHECK_DECL],
-  [AC_REQUIRE([gl_COMPILER_PREPARE_CHECK_DECL])]m4_defn([AC_CHECK_DECL]))
+  [gl_COMPILER_PREPARE_CHECK_DECL dnl
+]m4_defn([AC_CHECK_DECL]))
 
 # gl_00GNULIB
 # -----------

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Paul Eggert]

On 1/10/20 2:47 PM, Bruno Haible wrote:
> Thanks for the simple-to-reproduce recipe. I've pushed this fix:

Unfortunately even with that fix I still have trouble building Emacs. 
That is, with this recipe:

mkdir d
cd d
git clone https://git.savannah.gnu.org/git/emacs.git
cd emacs
admin/merge-gnulib
./autogen.sh
./configure
make

(same recipe as before, but now with "configure; make" at the end) the 
build now fails this way:

   CC       strtoimax.o
strtoimax.c:44:15: error: redundant redeclaration of ‘strtoll’ 
[-Werror=redundant-decls]
    44 | long long int strtoll (char const *, char **, int);
       |               ^~~~~~~
In file included from ./stdlib.h:36,
                  from strtoimax.c:26:
/usr/include/stdlib.h:200:22: note: previous declaration of ‘strtoll’ 
was here
   200 | extern long long int strtoll (const char *__restrict __nptr,
       |                      ^~~~~~~
make[1]: *** [Makefile:95: strtoimax.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory '/home/eggert/src/gnu/emacs/static-checking/lib'
make: *** [Makefile:411: lib] Error 2


The above diagnostic comes because src/config.h contains the line 
"#define HAVE_DECL_STRTOLL 0" which is incorrect for this platform. And 
that incorrect line comes because ac_compile_for_check_decl is used 
before it is set. And this occurs because Emacs's configure.ac's first 
use of AC_CHECK_DECL is executed only on alpha platforms (which my 
platform is not), which means the initialization of 
ac_compile_for_check_decl is skipped.

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Hi Paul,

> that incorrect line comes because ac_compile_for_check_decl is used 
> before it is set. And this occurs because Emacs's configure.ac's first 
> use of AC_CHECK_DECL is executed only on alpha platforms (which my 
> platform is not), which means the initialization of 
> ac_compile_for_check_decl is skipped.

The Emacs configure.ac is not written in a robust way. It contains
'case' and 'if'/'test' statements which conditionally execute the Autoconf
macros

  AC_MSG_CHECKING
  AC_MSG_ERROR
  AC_MSG_WARN
  AC_MSG_RESULT
  AC_CACHE_CHECK
  AC_LINK_IFELSE
  AC_LANG_PROGRAM
  AC_PATH_PROG
  AC_CHECK_DECL
  AC_DEFINE
  AC_CHECK_HEADER
  AC_CHECK_PROG
  AC_CHECK_FUNCS
  AC_COMPILE_IFELSE
  AC_CHECK_LIB
  AC_TRY_LINK
  AC_PREPROC_IFELSE
  AC_CONFIG_FILES
  AC_MSG_NOTICE

If any of these macros, in its implementation, performs an AC_REQUIRE, you
may get a problem, as described in [1].

Do we have a statement in the Autoconf documentation that none of the built-in
macros does an AC_REQUIRE? I don't think so. Therefore I would suggest that
the particular 'case' and 'if'/'test' statements - or even the entire main body
of the configure.ac, from line 130 to line 5588 - gets wrapped in an AC_DEFUN
that gets invoked once.

The other alternative is to state formally that none of these AC_* macros do
an AC_REQUIRE. But then it's hard to find a proper place for the
gl_COMPILER_PREPARE_CHECK_DECL invocation (without adding extra code to configure
scripts that don't use AC_CHECK_DECL). What would you suggest?

Bruno

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Prerequisite-Macros.html

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Paul Eggert]

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

On 1/13/20 11:02 AM, Bruno Haible wrote:
> I would suggest that
> the particular 'case' and 'if'/'test' statements - or even the entire main body
> of the configure.ac, from line 130 to line 5588 - gets wrapped in an AC_DEFUN
> that gets invoked once.

Thanks for the diagnosis. I came up with a simpler patch to Emacs, and 
installed it into Emacs master (attached).

This patch doesn't solve the general problem, just this particular case. 
I doubt whether our collection of Emacs hackers can be induced to 
remember that the Autoconf macros you mentioned cannot be executed 
inside a shell condition, and I wouldn't be surprised if other configure 
scripts run into similar problems. I don't have any specific suggestion 
to work around this problem in Gnulib, though.

PS. I vaguely recall a long discussion many years ago when we added this 
AC_REQUIRE-ish stuff to Autoconf. Although we did fix some major 
glitches, we replacing them with other glitches that live on to this 
day. For example, there's now this note in the Autoconf manual:

      Many Autoconf macros use a compiler, and thus call
      `AC_REQUIRE([AC_PROG_CC])' to ensure that the compiler has been
      determined before the body of the outermost `AC_DEFUN' macro.
      Although `AC_PROG_CC' is safe to directly expand multiple times, it
      performs certain checks (such as the proper value of `EXEEXT') only
      on the first invocation.  Therefore, care must be used when
      invoking this macro from within another macro rather than at the
      top level (*note Expanded Before Required::).

all of which I had forgotten until I read your email today.

[-- Attachment #2: 0001-Port-configure.ac-to-future-Gnulib.patch --]
[-- Type: text/x-patch, Size: 2656 bytes --]

From 49ad550af6b5c1cfcb2fd31962967d7be71bfcc3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 13 Jan 2020 16:07:27 -0800
Subject: [PATCH] Port configure.ac to future Gnulib
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rewrite an ancient Alpha ELF check to port to a future Gnulib
version that may require AC_CHECK_DECL to be set up properly as
per the ‘Expanded Before Required’ section of the Autoconf manual
Autoconf doesn’t guarantee that AC_CHECK_DECL will work properly
if called conditionally (e.g., inside a shell ‘case’ statement)
and the condition is false.  Problem reported by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2020-01/msg00088.html
* configure.ac (LD_SWITCH_MACHINE): Migrate ELF check later,
when AC_CHECK_DECL is properly set up.
---
 configure.ac | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 08a4502122..f040b748d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1508,6 +1508,7 @@ AC_DEFUN
    UNEXEC_OBJ=unexelf.o
    ;;
 esac
+AC_SUBST(UNEXEC_OBJ)
 
 LD_SWITCH_SYSTEM=
 test "$with_unexec" = no || case "$opsys" in
@@ -1561,8 +1562,6 @@ AC_DEFUN
 test $with_unexec = yes &&
 case $canonical in
  alpha*)
-  AC_CHECK_DECL([__ELF__])
-  if test "$ac_cv_have_decl___ELF__" = "yes"; then
     ## With ELF, make sure that all common symbols get allocated to in the
     ## data section.  Otherwise, the dump of temacs may miss variables in
     ## the shared library that have been initialized.  For example, with
@@ -1573,18 +1572,10 @@ AC_DEFUN
     else
       AC_MSG_ERROR([Non-GCC compilers are not supported.])
     fi
-  else
-      dnl This was the unexalpha.c case.  Removed in 24.1, 2010-07-24,
-      dnl albeit under the mistaken assumption that said file
-      dnl was no longer used.
-      AC_MSG_ERROR([Non-ELF systems are not supported since Emacs 24.1.])
-  fi
   ;;
 esac
 AC_SUBST(C_SWITCH_MACHINE)
 
-AC_SUBST(UNEXEC_OBJ)
-
 C_SWITCH_SYSTEM=
 ## Some programs in src produce warnings saying certain subprograms
 ## are too complex and need a MAXMEM value greater than 2000 for
@@ -4216,6 +4207,12 @@ AC_DEFUN
 AC_CHECK_FUNCS([aligned_alloc posix_memalign], [break])
 AC_CHECK_DECLS([aligned_alloc], [], [], [[#include <stdlib.h>]])
 
+case $with_unexec,$canonical in
+  yes,alpha*)
+    AC_CHECK_DECL([__ELF__], [],
+      [AC_MSG_ERROR([Non-ELF systems are not supported on this platform.])]);;
+esac
+
 # Dump loading
 AC_CHECK_FUNCS([posix_madvise])
 
-- 
2.24.1

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Hi Paul,

> Thanks for the diagnosis. I came up with a simpler patch to Emacs, and 
> installed it into Emacs master (attached).
> 
> This patch doesn't solve the general problem, just this particular case. 

Yes. Among the hundreds of packages that use gnulib, it's likely that
some other package has the same issue.

> I don't have any specific suggestion 
> to work around this problem in Gnulib, though.

OK, here's my take at it:
- Restore the assumption that Autoconf built-in macros like AC_CHECK_DECL
  don't AC_REQUIRE anything.
- Perform the clang tests at the end of AC_PROG_CC instead.

Since I was testing this with the Emacs git, I noticed that the override
of AC_PROG_CC was not working, because Emacs redefines AC_PROG_CC.
As a mitigation:
  1) I moved the AC_PROG_CC override from 00gnulib.m4 to ~~gnulib.m4; this
     file sorts after the other *.m4 files in the same directory.
  2) Since I don't see a way to make things work if a package is redefining
     AC_PROG_CC in a file that sorts after m4/~~gnulib.m4: Use ac_compile
     if ac_compile_for_check_decl is not defined.


2020-01-18  Bruno Haible  <bruno@clisp.org>

	Fix major regression from 2020-01-10.
	Reported by Paul Eggert in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00079.html>.
	* m4/00gnulib.m4 (gl_COMPILER_CLANG, gl_COMPILER_PREPARE_CHECK_DECL):
	Don't AC_REQUIRE anything.
	(gl_COMPILER_PREPARE_CHECK_DECL): Define through AC_DEFUN, not
	AC_DEFUN_ONCE. Use _AC_COMPILE_IFELSE, not AC_COMPILE_IFELSE.
	(_AC_CHECK_DECL_BODY): If ac_compile_for_check_decl has not been set,
	use ac_compile instead.
	(AC_CHECK_DECL): Remove override.
	* m4/~~gnulib.m4: New file.
	* gnulib-tool (func_get_filelist): Add also ~~gnulib.m4.
	* pygnulib/GLModuleSystem.py (getFiles): Likewise.

diff --git a/m4/00gnulib.m4 b/m4/00gnulib.m4
index 58bc4ef..36304b9 100644
--- a/m4/00gnulib.m4
+++ b/m4/00gnulib.m4
@@ -1,4 +1,4 @@
-# 00gnulib.m4 serial 5
+# 00gnulib.m4 serial 6
 dnl Copyright (C) 2009-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -48,9 +48,14 @@ m4_version_prereq([2.63.263], [],
 # mode.  As of clang 9.0, these "known" functions are identified through
 # LIBBUILTIN invocations in the LLVM source file
 # llvm/tools/clang/include/clang/Basic/Builtins.def.
+# It's not possible to AC_REQUIRE the extra tests from AC_CHECK_DECL,
+# because AC_CHECK_DECL, like other Autoconf built-ins, is not supposed
+# to AC_REQUIRE anything: some configure.ac files have their first
+# AC_CHECK_DECL executed conditionally.  Therefore append the extra tests
+# to AC_PROG_CC.
 AC_DEFUN([gl_COMPILER_CLANG],
 [
-  AC_REQUIRE([AC_PROG_CC])
+dnl AC_REQUIRE([AC_PROG_CC])
   AC_CACHE_CHECK([whether the compiler is clang],
     [gl_cv_compiler_clang],
     [AC_EGREP_CPP([barfbarf],[
@@ -62,10 +67,10 @@ barfbarf
        [gl_cv_compiler_clang=no])
     ])
 ])
-AC_DEFUN_ONCE([gl_COMPILER_PREPARE_CHECK_DECL],
+AC_DEFUN([gl_COMPILER_PREPARE_CHECK_DECL],
 [
-  AC_REQUIRE([AC_PROG_CC])
-  AC_REQUIRE([gl_COMPILER_CLANG])
+dnl AC_REQUIRE([AC_PROG_CC])
+dnl AC_REQUIRE([gl_COMPILER_CLANG])
   AC_CACHE_CHECK([for compiler option needed when checking for declarations],
     [gl_cv_compiler_check_decl_option],
     [if test $gl_cv_compiler_clang = yes; then
@@ -73,7 +78,9 @@ AC_DEFUN_ONCE([gl_COMPILER_PREPARE_CHECK_DECL],
        dnl '-Werror=implicit-function-declaration'.
        save_ac_compile="$ac_compile"
        ac_compile="$ac_compile -Werror=implicit-function-declaration"
-       AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[]])],
+       dnl Use _AC_COMPILE_IFELSE instead of AC_COMPILE_IFELSE, to avoid a
+       dnl warning "AC_COMPILE_IFELSE was called before AC_USE_SYSTEM_EXTENSIONS".
+       _AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[]])],
          [gl_cv_compiler_check_decl_option='-Werror=implicit-function-declaration'],
          [gl_cv_compiler_check_decl_option=none])
        ac_compile="$save_ac_compile"
@@ -88,18 +95,15 @@ AC_DEFUN_ONCE([gl_COMPILER_PREPARE_CHECK_DECL],
   fi
 ])
 dnl Redefine _AC_CHECK_DECL_BODY so that it references ac_compile_for_check_decl
-dnl instead of ac_compile.
+dnl instead of ac_compile.  If, for whatever reason, the override of AC_PROG_CC
+dnl in ~~gnulib.m4 is inactive, use the original ac_compile.
 m4_define([_AC_CHECK_DECL_BODY],
 [  ac_save_ac_compile="$ac_compile"
-  ac_compile="$ac_compile_for_check_decl"]
+  if test -n "$ac_compile_for_check_decl"; then
+    ac_compile="$ac_compile_for_check_decl"
+  fi]
 m4_defn([_AC_CHECK_DECL_BODY])[  ac_compile="$ac_save_ac_compile"
 ])
-  ])
-dnl Redefine AC_CHECK_DECL so that it starts with an invocation of
-dnl gl_COMPILER_PREPARE_CHECK_DECL.
-m4_define([AC_CHECK_DECL],
-  [gl_COMPILER_PREPARE_CHECK_DECL dnl
-]m4_defn([AC_CHECK_DECL]))
 
 # gl_00GNULIB
 # -----------
diff --git a/m4/~~gnulib.m4 b/m4/~~gnulib.m4
new file mode 100644
index 0000000..bb5e910
--- /dev/null
+++ b/m4/~~gnulib.m4
@@ -0,0 +1,17 @@
+# ~~gnulib.m4 serial 1
+dnl Copyright (C) 2020 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+dnl This file must be named something that sorts after all other
+dnl package- or gnulib-provided .m4 files - at least for those packages
+dnl that redefine AC_PROG_CC.
+
+dnl Redefine AC_PROG_CC so that it ends with invocations of gl_COMPILER_CLANG
+dnl and gl_COMPILER_PREPARE_CHECK_DECL.
+m4_define([AC_PROG_CC],
+  m4_defn([AC_PROG_CC])[
+gl_COMPILER_CLANG
+gl_COMPILER_PREPARE_CHECK_DECL
+])
diff --git a/gnulib-tool b/gnulib-tool
index 1bf1be3..ba51606 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -2294,6 +2294,7 @@ func_get_filelist ()
     fi
   fi
   echo m4/00gnulib.m4
+  echo 'm4/~~gnulib.m4'
   echo m4/gnulib-common.m4
 }
 
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index 9560bc2..72a6c9f 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -544,6 +544,7 @@ Include:|Link:|License:|Maintainer:)'
                     parts += [line]
                 result = [part.strip() for part in parts if part.strip()]
             result += [joinpath('m4', '00gnulib.m4')]
+            result += [joinpath('m4', '~~gnulib.m4')]
             result += [joinpath('m4', 'gnulib-common.m4')]
             self.cache['files'] = list(result)
         return(list(self.cache['files']))

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

> 	* m4/~~gnulib.m4: New file.

Not sure whether this is needed, but it feels safer with this patch:


2020-01-18  Bruno Haible  <bruno@clisp.org>

	Ensure Automake does not drop ~~gnulib.m4.
	* m4/~~gnulib.m4 (gl_ZZGNULIB): New macro.
	* m4/gnulib-common.m4 (gl_COMMON): Require it.

diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index 6c4cad6..276ed1a 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -1,4 +1,4 @@
-# gnulib-common.m4 serial 46
+# gnulib-common.m4 serial 47
 dnl Copyright (C) 2007-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -12,6 +12,7 @@ AC_DEFUN([gl_COMMON], [
   dnl Use AC_REQUIRE here, so that the code is expanded once only.
   AC_REQUIRE([gl_00GNULIB])
   AC_REQUIRE([gl_COMMON_BODY])
+  AC_REQUIRE([gl_ZZGNULIB])
 ])
 AC_DEFUN([gl_COMMON_BODY], [
   AH_VERBATIM([_Noreturn],
diff --git a/m4/~~gnulib.m4 b/m4/~~gnulib.m4
index bb5e910..f476918 100644
--- a/m4/~~gnulib.m4
+++ b/m4/~~gnulib.m4
@@ -15,3 +15,9 @@ m4_define([AC_PROG_CC],
 gl_COMPILER_CLANG
 gl_COMPILER_PREPARE_CHECK_DECL
 ])
+
+# gl_ZZGNULIB
+# -----------
+# Witness macro that this file has been included.  Needed to force
+# Automake to include this file after all other gnulib .m4 files.
+AC_DEFUN([gl_ZZGNULIB])

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Mats Erik Andersson]

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

Sorry to pester you again.

The first change after my report was successful, but now
you continue to push non-portable, or rather, seemingly
version-dependent code.

Lördag den 18:e january 2020, klockan 14:34, skrev Bruno Haible detta:
> > 	* m4/~~gnulib.m4: New file.
> 
> Not sure whether this is needed, but it feels safer with this patch:

The following appears with Gnulib pulled just minutes ago,
when bootstrapping Inetutils on OpenIndiana. It was successful
with Gnulib ba4b91abd from the 10th of January!

Happy hunting wishes

  Mats E Andersson

[-- Attachment #2: autoreconf_failure.txt --]
[-- Type: text/plain, Size: 1867 bytes --]

running: AUTOPOINT=true LIBTOOLIZE=true autoreconf --verbose --install --force -I m4 -I am -I m4 --no-recursive
autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal-1.11 -I m4 -I am -I m4 --force -I am -I m4
/usr/share/aclocal/aalib.m4:12: warning: underquoted definition of AM_PATH_AALIB
/usr/share/aclocal/aalib.m4:12:   run info '(automake)Extending aclocal'
/usr/share/aclocal/aalib.m4:12:   or see http://www.gnu.org/software/automake/manual/automake.html#Extending-aclocal
configure.ac:126: error: m4_require: circular dependency of AC_LANG_COMPILER(C)
../../lib/autoconf/lang.m4:329: AC_LANG_COMPILER_REQUIRE is expanded from...
../../lib/autoconf/lang.m4:372: AC_LANG_PREPROC_REQUIRE is expanded from...
../../lib/autoconf/general.m4:2538: AC_EGREP_CPP is expanded from...
../../lib/m4sugar/m4sh.m4:639: AS_IF is expanded from...
../../lib/autoconf/general.m4:2031: AC_CACHE_VAL is expanded from...
../../lib/autoconf/general.m4:2052: AC_CACHE_CHECK is expanded from...
m4/00gnulib.m4:56: gl_COMPILER_CLANG is expanded from...
../../lib/autoconf/c.m4:429: AC_LANG_COMPILER(C) is expanded from...
../../lib/autoconf/lang.m4:329: AC_LANG_COMPILER_REQUIRE is expanded from...
../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
../../lib/m4sugar/m4sh.m4:639: AS_IF is expanded from...
../../lib/autoconf/general.m4:2031: AC_CACHE_VAL is expanded from...
../../lib/autoconf/general.m4:2052: AC_CACHE_CHECK is expanded from...
../../lib/autoconf/libs.m4:47: AC_SEARCH_LIBS is expanded from...
../../lib/autoconf/specific.m4:446: AC_ISC_POSIX is expanded from...
configure.ac:126: the top level
autom4te: /usr/sfw/bin/gm4 failed with exit status: 1
aclocal-1.11: /usr/bin/autom4te failed with exit status: 1
autoreconf: aclocal-1.11 failed with exit status: 1
./bootstrap: autoreconf failed

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Paul Eggert wrote:
> PS. I vaguely recall a long discussion many years ago when we added this 
> AC_REQUIRE-ish stuff to Autoconf. Although we did fix some major 
> glitches, we replacing them with other glitches that live on to this 
> day. For example, there's now this note in the Autoconf manual:
> 
>       Many Autoconf macros use a compiler, and thus call
>       `AC_REQUIRE([AC_PROG_CC])' to ensure that the compiler has been
>       determined before the body of the outermost `AC_DEFUN' macro.
>       Although `AC_PROG_CC' is safe to directly expand multiple times, it
>       performs certain checks (such as the proper value of `EXEEXT') only
>       on the first invocation.

and likewise for AC_EGREP_CPP, whose definition starts like this:

AC_DEFUN([AC_EGREP_CPP],
[AC_LANG_PREPROC_REQUIRE()dnl
AC_REQUIRE([AC_PROG_EGREP])dnl
...

where AC_LANG_PREPROC_REQUIRE() in this case requires AC_LANG_COMPILER(C).
So, you could also run into trouble if the first AC_EGREP_CPP invocation
in a configure.ac is within

case "$host_os" in
  alpha*)
    AC_EGREP_CPP(...)
    ;;
esac

Bruno

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Hi Mats,

> The following appears with Gnulib pulled just minutes ago,
> when bootstrapping Inetutils on OpenIndiana. It was successful
> with Gnulib ba4b91abd from the 10th of January!

Thanks for the report. The patch below fixes it.

> The first change after my report was successful, but now
> you continue to push non-portable, or rather, seemingly
> version-dependent code.

No, the changes I pushed were neither non-portable nor version-dependent.

The cause of the failure is that inetutils/configure.ac invokes
AC_ISC_POSIX before AC_PROG_CC. But AC_ISC_POSIX is documented to expand
into an invocation of AC_SEARCH_LIBS; therefore it requires to run the C
compiler. It therefore does not really make sense to invoke AC_ISC_POSIX
before AC_PROG_CC. You were just lucky that it worked before.

This is a grey area of Autoconf: where you don't get a warning about wrong
order of invocation of two macros, and the result seemingly works, but
stops working when one of the involved macros changes a bit.


2020-01-18  Bruno Haible  <bruno@clisp.org>

	Avoid error "m4_require: circular dependency of AC_LANG_COMPILER(C)".
	Reported by Mats Erik Andersson <mats.andersson@gisladisker.se> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00109.html>.
	* m4/00gnulib.m4 (gl_COMPILER_CLANG): Use _AC_COMPILE_IFELSE, not
	AC_EGREP_CPP.

diff --git a/m4/00gnulib.m4 b/m4/00gnulib.m4
index 36304b9..90e7e04 100644
--- a/m4/00gnulib.m4
+++ b/m4/00gnulib.m4
@@ -1,4 +1,4 @@
-# 00gnulib.m4 serial 6
+# 00gnulib.m4 serial 7
 dnl Copyright (C) 2009-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -58,13 +58,18 @@ AC_DEFUN([gl_COMPILER_CLANG],
 dnl AC_REQUIRE([AC_PROG_CC])
   AC_CACHE_CHECK([whether the compiler is clang],
     [gl_cv_compiler_clang],
-    [AC_EGREP_CPP([barfbarf],[
-#ifdef __clang__
-barfbarf
-#endif
-       ],
-       [gl_cv_compiler_clang=yes],
-       [gl_cv_compiler_clang=no])
+    [dnl Use _AC_COMPILE_IFELSE instead of AC_EGREP_CPP, to avoid error
+     dnl "circular dependency of AC_LANG_COMPILER(C)" if AC_PROG_CC has
+     dnl not yet been invoked.
+     _AC_COMPILE_IFELSE(
+        [AC_LANG_PROGRAM([[
+           #ifdef __clang__
+           barfbarf
+           #endif
+           ]],[[]])
+        ],
+        [gl_cv_compiler_clang=no],
+        [gl_cv_compiler_clang=yes])
     ])
 ])
 AC_DEFUN([gl_COMPILER_PREPARE_CHECK_DECL],

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Paul Eggert]

On 1/18/20 5:14 AM, Bruno Haible wrote:
> 1) I moved the AC_PROG_CC override from 00gnulib.m4 to ~~gnulib.m4

Although I haven't tried the patch, surely the name '~~gnulib.m4' causes 
problems. I imagine some programs use '~' for other naming conventions (e.g., 
for deleted files). On systems that use alphabetic characters first for 
collating, 'ls' and 'echo *' don't put the '~~' file last anyway; this happened 
on my Ubuntu 18.04.3 machine in the en_US.utf8 locale. On systems with GNU ls, 
the leading '~' causes ls to quote the file name and to put spaces in front of 
other file names in the same directory, which is an annoyance (albeit a 
necessary one to warn users of the problem with cutting and pasting that file 
name). And if there is a user whose name happens to be '~gnulib.m4' (admittedly 
unlikely, but POSIX allows this as an extension), the string ~~gnulib.m4 will 
expand to that user's home directory in the shell.

For all these reasons I suggest renaming the file to zzgnulib.m4.

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Mats Erik Andersson]

Dear Bruno,

Lördag den 18:e january 2020, klockan 19:11, skrev Bruno Haible detta:
> 
> The cause of the failure is that inetutils/configure.ac invokes
> AC_ISC_POSIX before AC_PROG_CC. But AC_ISC_POSIX is documented to expand
> into an invocation of AC_SEARCH_LIBS; therefore it requires to run the C
> compiler. It therefore does not really make sense to invoke AC_ISC_POSIX
> before AC_PROG_CC. You were just lucky that it worked before.

The new patch in itself makes the bootrapping successful without further
changes to configure.ac of Inetutils. That being said, I inserted the macro
AC_PROG_EGREP as recently as January 11th. And immediately it discloses
my ignorance of many details, sigh!

In contrast, our AC_ISC_POSIX sits fixed since 2005, but does not interfere
visibly (beyond the error log copied last time) with the advent of your most
recent change. Nonetheless, I will move it to a position where other
libraries are probed. Thank you for your most illuminating comments!

Regards,

   Mats E A

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Hi Paul,

> Although I haven't tried the patch, surely the name '~~gnulib.m4' causes 
> problems. I imagine some programs use '~' for other naming conventions (e.g., 
> for deleted files). On systems that use alphabetic characters first for 
> collating, 'ls' and 'echo *' don't put the '~~' file last anyway; this happened 
> on my Ubuntu 18.04.3 machine in the en_US.utf8 locale.

The order that matters is not the one used by 'echo *', but the used by the
'aclocal' program when it creates aclocal.m4.

> On systems with GNU ls, 
> the leading '~' causes ls to quote the file name and to put spaces in front of 
> other file names in the same directory, which is an annoyance (albeit a 
> necessary one to warn users of the problem with cutting and pasting that file 
> name). And if there is a user whose name happens to be '~gnulib.m4' (admittedly 
> unlikely, but POSIX allows this as an extension), the string ~~gnulib.m4 will 
> expand to that user's home directory in the shell.

Similar things would hold for lt~obsolete.m4 from libtool.

> For all these reasons I suggest renaming the file to zzgnulib.m4.

In practice zzgnulib.m4 should be as good as ~~gnulib.m4, since hardly any
.m4 file has a name that starts with a '{', '|'. '}', or '~'. => Done:


2020-01-18  Bruno Haible  <bruno@clisp.org>

	Rename ~~gnulib.m4 to zzgnulib.m4.
	Suggested by Paul Eggert.
	* m4/zzgnulib.m4: Renamed from m4/~~gnulib.m4.
	* gnulib-tool (func_get_filelist): Update.
	* pygnulib/GLModuleSystem.py (getFiles): Likewise.

diff --git a/gnulib-tool b/gnulib-tool
index ba51606..89f6077 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -2294,7 +2294,7 @@ func_get_filelist ()
     fi
   fi
   echo m4/00gnulib.m4
-  echo 'm4/~~gnulib.m4'
+  echo m4/zzgnulib.m4
   echo m4/gnulib-common.m4
 }
 
diff --git a/m4/00gnulib.m4 b/m4/00gnulib.m4
index 90e7e04..06eff4f 100644
--- a/m4/00gnulib.m4
+++ b/m4/00gnulib.m4
@@ -101,7 +101,7 @@ dnl AC_REQUIRE([gl_COMPILER_CLANG])
 ])
 dnl Redefine _AC_CHECK_DECL_BODY so that it references ac_compile_for_check_decl
 dnl instead of ac_compile.  If, for whatever reason, the override of AC_PROG_CC
-dnl in ~~gnulib.m4 is inactive, use the original ac_compile.
+dnl in zzgnulib.m4 is inactive, use the original ac_compile.
 m4_define([_AC_CHECK_DECL_BODY],
 [  ac_save_ac_compile="$ac_compile"
   if test -n "$ac_compile_for_check_decl"; then
diff --git a/m4/zzgnulib.m4 b/m4/zzgnulib.m4
index f476918..98fa68f 100644
--- a/m4/zzgnulib.m4
+++ b/m4/zzgnulib.m4
@@ -1,4 +1,4 @@
-# ~~gnulib.m4 serial 1
+# zzgnulib.m4 serial 1
 dnl Copyright (C) 2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
diff --git a/pygnulib/GLModuleSystem.py b/pygnulib/GLModuleSystem.py
index 72a6c9f..73c207d 100644
--- a/pygnulib/GLModuleSystem.py
+++ b/pygnulib/GLModuleSystem.py
@@ -544,7 +544,7 @@ Include:|Link:|License:|Maintainer:)'
                     parts += [line]
                 result = [part.strip() for part in parts if part.strip()]
             result += [joinpath('m4', '00gnulib.m4')]
-            result += [joinpath('m4', '~~gnulib.m4')]
+            result += [joinpath('m4', 'zzgnulib.m4')]
             result += [joinpath('m4', 'gnulib-common.m4')]
             self.cache['files'] = list(result)
         return(list(self.cache['files']))

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Mats Erik Andersson wrote:
> The new patch in itself makes the bootrapping successful without further
> changes to configure.ac of Inetutils.

Thanks for having tested it.

> In contrast, our AC_ISC_POSIX sits fixed since 2005, but does not interfere
> visibly (beyond the error log copied last time) with the advent of your most
> recent change. Nonetheless, I will move it to a position where other
> libraries are probed.

Given that its documentation [1] says that ISC is unsupported for more than
13 years (and no gnulib developer ever had access to it, since 2003), you can
just as well remove its invocation.

Bruno

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Paul Eggert]

Thanks for renaming the file.

On 1/18/20 2:11 PM, Bruno Haible wrote:
> Similar things would hold for lt~obsolete.m4 from libtool.

Those particular problems occur only with file names that begin with '~'. Shell 
tilde expansion applies only to leading '~' and 'ls' takes that into account 
when quoting file names, so 'lt~obsolete.m4' should be immune.

Re: Messed up gl_COMPILER_PREPARE_CHECK_DECL

[Bruno Haible]

Paul Eggert wrote:
> > Similar things would hold for lt~obsolete.m4 from libtool.
> 
> Those particular problems occur only with file names that begin with '~'. Shell 
> tilde expansion applies only to leading '~' and 'ls' takes that into account 
> when quoting file names, so 'lt~obsolete.m4' should be immune.

Ah, ok. I stand corrected.

Bruno