Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories

[Christophe Lyon <christophe.lyon@linaro.org>]

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