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

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

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