public inbox for buildbot@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/6] autoregen.py: Improve support for GCC
@ 2024-04-12 20:05 Christophe Lyon
  2024-04-12 20:05 ` [PATCH 1/6] auroregen.py: Check AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS Christophe Lyon
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

Hi,

I looked into more details to improve autoregen.py's support for GCC.

I noticed that autoreconf works well in most directories, so this
patch series adds most GCC subdirs to AUTORECONF_DIRS.

Following that, I noticed that quite a few more subdirs from
binutils-gdb can also be regenerated using autoreconf, so add them to
AUTORECONF_DIRS.

In addition, add support for autogen, to regenerate the top-level
Makefile.in.

There are still some peculiarities with some subdirs, so there are
probably some fixes to make in binutils, gdb and gcc.  We can update
this script when that is the case.

Also, this script should probably be copied into contrib/ for the
benefit of the projects.

Christophe Lyon (6):
  auroregen.py: Check AC_CONFIG_MACRO_DIR instead of
    AC_CONFIG_MACRO_DIRS
  autoregen.py: Move comment...
  autoregen.py: Flush all print commands
  autoregen.py: Use autoreconf in most GCC directories
  autoregen.py: Add support for autogen
  autoregen.py: Add binutils directories to AUTORECONF_DIRS

 builder/containers/autoregen.py | 93 +++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 4 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/6] auroregen.py: Check AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-12 20:05 ` [PATCH 2/6] autoregen.py: Move comment Christophe Lyon
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

Some configure.ac files still use AC_CONFIG_MACRO_DIR rather than
AC_CONFIG_MACRO_DIRS, so we would skip them without this patch.
---
 builder/containers/autoregen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index 64b1626..7f18f35 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -71,7 +71,7 @@ def regenerate_with_autoreconf():
 
 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")):
+    if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
         # aclocal does not support the -f short option for force
         include_arg = ""
         include_arg2 = ""
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/6] autoregen.py: Move comment...
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
  2024-04-12 20:05 ` [PATCH 1/6] auroregen.py: Check AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-12 20:05 ` [PATCH 3/6] autoregen.py: Flush all print commands Christophe Lyon
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

... just before the line where it makes sense
---
 builder/containers/autoregen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index 7f18f35..6e341e2 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -72,7 +72,6 @@ def regenerate_with_autoreconf():
 def regenerate_manually():
     configure_lines = open("configure.ac").read().splitlines()
     if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
-        # aclocal does not support the -f short option for force
         include_arg = ""
         include_arg2 = ""
         if (folder / ".." / "config").is_dir():
@@ -84,6 +83,7 @@ def regenerate_manually():
             include_arg = "-I../.."
             include_arg2 = "-I../../config"
 
