public inbox for buildbot@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: buildbot@sourceware.org
Subject: Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories
Date: Mon, 15 Apr 2024 10:48:14 -0400	[thread overview]
Message-ID: <e92dbf56-d19a-4be9-99af-ea7c00c37778@simark.ca> (raw)
In-Reply-To: <CAPS5khY=b8qMQrd7Mr5T9yv49qqN6KP8t91tQOe3fOHp8+6eHQ@mail.gmail.com>

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

  reply	other threads:[~2024-04-15 14:48 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e92dbf56-d19a-4be9-99af-ea7c00c37778@simark.ca \
    --to=simark@simark.ca \
    --cc=buildbot@sourceware.org \
    --cc=christophe.lyon@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).