From: Christophe Lyon <christophe.lyon@linaro.org>
To: Simon Marchi <simark@simark.ca>
Cc: binutils@sourceware.org, gdb@sourceware.org, gcc@gcc.gnu.org
Subject: Re: [RFC] add regenerate Makefile target
Date: Mon, 8 Apr 2024 11:22:04 +0200 [thread overview]
Message-ID: <CAPS5kha7axXbD=atJNOj-VJpiXUp88K7QgRyS0ZdC_qutvF7rw@mail.gmail.com> (raw)
In-Reply-To: <CAPS5kha=uhwO1=-74BrOKAw_+q0HOpmJSLBaAqdXgBbZXzqdGQ@mail.gmail.com>
Hi,
On Mon, 25 Mar 2024 at 15:19, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> On Thu, 21 Mar 2024 at 15:32, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
> >
> > On Wed, 20 Mar 2024 at 16:34, Simon Marchi <simark@simark.ca> wrote:
> > >
> > > On 3/18/24 13:25, Christophe Lyon wrote:
> > > > Well the rule to regenerate Makefile.in (eg in in opcodes/) is a bit
> > > > more complex
> > > > than just calling automake. IIUC it calls automake --foreign it any of
> > > > *.m4 file from $(am__configure_deps) that is newer than Makefile.in
> > > > (with an early exit in the loop), does nothing if Makefile.am or
> > > > doc/local.mk are newer than Makefile.in, and then calls 'automake
> > > > --foreign Makefile'
> > >
> > > The rules looks complex because they've been generated by automake, this
> > > Makefile.in is not written by hand. And I guess automake has put
> > > `--foreign` there because foreign is used in Makefile.am:
> > Yes, I know :-)
> >
> > >
> > > AUTOMAKE_OPTIONS = foreign no-dist
> > >
> > > But a simple call so `automake -f` (or `autoreconf -f`) just works, as
> > > automake picks up the foreign option from AUTOMAKE_OPTIONS, so a human
> > > or an external script who wants to regenerate things would probably just
> > > use that.
> >
> > Indeed. I guess my concern is: if some change happens to
> > Makefile.am/Makefile.in which would imply that 'autoreconf -f' would
> > not work, how do we make sure autoregen.py (or whatever script) is
> > updated accordingly? Or maybe whatever change is made to
> > Makefile.am/Makefile.in, 'autoreconf -f' is supposed to handle it
> > without additional flag?
> >
> I think I've just noticed a variant of this: if you look at
> opcodes/Makefile.in, you can see that aclocal.m4 depends on
> configure.ac (among others). So if configure.ac is updated, a
> maintainer-mode rule in Makefile.in will call aclocal and regenerate
> aclocal.m4.
>
> However, autoregen.py calls aclocal only if configure.ac contains
> AC_CONFIG_MACRO_DIRS, which is not the case here.
>
> That's probably a bug in opcode/configure.ac, but still the current
> Makefile.in machinery would update aclocal.m4 as needed when
> autoregen.py will not.
>
> I haven't audited all configure.ac but there are probably other
> occurrences of this.
>
Another discrepancy I've just noticed: if you look at libsframe/Makefile.am,
you can see that ACLOCAL_AMFLAGS = -I .. -I ../config -I ../bfd,
so if you run autoreconf -f, it will invoke aclocal with these flags
(the same is performed by the aclocal.m4 regeneration rule in the Makefile),
but autoregen.py won't run aclocal because configure.ac does not define
AC_CONFIG_MACRO_DIRS, and even if it did, it would only use -I../config
I guess the same applies for several other subdirs.
So in general how do we make sure autoregen.py uses the right flags?
Or what prevents us from just using autoreconf -f? If that does not work
because configure.ac/Makeline.am and others have bugs, maybe
we should fix those bugs instead?
which makes me think about Eric's reply:
> `autoreconf -f` works fine in individual subdirectories, the problem
> is that the top-level configure.ac doesn't use the AC_CONFIG_SUBDIRS
> macro to specify its subdirectories, but rather uses its own
> hand-rolled method of specifying subdirectories that autoreconf
> doesn't know about. This means that autoreconf won't automatically
> recurse into all the necessary subdirectories by itself automatically,
> and instead has to be run manually in each subdirectory separately.
It's not clear to me if that "problem" is a bug, or a design decision
we must take into account when writing tools to help regeneration?
> Also the various subdirectories are inconsistent about whether they
> have a rule for running it (autoreconf) from the Makefile or not,
should that be considered a bug, and fixed?
> which usually comes down to whether the subdirectory uses automake for
> its Makefile or not (the top-level Makefile doesn't; it uses its own
> weird autogen-based regeneration method instead, which means that it
> misses out on all the built-in rules that automake would implicitly
> generate, including ones related to build system regeneration).
Thanks,
Christophe
> Christophe
>
> > >
> > > > The bot I want to put in place would regenerate things as they are
> > > > supposed to be, then build and run the testsuite to make sure that
> > > > what is supposed to be committed would work (if the committer
> > > > regenerates everything correctly)
> > >
> > > For your job, would it be fine to just force-regenerate everything and
> > > ignore timestamps (just like the buildbot's autoregen job wants to do)?
> > > It would waste a few cycles, but it would be much simpler.
> > >
> > Yes, that would achieve the purpose: be able to handle as many patches
> > as possible in precommit-CI.
> > And as described earlier, for binutils this currently means:
> > autoregen
> > confgure --enable-maintainer-mode
> > make all (with a low -j value otherwise we have random build failures)
> > and my proposal to workaround the problem with -j is to do
> > make all-bfd all-libiberty regenerate -j1
> > make all -j XXX
> >
> > Another possibility would be a policy change in how patches are
> > submitted, to require that they contain all the autogenerated files.
> >
> >
> > > Simon
next prev parent reply other threads:[~2024-04-08 9:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-13 8:02 Christophe Lyon
2024-03-14 18:10 ` Simon Marchi
2024-03-15 8:50 ` Christophe Lyon
2024-03-15 14:13 ` Eric Gallager
2024-03-15 14:25 ` Tom Tromey
2024-03-16 17:30 ` Simon Marchi
2024-03-18 17:28 ` Christophe Lyon
2024-03-20 15:11 ` Simon Marchi
2024-03-18 16:13 ` Christophe Lyon
2024-03-16 17:16 ` Simon Marchi
2024-03-18 17:25 ` Christophe Lyon
2024-03-19 17:11 ` Christophe Lyon
2024-03-19 18:03 ` Tom Tromey
2024-03-20 12:05 ` Eric Gallager
2024-03-20 15:34 ` Simon Marchi
2024-03-21 14:32 ` Christophe Lyon
2024-03-25 14:19 ` Christophe Lyon
2024-03-27 18:22 ` Christophe Lyon
2024-04-08 9:22 ` Christophe Lyon [this message]
[not found] ` <78f1d113-f8ac-4a76-8dea-9f92519c1a89@linux.ibm.com>
2024-03-27 18:14 ` Christophe Lyon
2024-03-28 8:55 ` Jens Remus
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='CAPS5kha7axXbD=atJNOj-VJpiXUp88K7QgRyS0ZdC_qutvF7rw@mail.gmail.com' \
--to=christophe.lyon@linaro.org \
--cc=binutils@sourceware.org \
--cc=gcc@gcc.gnu.org \
--cc=gdb@sourceware.org \
--cc=simark@simark.ca \
/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).