+        # aclocal does not support the -f short option for force
         run_shell(f"{ACLOCAL_BIN} --force {include_arg} {include_arg2}")
 
     if (folder / "config.in").is_file() or any(
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/6] autoregen.py: Flush all print commands
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
  2024-04-12 20:05 ` [PATCH 1/6] auroregen.py: Check AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS Christophe Lyon
  2024-04-12 20:05 ` [PATCH 2/6] autoregen.py: Move comment Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-15  2:09   ` Simon Marchi
  2024-04-12 20:05 ` [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories Christophe Lyon
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

For consistency and easier debug of traces
---
 builder/containers/autoregen.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index 6e341e2..c52a524 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -56,7 +56,7 @@ AUTORECONF_DIRS = [
 #
 # Print the command on stdout prior to running it.
 def run_shell(cmd: str):
-    print(f"+ {cmd}")
+    print(f"+ {cmd}", flush=True)
     res = subprocess.run(
         f"{cmd}",
         shell=True,
@@ -105,7 +105,7 @@ run_shell(f"{AUTOMAKE_BIN} --version")
 run_shell(f"{ACLOCAL_BIN} --version")
 run_shell(f"{AUTOHEADER_BIN} --version")
 
-print(f"Extra environment: {EXTRA_ENV}")
+print(f"Extra environment: {EXTRA_ENV}", flush=True)
 
 config_folders: list[Path] = []
 repo_root = Path.cwd()
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
                   ` (2 preceding siblings ...)
  2024-04-12 20:05 ` [PATCH 3/6] autoregen.py: Flush all print commands Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-15  3:07   ` Simon Marchi
  2024-04-15 11:42   ` Mark Wielaard
  2024-04-12 20:05 ` [PATCH 5/6] autoregen.py: Add support for autogen Christophe Lyon
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
-f' works for them.

A few subdirs still need to be regenerated "manually", because they do
not have Makefile.am which autoreconf uses to determine which aclocal
flags to use.

Most use our default aclocal -I../config, but a few need special
flags, which I copied from the corresponding Makefile.in:
* fixincludes: -I.. -I../config
* gcc, libiberty, libobjc: -I../config -I..
* libgm2: -I../config -I.. although Makefile.in says otherwise. Using
  what Makefile.in defines results in generating aclocal.m4 with a
  different contents than what it is in the repo. The repo is
  presumably wrong, but use this until this is fixed.

Note that we do not regenerate
libvtv/testsuite/other-tests/Makefile.in, because
libvtv/testsuite/other-tests/ has no configure.ac.

Running this script over GCC's repository generates quite a few
warnings, which are the same as before this patch. Namely, these
subdirs seem to have incorrect autotools files (at least partially):
* libgfortran
* libgomp
* libsanitizer

This patch does not support regenerating gcc/m2/gm2-libs/config-host
and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
regenerate them with the exact same content. FTR I tried:
autoconf -f config-host.in > config-host
autoheader config-host.in
as described in gcc/m2/Make-maintainer.in

Similarly, we skip libcpp because the files we regenerate have small
differences with the current versions.

Tested by removing all aclocal.m4 files, Makefile.in files derived
from Makefile.am, configure files derived from configure.ac and
checking they are correctly regenerated (that is, 'git status' does
not report deleted nor modified files).
---
 builder/containers/autoregen.py | 60 ++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index c52a524..4731a87 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -40,15 +40,56 @@ SKIP_DIRS = [
     # readline and minizip are maintained with different autotools versions
     "readline",
     "minizip",
+
+    # aclocal.m4 gets an additional ../config/override.m4,
+    # and config.in gets a few more empty lines.
+    "libcpp",
 ]
 
 # these directories are known to can be re-generatable with a simple autoreconf
 # without special -I flags
 AUTORECONF_DIRS = [
+    # binutils-gdb subdirs
     "gdb",
     "gdbserver",
     "gdbsupport",
     "gnulib",
+
+    # gcc subdirs
+    "c++tools", # No aclocal.m4
+    #"gcc", # No Makefile.am
+    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
+    "gnattools", # No aclocal.m4
+    "gotools",
+    "libada", # No aclocal.m4
+    "libatomic",
+    "libbacktrace",
+    "libcc1",
+    "libcody", # No aclocal.m4
+    #"libcpp", # No Makefile.am
+    #"libdecnumber", # No Makefile.am
+    "libffi",
+    "libgcc", # No aclocal.m4
+    "libgfortran",
+    # Hack: ACLOCAL_AMFLAGS = -I .. -I ../config in Makefile.in but we
+    # apply -I../config -I.. otherwise we do not match the current
+    # contents
+    #"libgm2",
+    "libgo",
+    "libgomp",
+    "libgrust",
+    #"libiberty", # No Makefile.am
+    "libitm",
+    #"libobjc", # No Makefile.am
+    "libphobos",
+    "libquadmath",
+    "libsanitizer",
+    "libssp",
+    "libstdc++-v3",
+    # This does not cover libvtv/testsuite/other-tests/Makefile.in
+    "libvtv",
+    "lto-plugin",
+    "zlib",
 ]
 
 
@@ -71,7 +112,9 @@ def regenerate_with_autoreconf():
 
 def regenerate_manually():
     configure_lines = open("configure.ac").read().splitlines()
-    if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
+    if folder.stem == "fixincludes" or folder.stem == "libgm2" or any(
+            True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")
+    ):
         include_arg = ""
         include_arg2 = ""
         if (folder / ".." / "config").is_dir():
@@ -83,6 +126,14 @@ def regenerate_manually():
             include_arg = "-I../.."
             include_arg2 = "-I../../config"
 
+        if folder.stem == "fixincludes":
+            include_arg = "-I.."
+            include_arg2 = "-I../config"
+
+        if folder.stem == "libgm2" or folder.stem == "gcc" or folder.stem == "libiberty" or folder.stem == "libobjc":
+            include_arg = "-I../config"
+            include_arg2 = "-I.."
+
         # aclocal does not support the -f short option for force
         run_shell(f"{ACLOCAL_BIN} --force {include_arg} {include_arg2}")
 
@@ -91,6 +142,13 @@ def regenerate_manually():
     ):
         run_shell(f"{AUTOHEADER_BIN} -f")
 
+    # The few lines below do not regenerate the exact same content as
+    # currently in the repo. Disable them for now.
+    # if (folder / "gm2-libs").is_dir():
+    #     run_shell(f"{AUTOCONF_BIN} -f gm2-libs/config-host.in > gm2-libs/config-host")
+    #     run_shell(f"{AUTOHEADER_BIN} gm2-libs/config-host.in")
+
+
     # apparently automake is somehow unstable -> skip it for gotools
     if any(
         True for line in configure_lines if line.startswith("AM_INIT_AUTOMAKE")
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/6] autoregen.py: Add support for autogen
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
                   ` (3 preceding siblings ...)
  2024-04-12 20:05 ` [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-12 20:05 ` [PATCH 6/6] autoregen.py: Add binutils directories to AUTORECONF_DIRS Christophe Lyon
  2024-04-14 22:33 ` [PATCH 0/6] autoregen.py: Improve support for GCC Mark Wielaard
  6 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

GCC's toplevel Makefile.in is generated with
autogen Makefile.def
(which uses Makefile.tpl too)
---
 builder/containers/autoregen.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index 4731a87..d1ab217 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -21,6 +21,8 @@ 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))
 
+AUTOGEN_BIN = "autogen"
+
 # autoconf-wrapper and automake-wrapper from Gentoo look at this environment variable.
 # It's harmless to set it on other systems though.
 EXTRA_ENV = {
@@ -29,6 +31,7 @@ EXTRA_ENV = {
     "AUTOCONF": AUTOCONF_BIN,
     "ACLOCAL": ACLOCAL_BIN,
     "AUTOMAKE": AUTOMAKE_BIN,
+    "AUTOGEN": AUTOGEN_BIN,
 }
 ENV = os.environ.copy()
 ENV.update(EXTRA_ENV)
@@ -110,6 +113,9 @@ def run_shell(cmd: str):
 def regenerate_with_autoreconf():
     run_shell(f"{AUTORECONF_BIN} -f")
 
+def regenerate_with_autogen():
+    run_shell(f"{AUTOGEN_BIN} Makefile.def")
+
 def regenerate_manually():
     configure_lines = open("configure.ac").read().splitlines()
     if folder.stem == "fixincludes" or folder.stem == "libgm2" or any(
@@ -166,12 +172,20 @@ run_shell(f"{AUTOHEADER_BIN} --version")
 print(f"Extra environment: {EXTRA_ENV}", flush=True)
 
 config_folders: list[Path] = []
+autogen_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())
+        if file == "Makefile.tpl":
+            autogen_folders.append(Path(root).resolve())
+
+for folder in sorted(autogen_folders):
+    print(f"Entering directory {folder}", flush=True)
+    os.chdir(folder)
+    regenerate_with_autogen()
 
 for folder in sorted(config_folders):
     if folder.stem in SKIP_DIRS:
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/6] autoregen.py: Add binutils directories to AUTORECONF_DIRS
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
                   ` (4 preceding siblings ...)
  2024-04-12 20:05 ` [PATCH 5/6] autoregen.py: Add support for autogen Christophe Lyon
@ 2024-04-12 20:05 ` Christophe Lyon
  2024-04-14 22:33 ` [PATCH 0/6] autoregen.py: Improve support for GCC Mark Wielaard
  6 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-12 20:05 UTC (permalink / raw)
  To: buildbot; +Cc: Christophe Lyon

These directories are correctly regenerated using autoreconf, so use
it as it is simpler.
---
 builder/containers/autoregen.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
index d1ab217..64d132e 100755
--- a/builder/containers/autoregen.py
+++ b/builder/containers/autoregen.py
@@ -53,10 +53,23 @@ SKIP_DIRS = [
 # without special -I flags
 AUTORECONF_DIRS = [
     # binutils-gdb subdirs
+    "bfd",
+    "binutils",
+    "etc",
+    "gas",
     "gdb",
     "gdbserver",
     "gdbsupport",
     "gnulib",
+    "gold",
+    "gprof",
+    "gprofng",
+    "gprofng/libcollector",
+    "ld",
+    "libctf",
+    "libsframe",
+    "opcodes",
+    "sim",
 
     # gcc subdirs
     "c++tools", # No aclocal.m4
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/6] autoregen.py: Improve support for GCC
  2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
                   ` (5 preceding siblings ...)
  2024-04-12 20:05 ` [PATCH 6/6] autoregen.py: Add binutils directories to AUTORECONF_DIRS Christophe Lyon
@ 2024-04-14 22:33 ` Mark Wielaard
  2024-04-15 11:52   ` Christophe Lyon
  6 siblings, 1 reply; 21+ messages in thread
From: Mark Wielaard @ 2024-04-14 22:33 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: buildbot

Hi Christophe,

On Fri, Apr 12, 2024 at 08:05:53PM +0000, Christophe Lyon wrote:
> I looked into more details to improve autoregen.py's support for GCC.
> 
> I noticed that autoreconf works well in most directories, so this
> patch series adds most GCC subdirs to AUTORECONF_DIRS.

Nice, that simplifies things.

> Following that, I noticed that quite a few more subdirs from
> binutils-gdb can also be regenerated using autoreconf, so add them to
> AUTORECONF_DIRS.

Ack.

> In addition, add support for autogen, to regenerate the top-level
> Makefile.in.

I added autogen to the Containerfile-autotools to make sure it is
available. I assume the version doesn't matter.

> There are still some peculiarities with some subdirs, so there are
> probably some fixes to make in binutils, gdb and gcc.  We can update
> this script when that is the case.
> 
> Also, this script should probably be copied into contrib/ for the
> benefit of the projects.

Yes, that would be good. Do binutils-gdb/gcc share a contrib dir?
We could pick up the script from there, we don't necessary need to
have it in the builder.git itself.

> Christophe Lyon (6):
>   auroregen.py: Check AC_CONFIG_MACRO_DIR instead of
>     AC_CONFIG_MACRO_DIRS
>   autoregen.py: Move comment...
>   autoregen.py: Flush all print commands
>   autoregen.py: Use autoreconf in most GCC directories
>   autoregen.py: Add support for autogen
>   autoregen.py: Add binutils directories to AUTORECONF_DIRS

Applied all of them.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/6] autoregen.py: Flush all print commands
  2024-04-12 20:05 ` [PATCH 3/6] autoregen.py: Flush all print commands Christophe Lyon
@ 2024-04-15  2:09   ` Simon Marchi
  2024-04-15 11:55     ` Christophe Lyon
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2024-04-15  2:09 UTC (permalink / raw)
  To: Christophe Lyon, buildbot



On 2024-04-12 16:05, Christophe Lyon wrote:
> For consistency and easier debug of traces
> ---
>  builder/containers/autoregen.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
> index 6e341e2..c52a524 100755
> --- a/builder/containers/autoregen.py
> +++ b/builder/containers/autoregen.py
> @@ -56,7 +56,7 @@ AUTORECONF_DIRS = [
>  #
>  # Print the command on stdout prior to running it.
>  def run_shell(cmd: str):
> -    print(f"+ {cmd}")
> +    print(f"+ {cmd}", flush=True)
>      res = subprocess.run(
>          f"{cmd}",
>          shell=True,
> @@ -105,7 +105,7 @@ run_shell(f"{AUTOMAKE_BIN} --version")
>  run_shell(f"{ACLOCAL_BIN} --version")
>  run_shell(f"{AUTOHEADER_BIN} --version")
>  
> -print(f"Extra environment: {EXTRA_ENV}")
> +print(f"Extra environment: {EXTRA_ENV}", flush=True)
>  
>  config_folders: list[Path] = []
>  repo_root = Path.cwd()

Can you clarify when/where/why that's needed?  I would expect the stdout
stream to be flushed when there is an end-of-line (which print() does).

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-12 20:05 ` [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories Christophe Lyon
@ 2024-04-15  3:07   ` Simon Marchi
  2024-04-15 12:52     ` Christophe Lyon
  2024-04-15 11:42   ` Mark Wielaard
  1 sibling, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2024-04-15  3:07 UTC (permalink / raw)
  To: Christophe Lyon, buildbot



On 2024-04-12 16:05, Christophe Lyon wrote:
> Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> -f' works for them.
> 
> A few subdirs still need to be regenerated "manually", because they do
> not have Makefile.am which autoreconf uses to determine which aclocal
> flags to use.

gdb, for instance, doesn't use Makefile.am, and yet autoreconf works.  It
finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in
configure.ac.  If the remaining subdirs that still need to be manually
handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them?

Out of the directories you commented out from AUTORECONF_DIRS, the
following re-generated cleanly with autoreconf for me:

 - libdecnumber
 - gcc
 - libiberty
 - libobjc

So I would suggest adding them to AUTORECONF_DIRS.

> Most use our default aclocal -I../config, but a few need special
> flags, which I copied from the corresponding Makefile.in:
> * fixincludes: -I.. -I../config
> * gcc, libiberty, libobjc: -I../config -I..
> * libgm2: -I../config -I.. although Makefile.in says otherwise. Using
>   what Makefile.in defines results in generating aclocal.m4 with a
>   different contents than what it is in the repo. The repo is
>   presumably wrong, but use this until this is fixed.
> 
> Note that we do not regenerate
> libvtv/testsuite/other-tests/Makefile.in, because
> libvtv/testsuite/other-tests/ has no configure.ac.
> 
> Running this script over GCC's repository generates quite a few
> warnings, which are the same as before this patch. Namely, these
> subdirs seem to have incorrect autotools files (at least partially):
> * libgfortran
> * libgomp
> * libsanitizer
> 
> This patch does not support regenerating gcc/m2/gm2-libs/config-host
> and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
> regenerate them with the exact same content. FTR I tried:
> autoconf -f config-host.in > config-host
> autoheader config-host.in
> as described in gcc/m2/Make-maintainer.in
> 
> Similarly, we skip libcpp because the files we regenerate have small
> differences with the current versions.

libcpp's files appear to be in an invalid state in the repo itself.
According to libcpp/Makefile.in line 123, aclocal.m4 should be
regenerated with `aclocal -I ../config`.  When I use that, I get the
same results as what autoreconf would give me.  For that one, I think
you could send a patch to gcc to fix it in the repo, after which we can
switch it to use autoreconf.

> Tested by removing all aclocal.m4 files, Makefile.in files derived
> from Makefile.am, configure files derived from configure.ac and
> checking they are correctly regenerated (that is, 'git status' does
> not report deleted nor modified files).
> ---
>  builder/containers/autoregen.py | 60 ++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
> index c52a524..4731a87 100755
> --- a/builder/containers/autoregen.py
> +++ b/builder/containers/autoregen.py
> @@ -40,15 +40,56 @@ SKIP_DIRS = [
>      # readline and minizip are maintained with different autotools versions
>      "readline",
>      "minizip",
> +
> +    # aclocal.m4 gets an additional ../config/override.m4,
> +    # and config.in gets a few more empty lines.
> +    "libcpp",
>  ]
>  
>  # these directories are known to can be re-generatable with a simple autoreconf
>  # without special -I flags
>  AUTORECONF_DIRS = [
> +    # binutils-gdb subdirs
>      "gdb",
>      "gdbserver",
>      "gdbsupport",
>      "gnulib",
> +
> +    # gcc subdirs
> +    "c++tools", # No aclocal.m4
> +    #"gcc", # No Makefile.am
> +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> +    "gnattools", # No aclocal.m4
> +    "gotools",
> +    "libada", # No aclocal.m4
> +    "libatomic",
> +    "libbacktrace",
> +    "libcc1",
> +    "libcody", # No aclocal.m4
> +    #"libcpp", # No Makefile.am
> +    #"libdecnumber", # No Makefile.am
> +    "libffi",
> +    "libgcc", # No aclocal.m4
> +    "libgfortran",
> +    # Hack: ACLOCAL_AMFLAGS = -I .. -I ../config in Makefile.in but we
> +    # apply -I../config -I.. otherwise we do not match the current
> +    # contents
> +    #"libgm2",
> +    "libgo",
> +    "libgomp",
> +    "libgrust",
> +    #"libiberty", # No Makefile.am
> +    "libitm",
> +    #"libobjc", # No Makefile.am
> +    "libphobos",
> +    "libquadmath",
> +    "libsanitizer",
> +    "libssp",
> +    "libstdc++-v3",
> +    # This does not cover libvtv/testsuite/other-tests/Makefile.in
> +    "libvtv",
> +    "lto-plugin",
> +    "zlib",

If you really want to make the distinction between binutils-gdb and gcc
subdirs, perhaps add a "common" section for those directories that
appear in both repos?

>  ]
>  
>  
> @@ -71,7 +112,9 @@ def regenerate_with_autoreconf():
>  
>  def regenerate_manually():
>      configure_lines = open("configure.ac").read().splitlines()
> -    if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
> +    if folder.stem == "fixincludes" or folder.stem == "libgm2" or any(
> +            True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")
> +    ):
>          include_arg = ""
>          include_arg2 = ""
>          if (folder / ".." / "config").is_dir():
> @@ -83,6 +126,14 @@ def regenerate_manually():
>              include_arg = "-I../.."
>              include_arg2 = "-I../../config"
>  
> +        if folder.stem == "fixincludes":
> +            include_arg = "-I.."
> +            include_arg2 = "-I../config"
> +
> +        if folder.stem == "libgm2" or folder.stem == "gcc" or folder.stem == "libiberty" or folder.stem == "libobjc":

This could be a bit shorter:

  if folder.stem in ["libgm2", "gcc", "libiberty", "libobjc"]:

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-12 20:05 ` [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories Christophe Lyon
  2024-04-15  3:07   ` Simon Marchi
@ 2024-04-15 11:42   ` Mark Wielaard
  2024-04-15 12:46     ` Mark Wielaard
  1 sibling, 1 reply; 21+ messages in thread
From: Mark Wielaard @ 2024-04-15 11:42 UTC (permalink / raw)
  To: Christophe Lyon, buildbot; +Cc: Thomas Schwinge

Hi,

On Fri, 2024-04-12 at 20:05 +0000, Christophe Lyon wrote:
> Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> -f' works for them.
> [...]
>  # these directories are known to can be re-generatable with a simple autoreconf
>  # without special -I flags
>  AUTORECONF_DIRS = [
> +    # binutils-gdb subdirs
>      "gdb",
>      "gdbserver",
>      "gdbsupport",
>      "gnulib",
> +
> +    # gcc subdirs
> +    "c++tools", # No aclocal.m4
> +    #"gcc", # No Makefile.am
> +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> +    "gnattools", # No aclocal.m4
> +    "gotools",

Oddly this now generates:
https://builder.sourceware.org/buildbot/#/builders/269/builds/4325

Entering directory /home/builder/shared/bb3/worker/gcc-
autoregen/build/gnattools
+ autoreconf-2.69 -f

[... where before it was just autoconf-2.69 -f ...]

diff --git a/gotools/Makefile.in b/gotools/Makefile.in
index 36c2ec2abd3..f40883c39be 100644
--- a/gotools/Makefile.in
+++ b/gotools/Makefile.in
@@ -704,8 +704,8 @@ distclean-generic:
 maintainer-clean-generic:
 	@echo "This command is intended for maintainers to use"
 	@echo "it deletes files that may require special tools to
rebuild."
-@NATIVE_FALSE@install-exec-local:
 @NATIVE_FALSE@uninstall-local:
+@NATIVE_FALSE@install-exec-local:
 clean: clean-am
 
 clean-am: clean-binPROGRAMS clean-generic clean-noinstPROGRAMS \

This started with:
https://gcc.gnu.org/cgit/gcc/commit/?id=f7c8fa7280c85cbdea45be9c09f36123ff16a78a

Inline 'gcc/rust/Make-lang.in:RUST_LDFLAGS' into single user
	gcc/rust/
	* Make-lang.in (RUST_LDFLAGS): Inline into single user.

Which doesn't really make sense, since that doesn't touch gotools at
all...

I am not sure why.

Cheers,

Mark


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 0/6] autoregen.py: Improve support for GCC
  2024-04-14 22:33 ` [PATCH 0/6] autoregen.py: Improve support for GCC Mark Wielaard
@ 2024-04-15 11:52   ` Christophe Lyon
  0 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-15 11:52 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: buildbot

On Mon, 15 Apr 2024 at 00:33, Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Christophe,
>
> On Fri, Apr 12, 2024 at 08:05:53PM +0000, Christophe Lyon wrote:
> > I looked into more details to improve autoregen.py's support for GCC.
> >
> > I noticed that autoreconf works well in most directories, so this
> > patch series adds most GCC subdirs to AUTORECONF_DIRS.
>
> Nice, that simplifies things.
>
> > Following that, I noticed that quite a few more subdirs from
> > binutils-gdb can also be regenerated using autoreconf, so add them to
> > AUTORECONF_DIRS.
>
> Ack.
>
> > In addition, add support for autogen, to regenerate the top-level
> > Makefile.in.
>
> I added autogen to the Containerfile-autotools to make sure it is
> available. I assume the version doesn't matter.

I did not see any mention of a version number in the generated files,
so I assumed it doesn't matter.

>
> > There are still some peculiarities with some subdirs, so there are
> > probably some fixes to make in binutils, gdb and gcc.  We can update
> > this script when that is the case.
> >
> > Also, this script should probably be copied into contrib/ for the
> > benefit of the projects.
>
> Yes, that would be good. Do binutils-gdb/gcc share a contrib dir?
> We could pick up the script from there, we don't necessary need to
> have it in the builder.git itself.

There's a contrib dir in both binutils-gdb and gcc, though gcc's has
many more utilities.
dg-extraact-results.{py,sh} have the same contents, while mklog.py is
not the same in the two repos: the version in binutils-gdb wasn't
updated since it was first copied from the gcc repo.

I suspect the same sync issue would/will happen with autoregen.py, but
still I think it would help developers of both projects to have it in
contrib rather than in builder.git (which I think most people don't
know about).

>
> > Christophe Lyon (6):
> >   auroregen.py: Check AC_CONFIG_MACRO_DIR instead of
> >     AC_CONFIG_MACRO_DIRS
> >   autoregen.py: Move comment...
> >   autoregen.py: Flush all print commands
> >   autoregen.py: Use autoreconf in most GCC directories
> >   autoregen.py: Add support for autogen
> >   autoregen.py: Add binutils directories to AUTORECONF_DIRS
>
> Applied all of them.
>
Thanks!

Christophe

> Thanks,
>
> Mark

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/6] autoregen.py: Flush all print commands
  2024-04-15  2:09   ` Simon Marchi
@ 2024-04-15 11:55     ` Christophe Lyon
  0 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-15 11:55 UTC (permalink / raw)
  To: Simon Marchi; +Cc: buildbot

On Mon, 15 Apr 2024 at 04:09, Simon Marchi <simark@simark.ca> wrote:
>
>
>
> On 2024-04-12 16:05, Christophe Lyon wrote:
> > For consistency and easier debug of traces
> > ---
> >  builder/containers/autoregen.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
> > index 6e341e2..c52a524 100755
> > --- a/builder/containers/autoregen.py
> > +++ b/builder/containers/autoregen.py
> > @@ -56,7 +56,7 @@ AUTORECONF_DIRS = [
> >  #
> >  # Print the command on stdout prior to running it.
> >  def run_shell(cmd: str):
> > -    print(f"+ {cmd}")
> > +    print(f"+ {cmd}", flush=True)
> >      res = subprocess.run(
> >          f"{cmd}",
> >          shell=True,
> > @@ -105,7 +105,7 @@ run_shell(f"{AUTOMAKE_BIN} --version")
> >  run_shell(f"{ACLOCAL_BIN} --version")
> >  run_shell(f"{AUTOHEADER_BIN} --version")
> >
> > -print(f"Extra environment: {EXTRA_ENV}")
> > +print(f"Extra environment: {EXTRA_ENV}", flush=True)
> >
> >  config_folders: list[Path] = []
> >  repo_root = Path.cwd()
>
> Can you clarify when/where/why that's needed?  I would expect the stdout
> stream to be flushed when there is an end-of-line (which print() does).
>

I'm not quite sure now.... I think I was annoyed when looking at
builds in progress on the sourceware buildbot.

> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15 11:42   ` Mark Wielaard
@ 2024-04-15 12:46     ` Mark Wielaard
  2024-04-15 12:56       ` Christophe Lyon
  0 siblings, 1 reply; 21+ messages in thread
From: Mark Wielaard @ 2024-04-15 12:46 UTC (permalink / raw)
  To: Christophe Lyon, buildbot
  Cc: Thomas Schwinge, Ian Lance Taylor, Jakub Jelinek

Hi (adding Jakub and Ian to CC),

On Mon, 2024-04-15 at 13:42 +0200, Mark Wielaard wrote:
> On Fri, 2024-04-12 at 20:05 +0000, Christophe Lyon wrote:
> > Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> > -f' works for them.
> > [...]
> >  # these directories are known to can be re-generatable with a simple autoreconf
> >  # without special -I flags
> >  AUTORECONF_DIRS = [
> > +    # binutils-gdb subdirs
> >      "gdb",
> >      "gdbserver",
> >      "gdbsupport",
> >      "gnulib",
> > +
> > +    # gcc subdirs
> > +    "c++tools", # No aclocal.m4
> > +    #"gcc", # No Makefile.am
> > +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> > +    "gnattools", # No aclocal.m4
> > +    "gotools",
> 
> Oddly this now generates:
> https://builder.sourceware.org/buildbot/#/builders/269/builds/4325
> 
> Entering directory /home/builder/shared/bb3/worker/gcc-
> autoregen/build/gnattools
> + autoreconf-2.69 -f
> 
> [... where before it was just autoconf-2.69 -f ...]
> 
> diff --git a/gotools/Makefile.in b/gotools/Makefile.in
> index 36c2ec2abd3..f40883c39be 100644
> --- a/gotools/Makefile.in
> +++ b/gotools/Makefile.in
> @@ -704,8 +704,8 @@ distclean-generic:
>  maintainer-clean-generic:
>  	@echo "This command is intended for maintainers to use"
>  	@echo "it deletes files that may require special tools to
> rebuild."
> -@NATIVE_FALSE@install-exec-local:
>  @NATIVE_FALSE@uninstall-local:
> +@NATIVE_FALSE@install-exec-local:
>  clean: clean-am
>  
>  clean-am: clean-binPROGRAMS clean-generic clean-noinstPROGRAMS \
> 
> This started with:
> https://gcc.gnu.org/cgit/gcc/commit/?id=f7c8fa7280c85cbdea45be9c09f36123ff16a78a
> 
> Inline 'gcc/rust/Make-lang.in:RUST_LDFLAGS' into single user
> 	gcc/rust/
> 	* Make-lang.in (RUST_LDFLAGS): Inline into single user.
> 
> Which doesn't really make sense, since that doesn't touch gotools at
> all...
> 
> I am not sure why.

Looks like this is just random/non-deterministic.
Jakub has a patch here:
https://inbox.sourceware.org/gcc-patches/Zh0grrP%2FMPu3pXbC@tucnak/T/#u

Cheers,

Mark


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15  3:07   ` Simon Marchi
@ 2024-04-15 12:52     ` Christophe Lyon
  2024-04-15 14:48       ` Simon Marchi
  2024-04-17 14:30       ` Christophe Lyon
  0 siblings, 2 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-15 12:52 UTC (permalink / raw)
  To: Simon Marchi; +Cc: buildbot

On Mon, 15 Apr 2024 at 05:07, Simon Marchi <simark@simark.ca> wrote:
>
>
>
> On 2024-04-12 16:05, Christophe Lyon wrote:
> > Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> > -f' works for them.
> >
> > A few subdirs still need to be regenerated "manually", because they do
> > not have Makefile.am which autoreconf uses to determine which aclocal
> > flags to use.
>
> gdb, for instance, doesn't use Makefile.am, and yet autoreconf works.  It
> finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in
> configure.ac.  If the remaining subdirs that still need to be manually
> handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them?

that's because gdb's aclocal.m4 is generated with 'aclocal' without anyflag.

If you look at gcc/gcc/Makefile.in, you'll see:
ACLOCAL_AMFLAGS = -I ../config -I ..
So to regenerate gcc/gcc/aclocal.m4, we need to call
aclocal  -I ../config -I ..
but autoreconf looks for this info in Makefile.am which does not exist
in this case.
(see line 410 and below in autoreconf 2.69)

I don't know how AC_CONFIG_MACRO_DIRS would help, maybe it would...

> Out of the directories you commented out from AUTORECONF_DIRS, the
> following re-generated cleanly with autoreconf for me:
>
>  - libdecnumber
So -I ../config is not needed by aclocal?

>  - gcc
as mentioned above, here we have ACLOCAL_AMFLAGS = -I ../config -I ..
so it's surprising it works without these flags?

>  - libiberty
>  - libobjc
same as for gcc.

> So I would suggest adding them to AUTORECONF_DIRS.

Well, maybe :-)
I must confess the doc for autoconf is not clear enough for me :-)
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Input.html
does not mention AC_CONFIG_MACRO_DIRS, only AC_CONFIG_MACRO_DIR

And the "Note that if you use aclocal from Automake to generate aclocal.m4[...]"
does not clearly say that AC_CONFIG_MACRO_DIR[S] is enough.
(and the code I saw in autoreconf only checks /^ACLOCAL_[A-Z_]*FLAGS\s*=\s*(.*)
in Makefile.am.

https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html
is more verbose and does mention AC_CONFIG_MACRO_DIRS, but we still use 2.69
so I prefered to stay on the "safe" side.

Maybe worth asking on the autoconf/automake list?

> > Most use our default aclocal -I../config, but a few need special
> > flags, which I copied from the corresponding Makefile.in:
> > * fixincludes: -I.. -I../config
> > * gcc, libiberty, libobjc: -I../config -I..
> > * libgm2: -I../config -I.. although Makefile.in says otherwise. Using
> >   what Makefile.in defines results in generating aclocal.m4 with a
> >   different contents than what it is in the repo. The repo is
> >   presumably wrong, but use this until this is fixed.
> >
> > Note that we do not regenerate
> > libvtv/testsuite/other-tests/Makefile.in, because
> > libvtv/testsuite/other-tests/ has no configure.ac.
> >
> > Running this script over GCC's repository generates quite a few
> > warnings, which are the same as before this patch. Namely, these
> > subdirs seem to have incorrect autotools files (at least partially):
> > * libgfortran
> > * libgomp
> > * libsanitizer
> >
> > This patch does not support regenerating gcc/m2/gm2-libs/config-host
> > and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
> > regenerate them with the exact same content. FTR I tried:
> > autoconf -f config-host.in > config-host
> > autoheader config-host.in
> > as described in gcc/m2/Make-maintainer.in
> >
> > Similarly, we skip libcpp because the files we regenerate have small
> > differences with the current versions.
>
> libcpp's files appear to be in an invalid state in the repo itself.
> According to libcpp/Makefile.in line 123, aclocal.m4 should be
> regenerated with `aclocal -I ../config`.  When I use that, I get the
> same results as what autoreconf would give me.  For that one, I think
> you could send a patch to gcc to fix it in the repo, after which we can
> switch it to use autoreconf.

Yes I noticed a discrepancy about override.m4, but I thought this is
too late in GCC stage4 to fix it.
Maybe I'm wrong on this assumption though :-)

>
> > Tested by removing all aclocal.m4 files, Makefile.in files derived
> > from Makefile.am, configure files derived from configure.ac and
> > checking they are correctly regenerated (that is, 'git status' does
> > not report deleted nor modified files).
> > ---
> >  builder/containers/autoregen.py | 60 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
> > index c52a524..4731a87 100755
> > --- a/builder/containers/autoregen.py
> > +++ b/builder/containers/autoregen.py
> > @@ -40,15 +40,56 @@ SKIP_DIRS = [
> >      # readline and minizip are maintained with different autotools versions
> >      "readline",
> >      "minizip",
> > +
> > +    # aclocal.m4 gets an additional ../config/override.m4,
> > +    # and config.in gets a few more empty lines.
> > +    "libcpp",
> >  ]
> >
> >  # these directories are known to can be re-generatable with a simple autoreconf
> >  # without special -I flags
> >  AUTORECONF_DIRS = [
> > +    # binutils-gdb subdirs
> >      "gdb",
> >      "gdbserver",
> >      "gdbsupport",
> >      "gnulib",
> > +
> > +    # gcc subdirs
> > +    "c++tools", # No aclocal.m4
> > +    #"gcc", # No Makefile.am
> > +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> > +    "gnattools", # No aclocal.m4
> > +    "gotools",
> > +    "libada", # No aclocal.m4
> > +    "libatomic",
> > +    "libbacktrace",
> > +    "libcc1",
> > +    "libcody", # No aclocal.m4
> > +    #"libcpp", # No Makefile.am
> > +    #"libdecnumber", # No Makefile.am
> > +    "libffi",
> > +    "libgcc", # No aclocal.m4
> > +    "libgfortran",
> > +    # Hack: ACLOCAL_AMFLAGS = -I .. -I ../config in Makefile.in but we
> > +    # apply -I../config -I.. otherwise we do not match the current
> > +    # contents
> > +    #"libgm2",
> > +    "libgo",
> > +    "libgomp",
> > +    "libgrust",
> > +    #"libiberty", # No Makefile.am
> > +    "libitm",
> > +    #"libobjc", # No Makefile.am
> > +    "libphobos",
> > +    "libquadmath",
> > +    "libsanitizer",
> > +    "libssp",
> > +    "libstdc++-v3",
> > +    # This does not cover libvtv/testsuite/other-tests/Makefile.in
> > +    "libvtv",
> > +    "lto-plugin",
> > +    "zlib",
>
> If you really want to make the distinction between binutils-gdb and gcc
> subdirs, perhaps add a "common" section for those directories that
> appear in both repos?

OK

>
> >  ]
> >
> >
> > @@ -71,7 +112,9 @@ def regenerate_with_autoreconf():
> >
> >  def regenerate_manually():
> >      configure_lines = open("configure.ac").read().splitlines()
> > -    if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
> > +    if folder.stem == "fixincludes" or folder.stem == "libgm2" or any(
> > +            True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")
> > +    ):
> >          include_arg = ""
> >          include_arg2 = ""
> >          if (folder / ".." / "config").is_dir():
> > @@ -83,6 +126,14 @@ def regenerate_manually():
> >              include_arg = "-I../.."
> >              include_arg2 = "-I../../config"
> >
> > +        if folder.stem == "fixincludes":
> > +            include_arg = "-I.."
> > +            include_arg2 = "-I../config"
> > +
> > +        if folder.stem == "libgm2" or folder.stem == "gcc" or folder.stem == "libiberty" or folder.stem == "libobjc":
>
> This could be a bit shorter:
>
>   if folder.stem in ["libgm2", "gcc", "libiberty", "libobjc"]:
Thanks, too late I think Mark has already pushed my patch as-is :-)
But this can be fixed later.

Thanks,

Christophe


>
> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15 12:46     ` Mark Wielaard
@ 2024-04-15 12:56       ` Christophe Lyon
  0 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-15 12:56 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: buildbot, Thomas Schwinge, Ian Lance Taylor, Jakub Jelinek

On Mon, 15 Apr 2024 at 14:46, Mark Wielaard <mark@klomp.org> wrote:
>
> Hi (adding Jakub and Ian to CC),
>
> On Mon, 2024-04-15 at 13:42 +0200, Mark Wielaard wrote:
> > On Fri, 2024-04-12 at 20:05 +0000, Christophe Lyon wrote:
> > > Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> > > -f' works for them.
> > > [...]
> > >  # these directories are known to can be re-generatable with a simple autoreconf
> > >  # without special -I flags
> > >  AUTORECONF_DIRS = [
> > > +    # binutils-gdb subdirs
> > >      "gdb",
> > >      "gdbserver",
> > >      "gdbsupport",
> > >      "gnulib",
> > > +
> > > +    # gcc subdirs
> > > +    "c++tools", # No aclocal.m4
> > > +    #"gcc", # No Makefile.am
> > > +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> > > +    "gnattools", # No aclocal.m4
> > > +    "gotools",
> >
> > Oddly this now generates:
> > https://builder.sourceware.org/buildbot/#/builders/269/builds/4325
> >
> > Entering directory /home/builder/shared/bb3/worker/gcc-
> > autoregen/build/gnattools
> > + autoreconf-2.69 -f
> >
> > [... where before it was just autoconf-2.69 -f ...]
> >
> > diff --git a/gotools/Makefile.in b/gotools/Makefile.in
> > index 36c2ec2abd3..f40883c39be 100644
> > --- a/gotools/Makefile.in
> > +++ b/gotools/Makefile.in
> > @@ -704,8 +704,8 @@ distclean-generic:
> >  maintainer-clean-generic:
> >       @echo "This command is intended for maintainers to use"
> >       @echo "it deletes files that may require special tools to
> > rebuild."
> > -@NATIVE_FALSE@install-exec-local:
> >  @NATIVE_FALSE@uninstall-local:
> > +@NATIVE_FALSE@install-exec-local:
> >  clean: clean-am
> >
> >  clean-am: clean-binPROGRAMS clean-generic clean-noinstPROGRAMS \
> >
> > This started with:
> > https://gcc.gnu.org/cgit/gcc/commit/?id=f7c8fa7280c85cbdea45be9c09f36123ff16a78a
> >
> > Inline 'gcc/rust/Make-lang.in:RUST_LDFLAGS' into single user
> >       gcc/rust/
> >       * Make-lang.in (RUST_LDFLAGS): Inline into single user.
> >
> > Which doesn't really make sense, since that doesn't touch gotools at
> > all...
> >
> > I am not sure why.
>
> Looks like this is just random/non-deterministic.
> Jakub has a patch here:
> https://inbox.sourceware.org/gcc-patches/Zh0grrP%2FMPu3pXbC@tucnak/T/#u
>

Ha! Thanks for looking at this, I had noticed a problem a while back
when working on autoregen.py but couldn't reproduce it anymore lately.

Thanks,

Christophe

> Cheers,
>
> Mark
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15 12:52     ` Christophe Lyon
@ 2024-04-15 14:48       ` Simon Marchi
  2024-04-16 14:47         ` Christophe Lyon
  2024-04-17 14:30       ` Christophe Lyon
  1 sibling, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2024-04-15 14:48 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: buildbot

On 4/15/24 8:52 AM, Christophe Lyon wrote:
> On Mon, 15 Apr 2024 at 05:07, Simon Marchi <simark@simark.ca> wrote:
>>
>>
>>
>> On 2024-04-12 16:05, Christophe Lyon wrote:
>>> Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
>>> -f' works for them.
>>>
>>> A few subdirs still need to be regenerated "manually", because they do
>>> not have Makefile.am which autoreconf uses to determine which aclocal
>>> flags to use.
>>
>> gdb, for instance, doesn't use Makefile.am, and yet autoreconf works.  It
>> finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in
>> configure.ac.  If the remaining subdirs that still need to be manually
>> handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them?
> 
> that's because gdb's aclocal.m4 is generated with 'aclocal' without anyflag.
> 
> If you look at gcc/gcc/Makefile.in, you'll see:
> ACLOCAL_AMFLAGS = -I ../config -I ..
> So to regenerate gcc/gcc/aclocal.m4, we need to call
> aclocal  -I ../config -I ..
> but autoreconf looks for this info in Makefile.am which does not exist
> in this case.
> (see line 410 and below in autoreconf 2.69)

I don't think aclocal gets the macro include paths from Makefile.am.  It
gets it from configure.ac's AC_CONFIG_MACRO_DIRS.  See the doc for
AC_CONFIG_MACRO_DIRS:

https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html#index-AC_005fCONFIG_005fMACRO_005fDIRS-1

    Specify the given directories as the location of additional local
    Autoconf macros. These macros are intended for use by commands like
    autoreconf or aclocal that trace macro calls; they should be called
    directly from configure.ac so that tools that install macros for
    aclocal can find the macros’ declarations.

If I call `aclocal` without args in gcc/gcc, it re-generates aclocal.m4
fine, because it got the include path info from configure.ac.  If I
remove the `AC_CONFIG_MACRO_DIRS` line from gcc/gcc/configure.ac and run
aclocal.m4, then I am missing a bunch of m4 files in the resulting
aclocal.m4.  If I remove the `ACLOCAL_AMFLAGS` variable from
gcc/gcc/Makefile.am, it has no effect on `aclocal`.  That variable only
exists for the benefit of the Makefile rule that regenerates aclocal.m4,
lower in Makefile.am.  But I would argue that it's unnecessary, since
that info is encoded in configure.ac, so the rule could call aclocal
without any -I args.

> I don't know how AC_CONFIG_MACRO_DIRS would help, maybe it would...
> 
>> Out of the directories you commented out from AUTORECONF_DIRS, the
>> following re-generated cleanly with autoreconf for me:
>>
>>  - libdecnumber
> So -I ../config is not needed by aclocal?
> 
>>  - gcc
> as mentioned above, here we have ACLOCAL_AMFLAGS = -I ../config -I ..
> so it's surprising it works without these flags?
> 
>>  - libiberty
>>  - libobjc
> same as for gcc.

Same answer as above, aclocal reads AC_CONFIG_MACRO_DIRS.

> 
>> So I would suggest adding them to AUTORECONF_DIRS.
> 
> Well, maybe :-)
> I must confess the doc for autoconf is not clear enough for me :-)
> https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Input.html
> does not mention AC_CONFIG_MACRO_DIRS, only AC_CONFIG_MACRO_DIR
> 
> And the "Note that if you use aclocal from Automake to generate aclocal.m4[...]"
> does not clearly say that AC_CONFIG_MACRO_DIR[S] is enough.
> (and the code I saw in autoreconf only checks /^ACLOCAL_[A-Z_]*FLAGS\s*=\s*(.*)
> in Makefile.am.
> 
> https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html
> is more verbose and does mention AC_CONFIG_MACRO_DIRS, but we still use 2.69
> so I prefered to stay on the "safe" side.
> 
> Maybe worth asking on the autoconf/automake list?

I had to dig a bit to understand, because autoconf 2.69 indeed does not
mention AC_CONFIG_MACRO_DIRS.  The answer is that automake 1.15.1 ships
with this for "older" autoconfs - like 2.69 - that don't provide
AC_CONFIG_MACRO_DIRS:

https://git.savannah.gnu.org/cgit/automake.git/tree/m4/internal/ac-config-macro-dirs.m4?h=v1.15.1

The macro doesn't actually do anything, it's just some kind of marker
that aclocal looks for to get the paths.  If we use AC_CONFIG_MACRO_DIRS
throughout the binutils-gdb and gcc repositories, it must be because it
works with autoconf 2.69 somehow, because we never used an autoconf more
recent than 2.69.

> 
>>> Most use our default aclocal -I../config, but a few need special
>>> flags, which I copied from the corresponding Makefile.in:
>>> * fixincludes: -I.. -I../config
>>> * gcc, libiberty, libobjc: -I../config -I..
>>> * libgm2: -I../config -I.. although Makefile.in says otherwise. Using
>>>   what Makefile.in defines results in generating aclocal.m4 with a
>>>   different contents than what it is in the repo. The repo is
>>>   presumably wrong, but use this until this is fixed.
>>>
>>> Note that we do not regenerate
>>> libvtv/testsuite/other-tests/Makefile.in, because
>>> libvtv/testsuite/other-tests/ has no configure.ac.
>>>
>>> Running this script over GCC's repository generates quite a few
>>> warnings, which are the same as before this patch. Namely, these
>>> subdirs seem to have incorrect autotools files (at least partially):
>>> * libgfortran
>>> * libgomp
>>> * libsanitizer
>>>
>>> This patch does not support regenerating gcc/m2/gm2-libs/config-host
>>> and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
>>> regenerate them with the exact same content. FTR I tried:
>>> autoconf -f config-host.in > config-host
>>> autoheader config-host.in
>>> as described in gcc/m2/Make-maintainer.in
>>>
>>> Similarly, we skip libcpp because the files we regenerate have small
>>> differences with the current versions.
>>
>> libcpp's files appear to be in an invalid state in the repo itself.
>> According to libcpp/Makefile.in line 123, aclocal.m4 should be
>> regenerated with `aclocal -I ../config`.  When I use that, I get the
>> same results as what autoreconf would give me.  For that one, I think
>> you could send a patch to gcc to fix it in the repo, after which we can
>> switch it to use autoreconf.
> 
> Yes I noticed a discrepancy about override.m4, but I thought this is
> too late in GCC stage4 to fix it.
> Maybe I'm wrong on this assumption though :-)

IMO it's worth asking, it could be considered a bug to not have the
files correctly re-generated.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15 14:48       ` Simon Marchi
@ 2024-04-16 14:47         ` Christophe Lyon
  2024-04-16 15:32           ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Christophe Lyon @ 2024-04-16 14:47 UTC (permalink / raw)
  To: Simon Marchi; +Cc: buildbot

On Mon, 15 Apr 2024 at 16:48, Simon Marchi <simark@simark.ca> wrote:
>
> On 4/15/24 8:52 AM, Christophe Lyon wrote:
> > On Mon, 15 Apr 2024 at 05:07, Simon Marchi <simark@simark.ca> wrote:
> >>
> >>
> >>
> >> On 2024-04-12 16:05, Christophe Lyon wrote:
> >>> Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> >>> -f' works for them.
> >>>
> >>> A few subdirs still need to be regenerated "manually", because they do
> >>> not have Makefile.am which autoreconf uses to determine which aclocal
> >>> flags to use.
> >>
> >> gdb, for instance, doesn't use Makefile.am, and yet autoreconf works.  It
> >> finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in
> >> configure.ac.  If the remaining subdirs that still need to be manually
> >> handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them?
> >
> > that's because gdb's aclocal.m4 is generated with 'aclocal' without anyflag.
> >
> > If you look at gcc/gcc/Makefile.in, you'll see:
> > ACLOCAL_AMFLAGS = -I ../config -I ..
> > So to regenerate gcc/gcc/aclocal.m4, we need to call
> > aclocal  -I ../config -I ..
> > but autoreconf looks for this info in Makefile.am which does not exist
> > in this case.
> > (see line 410 and below in autoreconf 2.69)
>
> I don't think aclocal gets the macro include paths from Makefile.am.  It
That's not what I am saying: it's autoreconf that gets aclocal_flags
from Makefile.am

> gets it from configure.ac's AC_CONFIG_MACRO_DIRS.  See the doc for
> AC_CONFIG_MACRO_DIRS:
>
> https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html#index-AC_005fCONFIG_005fMACRO_005fDIRS-1
>
>     Specify the given directories as the location of additional local
>     Autoconf macros. These macros are intended for use by commands like
>     autoreconf or aclocal that trace macro calls; they should be called
>     directly from configure.ac so that tools that install macros for
>     aclocal can find the macros’ declarations.
Indeed, but that's "slightly" different from what I meant :-)

> If I call `aclocal` without args in gcc/gcc, it re-generates aclocal.m4
> fine, because it got the include path info from configure.ac.  If I
> remove the `AC_CONFIG_MACRO_DIRS` line from gcc/gcc/configure.ac and run
> aclocal.m4, then I am missing a bunch of m4 files in the resulting
> aclocal.m4.  If I remove the `ACLOCAL_AMFLAGS` variable from
> gcc/gcc/Makefile.am, it has no effect on `aclocal`.  That variable only
> exists for the benefit of the Makefile rule that regenerates aclocal.m4,
> lower in Makefile.am.  But I would argue that it's unnecessary, since
> that info is encoded in configure.ac, so the rule could call aclocal
> without any -I args.

Of course: aclocal does NOT read Makefile.am, but autoreconf does.

So what I noticed for the 'gcc' subdir is that autoreconf will
generate the same contents but also print a few warnings.
This is because when calling aclocal with some -I flags, the contents
of those included .m4 files is taken into account before scanning
configure.ac, so macros like AM_PATH_PROG_WITH_TEST are defined before
being used, while when using autoreconf (thus calling aclocal without
any -I flag), those .m4 files are processed later, leading to warnings
that AM_PATH_PROG_WITH_TEST is "not found in library". IIUC, there's
an iterative process which means that things are re-assessed later.

So.... it seems it's not a real problem to use autoreconf as you
suggest, but we'll see such warnings in the buildbot logs (not causing
errors IIUC).

Maybe using explicit m4_include(....) would avoid this discrepancy but
I'm not sure that's desirable.

>
> > I don't know how AC_CONFIG_MACRO_DIRS would help, maybe it would...
> >
> >> Out of the directories you commented out from AUTORECONF_DIRS, the
> >> following re-generated cleanly with autoreconf for me:
> >>
> >>  - libdecnumber
> > So -I ../config is not needed by aclocal?
> >
> >>  - gcc
> > as mentioned above, here we have ACLOCAL_AMFLAGS = -I ../config -I ..
> > so it's surprising it works without these flags?
> >
> >>  - libiberty
> >>  - libobjc
> > same as for gcc.
>
> Same answer as above, aclocal reads AC_CONFIG_MACRO_DIRS.
>
> >
> >> So I would suggest adding them to AUTORECONF_DIRS.
> >
> > Well, maybe :-)
> > I must confess the doc for autoconf is not clear enough for me :-)
> > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Input.html
> > does not mention AC_CONFIG_MACRO_DIRS, only AC_CONFIG_MACRO_DIR
> >
> > And the "Note that if you use aclocal from Automake to generate aclocal.m4[...]"
> > does not clearly say that AC_CONFIG_MACRO_DIR[S] is enough.
> > (and the code I saw in autoreconf only checks /^ACLOCAL_[A-Z_]*FLAGS\s*=\s*(.*)
> > in Makefile.am.
> >
> > https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html
> > is more verbose and does mention AC_CONFIG_MACRO_DIRS, but we still use 2.69
> > so I prefered to stay on the "safe" side.
> >
> > Maybe worth asking on the autoconf/automake list?
>
> I had to dig a bit to understand, because autoconf 2.69 indeed does not
> mention AC_CONFIG_MACRO_DIRS.  The answer is that automake 1.15.1 ships
> with this for "older" autoconfs - like 2.69 - that don't provide
> AC_CONFIG_MACRO_DIRS:
>
> https://git.savannah.gnu.org/cgit/automake.git/tree/m4/internal/ac-config-macro-dirs.m4?h=v1.15.1
>
> The macro doesn't actually do anything, it's just some kind of marker
> that aclocal looks for to get the paths.  If we use AC_CONFIG_MACRO_DIRS
> throughout the binutils-gdb and gcc repositories, it must be because it
> works with autoconf 2.69 somehow, because we never used an autoconf more
> recent than 2.69.
>
> >
> >>> Most use our default aclocal -I../config, but a few need special
> >>> flags, which I copied from the corresponding Makefile.in:
> >>> * fixincludes: -I.. -I../config
> >>> * gcc, libiberty, libobjc: -I../config -I..
> >>> * libgm2: -I../config -I.. although Makefile.in says otherwise. Using
> >>>   what Makefile.in defines results in generating aclocal.m4 with a
> >>>   different contents than what it is in the repo. The repo is
> >>>   presumably wrong, but use this until this is fixed.
> >>>
> >>> Note that we do not regenerate
> >>> libvtv/testsuite/other-tests/Makefile.in, because
> >>> libvtv/testsuite/other-tests/ has no configure.ac.
> >>>
> >>> Running this script over GCC's repository generates quite a few
> >>> warnings, which are the same as before this patch. Namely, these
> >>> subdirs seem to have incorrect autotools files (at least partially):
> >>> * libgfortran
> >>> * libgomp
> >>> * libsanitizer
> >>>
> >>> This patch does not support regenerating gcc/m2/gm2-libs/config-host
> >>> and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
> >>> regenerate them with the exact same content. FTR I tried:
> >>> autoconf -f config-host.in > config-host
> >>> autoheader config-host.in
> >>> as described in gcc/m2/Make-maintainer.in
> >>>
> >>> Similarly, we skip libcpp because the files we regenerate have small
> >>> differences with the current versions.
> >>
> >> libcpp's files appear to be in an invalid state in the repo itself.
> >> According to libcpp/Makefile.in line 123, aclocal.m4 should be
> >> regenerated with `aclocal -I ../config`.  When I use that, I get the
> >> same results as what autoreconf would give me.  For that one, I think
> >> you could send a patch to gcc to fix it in the repo, after which we can
> >> switch it to use autoreconf.
> >
> > Yes I noticed a discrepancy about override.m4, but I thought this is
> > too late in GCC stage4 to fix it.
> > Maybe I'm wrong on this assumption though :-)
>
> IMO it's worth asking, it could be considered a bug to not have the
> files correctly re-generated.
>
> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-16 14:47         ` Christophe Lyon
@ 2024-04-16 15:32           ` Simon Marchi
  2024-04-16 16:12             ` Christophe Lyon
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2024-04-16 15:32 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: buildbot



On 2024-04-16 10:47, Christophe Lyon wrote:
>> If I call `aclocal` without args in gcc/gcc, it re-generates aclocal.m4
>> fine, because it got the include path info from configure.ac.  If I
>> remove the `AC_CONFIG_MACRO_DIRS` line from gcc/gcc/configure.ac and run
>> aclocal.m4, then I am missing a bunch of m4 files in the resulting
>> aclocal.m4.  If I remove the `ACLOCAL_AMFLAGS` variable from
>> gcc/gcc/Makefile.am, it has no effect on `aclocal`.  That variable only

Sorry, I wrote Makefile.am here, it should be Makefile.in.

>> exists for the benefit of the Makefile rule that regenerates aclocal.m4,
>> lower in Makefile.am.  But I would argue that it's unnecessary, since
>> that info is encoded in configure.ac, so the rule could call aclocal
>> without any -I args.
> 
> Of course: aclocal does NOT read Makefile.am, but autoreconf does.

Ok, I wasn't aware of autoreconf getting flags for aclocal from
ACLOCAL_AMFLAGS in Makefile.am, now I see it in the docs.  But it seems
to me like autoreconf will "just work" in most cases then:

 - for directories with a Makefile.am, autoreconf will read the -I flags
   specified in Makefile.am/ACLOCAL_AMFLAGS and pass them to aclocal.
 - for directories without a Makefile.am (like GDB), autoreconf will
   call aclocal without -I flags, but aclocal will use the include paths
   specified in configure.ac/AC_CONFIG_MACRO_DIRS.

> So what I noticed for the 'gcc' subdir is that autoreconf will
> generate the same contents but also print a few warnings.
> This is because when calling aclocal with some -I flags, the contents
> of those included .m4 files is taken into account before scanning
> configure.ac, so macros like AM_PATH_PROG_WITH_TEST are defined before
> being used, while when using autoreconf (thus calling aclocal without
> any -I flag), those .m4 files are processed later, leading to warnings
> that AM_PATH_PROG_WITH_TEST is "not found in library". IIUC, there's
> an iterative process which means that things are re-assessed later.
> 
> So.... it seems it's not a real problem to use autoreconf as you
> suggest, but we'll see such warnings in the buildbot logs (not causing
> errors IIUC).

I don't really know how this is supposed to work then, it seems
illogical to me.  aclocal recommends using AC_CONFIG_MACRO_DIRS:

  https://www.gnu.org/software/automake/manual/html_node/Local-Macros.html

So it seems to me like it should consider the macros found in these
directories when processing the rest of the files...

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-16 15:32           ` Simon Marchi
@ 2024-04-16 16:12             ` Christophe Lyon
  0 siblings, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-16 16:12 UTC (permalink / raw)
  To: Simon Marchi; +Cc: buildbot

On Tue, 16 Apr 2024 at 17:33, Simon Marchi <simark@simark.ca> wrote:
>
>
>
> On 2024-04-16 10:47, Christophe Lyon wrote:
> >> If I call `aclocal` without args in gcc/gcc, it re-generates aclocal.m4
> >> fine, because it got the include path info from configure.ac.  If I
> >> remove the `AC_CONFIG_MACRO_DIRS` line from gcc/gcc/configure.ac and run
> >> aclocal.m4, then I am missing a bunch of m4 files in the resulting
> >> aclocal.m4.  If I remove the `ACLOCAL_AMFLAGS` variable from
> >> gcc/gcc/Makefile.am, it has no effect on `aclocal`.  That variable only
>
> Sorry, I wrote Makefile.am here, it should be Makefile.in.
>
> >> exists for the benefit of the Makefile rule that regenerates aclocal.m4,
> >> lower in Makefile.am.  But I would argue that it's unnecessary, since
> >> that info is encoded in configure.ac, so the rule could call aclocal
> >> without any -I args.
> >
> > Of course: aclocal does NOT read Makefile.am, but autoreconf does.
>
> Ok, I wasn't aware of autoreconf getting flags for aclocal from
> ACLOCAL_AMFLAGS in Makefile.am, now I see it in the docs.  But it seems
> to me like autoreconf will "just work" in most cases then:
>
>  - for directories with a Makefile.am, autoreconf will read the -I flags
>    specified in Makefile.am/ACLOCAL_AMFLAGS and pass them to aclocal.
>  - for directories without a Makefile.am (like GDB), autoreconf will
>    call aclocal without -I flags, but aclocal will use the include paths
>    specified in configure.ac/AC_CONFIG_MACRO_DIRS.
>
yes

> > So what I noticed for the 'gcc' subdir is that autoreconf will
> > generate the same contents but also print a few warnings.
> > This is because when calling aclocal with some -I flags, the contents
> > of those included .m4 files is taken into account before scanning
> > configure.ac, so macros like AM_PATH_PROG_WITH_TEST are defined before
> > being used, while when using autoreconf (thus calling aclocal without
> > any -I flag), those .m4 files are processed later, leading to warnings
> > that AM_PATH_PROG_WITH_TEST is "not found in library". IIUC, there's
> > an iterative process which means that things are re-assessed later.
> >
> > So.... it seems it's not a real problem to use autoreconf as you
> > suggest, but we'll see such warnings in the buildbot logs (not causing
> > errors IIUC).
>
> I don't really know how this is supposed to work then, it seems
> illogical to me.  aclocal recommends using AC_CONFIG_MACRO_DIRS:
>
>   https://www.gnu.org/software/automake/manual/html_node/Local-Macros.html
>
> So it seems to me like it should consider the macros found in these
> directories when processing the rest of the files...
>

Agreed, this is not super clear :-)
I added some traces to my aclocal.
When called with -I../config, the main loop calls scan_configure which
calls scan_configure_dep on:
acinclude.m4
configure.ac
../config/acx.m4
../config/picflag.m4
etc...
automake-1.15.1/build/../install/share/aclocal-1.15/cond.m4
automake-1.15.1/build/../install/share/aclocal-1.15/substnot.m4
../config/zlib.m4
../config/gcc-plugin.m4
../config/cet.m4
../config/enable.m4

then a 2nd iteration adds:
../lt~obsolete.m4
../libtool.m4
../ltoptions.m4
../ltsugar.m4
../ltversion.m4
../ltgcc.m4


When called by autoreconf (thus without -I), the main loop calls
scan_configure which calls scan_configure_dep on:
acinclude.m4
configure.ac (and warns about AM_PATH_PROG_WITH_TEST AM_ICONV
AM_LC_MESSAGES and AM_LANGINFO_CODESET)
automake-1.15.1/build/../install/share/aclocal-1.15/cond.m4
automake-1.15.1/build/../install/share/aclocal-1.15/substnot.m4
(warns about AM_ZLIB)
then a 2nd iteration adds:
../config/acx.m4
../config/picflag.m4
etc...


Not sure how to understand that from the documentation....







> Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
  2024-04-15 12:52     ` Christophe Lyon
  2024-04-15 14:48       ` Simon Marchi
@ 2024-04-17 14:30       ` Christophe Lyon
  1 sibling, 0 replies; 21+ messages in thread
From: Christophe Lyon @ 2024-04-17 14:30 UTC (permalink / raw)
  To: Simon Marchi; +Cc: buildbot

On Mon, 15 Apr 2024 at 14:52, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> On Mon, 15 Apr 2024 at 05:07, Simon Marchi <simark@simark.ca> wrote:
> >
> >
> >
> > On 2024-04-12 16:05, Christophe Lyon wrote:
> > > Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autoreconf
> > > -f' works for them.
> > >
> > > A few subdirs still need to be regenerated "manually", because they do
> > > not have Makefile.am which autoreconf uses to determine which aclocal
> > > flags to use.
> >
> > gdb, for instance, doesn't use Makefile.am, and yet autoreconf works.  It
> > finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in
> > configure.ac.  If the remaining subdirs that still need to be manually
> > handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them?
>
> that's because gdb's aclocal.m4 is generated with 'aclocal' without anyflag.
>
> If you look at gcc/gcc/Makefile.in, you'll see:
> ACLOCAL_AMFLAGS = -I ../config -I ..
> So to regenerate gcc/gcc/aclocal.m4, we need to call
> aclocal  -I ../config -I ..
> but autoreconf looks for this info in Makefile.am which does not exist
> in this case.
> (see line 410 and below in autoreconf 2.69)
>
> I don't know how AC_CONFIG_MACRO_DIRS would help, maybe it would...
>
> > Out of the directories you commented out from AUTORECONF_DIRS, the
> > following re-generated cleanly with autoreconf for me:
> >
> >  - libdecnumber
> So -I ../config is not needed by aclocal?
>
> >  - gcc
> as mentioned above, here we have ACLOCAL_AMFLAGS = -I ../config -I ..
> so it's surprising it works without these flags?
>
> >  - libiberty
> >  - libobjc
> same as for gcc.
>
> > So I would suggest adding them to AUTORECONF_DIRS.
>
> Well, maybe :-)
> I must confess the doc for autoconf is not clear enough for me :-)
> https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Input.html
> does not mention AC_CONFIG_MACRO_DIRS, only AC_CONFIG_MACRO_DIR
>
> And the "Note that if you use aclocal from Automake to generate aclocal.m4[...]"
> does not clearly say that AC_CONFIG_MACRO_DIR[S] is enough.
> (and the code I saw in autoreconf only checks /^ACLOCAL_[A-Z_]*FLAGS\s*=\s*(.*)
> in Makefile.am.
>
> https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Input.html
> is more verbose and does mention AC_CONFIG_MACRO_DIRS, but we still use 2.69
> so I prefered to stay on the "safe" side.
>
> Maybe worth asking on the autoconf/automake list?
>
> > > Most use our default aclocal -I../config, but a few need special
> > > flags, which I copied from the corresponding Makefile.in:
> > > * fixincludes: -I.. -I../config
> > > * gcc, libiberty, libobjc: -I../config -I..
> > > * libgm2: -I../config -I.. although Makefile.in says otherwise. Using
> > >   what Makefile.in defines results in generating aclocal.m4 with a
> > >   different contents than what it is in the repo. The repo is
> > >   presumably wrong, but use this until this is fixed.
> > >
> > > Note that we do not regenerate
> > > libvtv/testsuite/other-tests/Makefile.in, because
> > > libvtv/testsuite/other-tests/ has no configure.ac.
> > >
> > > Running this script over GCC's repository generates quite a few
> > > warnings, which are the same as before this patch. Namely, these
> > > subdirs seem to have incorrect autotools files (at least partially):
> > > * libgfortran
> > > * libgomp
> > > * libsanitizer
> > >
> > > This patch does not support regenerating gcc/m2/gm2-libs/config-host
> > > and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to
> > > regenerate them with the exact same content. FTR I tried:
> > > autoconf -f config-host.in > config-host
> > > autoheader config-host.in
> > > as described in gcc/m2/Make-maintainer.in
> > >
> > > Similarly, we skip libcpp because the files we regenerate have small
> > > differences with the current versions.
> >
> > libcpp's files appear to be in an invalid state in the repo itself.
> > According to libcpp/Makefile.in line 123, aclocal.m4 should be
> > regenerated with `aclocal -I ../config`.  When I use that, I get the
> > same results as what autoreconf would give me.  For that one, I think
> > you could send a patch to gcc to fix it in the repo, after which we can
> > switch it to use autoreconf.
>
> Yes I noticed a discrepancy about override.m4, but I thought this is
> too late in GCC stage4 to fix it.
> Maybe I'm wrong on this assumption though :-)

So I was wrong :-)
I fixed the libcpp files, so we can now use autoreconf for it too.

Thanks,

Christophe

>
> >
> > > Tested by removing all aclocal.m4 files, Makefile.in files derived
> > > from Makefile.am, configure files derived from configure.ac and
> > > checking they are correctly regenerated (that is, 'git status' does
> > > not report deleted nor modified files).
> > > ---
> > >  builder/containers/autoregen.py | 60 ++++++++++++++++++++++++++++++++-
> > >  1 file changed, 59 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/builder/containers/autoregen.py b/builder/containers/autoregen.py
> > > index c52a524..4731a87 100755
> > > --- a/builder/containers/autoregen.py
> > > +++ b/builder/containers/autoregen.py
> > > @@ -40,15 +40,56 @@ SKIP_DIRS = [
> > >      # readline and minizip are maintained with different autotools versions
> > >      "readline",
> > >      "minizip",
> > > +
> > > +    # aclocal.m4 gets an additional ../config/override.m4,
> > > +    # and config.in gets a few more empty lines.
> > > +    "libcpp",
> > >  ]
> > >
> > >  # these directories are known to can be re-generatable with a simple autoreconf
> > >  # without special -I flags
> > >  AUTORECONF_DIRS = [
> > > +    # binutils-gdb subdirs
> > >      "gdb",
> > >      "gdbserver",
> > >      "gdbsupport",
> > >      "gnulib",
> > > +
> > > +    # gcc subdirs
> > > +    "c++tools", # No aclocal.m4
> > > +    #"gcc", # No Makefile.am
> > > +    #"fixincludes", # autoreconf complains about GCC_AC_FUNC_MMAP_BLACKLIST
> > > +    "gnattools", # No aclocal.m4
> > > +    "gotools",
> > > +    "libada", # No aclocal.m4
> > > +    "libatomic",
> > > +    "libbacktrace",
> > > +    "libcc1",
> > > +    "libcody", # No aclocal.m4
> > > +    #"libcpp", # No Makefile.am
> > > +    #"libdecnumber", # No Makefile.am
> > > +    "libffi",
> > > +    "libgcc", # No aclocal.m4
> > > +    "libgfortran",
> > > +    # Hack: ACLOCAL_AMFLAGS = -I .. -I ../config in Makefile.in but we
> > > +    # apply -I../config -I.. otherwise we do not match the current
> > > +    # contents
> > > +    #"libgm2",
> > > +    "libgo",
> > > +    "libgomp",
> > > +    "libgrust",
> > > +    #"libiberty", # No Makefile.am
> > > +    "libitm",
> > > +    #"libobjc", # No Makefile.am
> > > +    "libphobos",
> > > +    "libquadmath",
> > > +    "libsanitizer",
> > > +    "libssp",
> > > +    "libstdc++-v3",
> > > +    # This does not cover libvtv/testsuite/other-tests/Makefile.in
> > > +    "libvtv",
> > > +    "lto-plugin",
> > > +    "zlib",
> >
> > If you really want to make the distinction between binutils-gdb and gcc
> > subdirs, perhaps add a "common" section for those directories that
> > appear in both repos?
>
> OK
>
> >
> > >  ]
> > >
> > >
> > > @@ -71,7 +112,9 @@ def regenerate_with_autoreconf():
> > >
> > >  def regenerate_manually():
> > >      configure_lines = open("configure.ac").read().splitlines()
> > > -    if any(True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")):
> > > +    if folder.stem == "fixincludes" or folder.stem == "libgm2" or any(
> > > +            True for line in configure_lines if line.startswith("AC_CONFIG_MACRO_DIR")
> > > +    ):
> > >          include_arg = ""
> > >          include_arg2 = ""
> > >          if (folder / ".." / "config").is_dir():
> > > @@ -83,6 +126,14 @@ def regenerate_manually():
> > >              include_arg = "-I../.."
> > >              include_arg2 = "-I../../config"
> > >
> > > +        if folder.stem == "fixincludes":
> > > +            include_arg = "-I.."
> > > +            include_arg2 = "-I../config"
> > > +
> > > +        if folder.stem == "libgm2" or folder.stem == "gcc" or folder.stem == "libiberty" or folder.stem == "libobjc":
> >
> > This could be a bit shorter:
> >
> >   if folder.stem in ["libgm2", "gcc", "libiberty", "libobjc"]:
> Thanks, too late I think Mark has already pushed my patch as-is :-)
> But this can be fixed later.
>
> Thanks,
>
> Christophe
>
>
> >
> > Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2024-04-17 14:30 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-12 20:05 [PATCH 0/6] autoregen.py: Improve support for GCC Christophe Lyon
2024-04-12 20:05 ` [PATCH 1/6] auroregen.py: Check AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS Christophe Lyon
2024-04-12 20:05 ` [PATCH 2/6] autoregen.py: Move comment Christophe Lyon
2024-04-12 20:05 ` [PATCH 3/6] autoregen.py: Flush all print commands Christophe Lyon
2024-04-15  2:09   ` Simon Marchi
2024-04-15 11:55     ` Christophe Lyon
2024-04-12 20:05 ` [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories Christophe Lyon
2024-04-15  3:07   ` Simon Marchi
2024-04-15 12:52     ` Christophe Lyon
2024-04-15 14:48       ` Simon Marchi
2024-04-16 14:47         ` Christophe Lyon
2024-04-16 15:32           ` Simon Marchi
2024-04-16 16:12             ` Christophe Lyon
2024-04-17 14:30       ` Christophe Lyon
2024-04-15 11:42   ` Mark Wielaard
2024-04-15 12:46     ` Mark Wielaard
2024-04-15 12:56       ` Christophe Lyon
2024-04-12 20:05 ` [PATCH 5/6] autoregen.py: Add support for autogen Christophe Lyon
2024-04-12 20:05 ` [PATCH 6/6] autoregen.py: Add binutils directories to AUTORECONF_DIRS Christophe Lyon
2024-04-14 22:33 ` [PATCH 0/6] autoregen.py: Improve support for GCC Mark Wielaard
2024-04-15 11:52   ` Christophe Lyon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).