[PATCH] autoregen.py: use autoreconf in some directories

[PATCH] autoregen.py: use autoreconf in some directories

[Simon Marchi]

The builder currently mis-generates the config files for the gdb and
gnulib directories.  It passes `-I ../config` to aclocal, causing the
include path order to be different to what it would be without that -I
argument, causing the generated aclocal.m4 to be different (same entries
but different order).

For the gdb, gdbserver, gdbsupport and gnulib directories, the config
files are usually re-generated without any -I argument.  In fact, for
gnulib, it is baked in gnulib/update-gnulib.sh that aclocal should be
called without -I arguments:

    aclocal &&
    autoconf &&
    autoheader &&
    automake

Moreover, they can be re-generated using autoreconf, removing the need
to guess which speicific tools need to be invoked.  autoreconf takes a
bit more time to run than the individual tools, but it's worth it for
the simplicity.

So, add a list of directories for which it is known that they can be
re-generated using a simple autoreconf without special flags.  For these
directories, call `autoreconf -f`.
---
 builder/containers/autoregen.py | 63 ++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index ac16f54f6caa..138b9af2590f 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -13,12 +13,14 @@ AUTOCONF_NAMES = ["autoconf-vanilla-2.69", "autoconf-2.69", "autoconf"]
 AUTOMAKE_NAMES = ["automake-vanilla-1.15", "automake-1.15.1", "automake"]
 ACLOCAL_NAMES = ["aclocal-vanilla-1.15", "aclocal-1.15.1", "aclocal"]
 AUTOHEADER_NAMES = ["autoheader-vanilla-2.69", "autoheader-2.69", "autoheader"]
+AUTORECONF_NAMES = ["autoreconf-vanilla-2.69", "autoreconf-2.69", "autoreconf"]
 
 # Pick the first for each list that exists on this system.
 AUTOCONF_BIN = next(name for name in AUTOCONF_NAMES if shutil.which(name))
 AUTOMAKE_BIN = next(name for name in AUTOMAKE_NAMES if shutil.which(name))
 ACLOCAL_BIN = next(name for name in ACLOCAL_NAMES if shutil.which(name))
 AUTOHEADER_BIN = next(name for name in AUTOHEADER_NAMES if shutil.which(name))
+AUTORECONF_BIN = next(name for name in AUTORECONF_NAMES if shutil.which(name))
 
 # autoconf-wrapper and automake-wrapper from Gentoo look at this environment variable.
 # It's harmless to set it on other systems though.
@@ -41,6 +43,15 @@ SKIP_DIRS = [
     "minizip",
 ]
 
+# these directories are known to can be re-generatable with a simple autoreconf
+# without special -I flags
+AUTORECONF_DIRS = [
+    "gdb",
+    "gdbserver",
+    "gdbsupport",
+    "gnulib",
+]
+
 
 # Run the shell command CMD.
 #
@@ -57,28 +68,11 @@ def run_shell(cmd: str):
     res.check_returncode()
 
 
-run_shell(f"{AUTOCONF_BIN} --version")
-run_shell(f"{AUTOMAKE_BIN} --version")
-run_shell(f"{ACLOCAL_BIN} --version")
-run_shell(f"{AUTOHEADER_BIN} --version")
-
-print(f"Environment: {ENV}")
+def regenerate_with_autoreconf():
+    run_shell(f"{AUTORECONF_BIN} -f")
 
-config_folders: list[Path] = []
-
-for root, _, files in os.walk("."):
-    for file in files:
-        if file == "configure.ac":
-            config_folders.append(Path(root).resolve())
-
-for folder in sorted(config_folders):
-    if folder.stem in SKIP_DIRS:
-        print(f"Skipping directory {folder}", flush=True)
-        continue
-
-    print(f"Entering directory {folder}", flush=True)
-    os.chdir(folder)
 
+def regenerate_manually():
     configure_lines = open("configure.ac").read().splitlines()
     if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIRS")):
         # aclocal does not support the -f short option for force
@@ -107,3 +101,32 @@ for folder in sorted(config_folders):
         run_shell(f"{AUTOMAKE_BIN} -f")
 
     run_shell(f"{AUTOCONF_BIN} -f")
+
+
+run_shell(f"{AUTOCONF_BIN} --version")
+run_shell(f"{AUTOMAKE_BIN} --version")
+run_shell(f"{ACLOCAL_BIN} --version")
+run_shell(f"{AUTOHEADER_BIN} --version")
+
+print(f"Environment: {ENV}")
+
+config_folders: list[Path] = []
+repo_root = Path.cwd()
+
+for root, _, files in os.walk("."):
+    for file in files:
+        if file == "configure.ac":
+            config_folders.append(Path(root).resolve())
+
+for folder in sorted(config_folders):
+    if folder.stem in SKIP_DIRS:
+        print(f"Skipping directory {folder}", flush=True)
+        continue
+
+    print(f"Entering directory {folder}", flush=True)
+    os.chdir(folder)
+
+    if str(folder.relative_to(repo_root)) in AUTORECONF_DIRS:
+        regenerate_with_autoreconf()
+    else:
+        regenerate_manually()

base-commit: 3e013842366735c373ef19ec6dfc4a33d6f9c473
-- 
2.44.0

Re: [PATCH] autoregen.py: use autoreconf in some directories

[Mark Wielaard]

Hi Simon,

On Tue, Mar 12, 2024 at 01:53:37PM -0400, Simon Marchi wrote:
> The builder currently mis-generates the config files for the gdb and
> gnulib directories.  It passes `-I ../config` to aclocal, causing the
> include path order to be different to what it would be without that -I
> argument, causing the generated aclocal.m4 to be different (same entries
> but different order).
> 
> For the gdb, gdbserver, gdbsupport and gnulib directories, the config
> files are usually re-generated without any -I argument.  In fact, for
> gnulib, it is baked in gnulib/update-gnulib.sh that aclocal should be
> called without -I arguments:
> 
>     aclocal &&
>     autoconf &&
>     autoheader &&
>     automake
> 
> Moreover, they can be re-generated using autoreconf, removing the need
> to guess which speicific tools need to be invoked.  autoreconf takes a
> bit more time to run than the individual tools, but it's worth it for
> the simplicity.
> 
> So, add a list of directories for which it is known that they can be
> re-generated using a simple autoreconf without special flags.  For these
> directories, call `autoreconf -f`.

Very nice, now running this regenerates the binutils-gdb files exactly
as they are checked in. Also tested on the gcc tree, which also works
fine.

Applied (with small adjustments for the previous two patches).

Thanks,

Mark