Re: [RFC] add regenerate Makefile target

Hi,

On Mon, 25 Mar 2024 at 15:19, Christophe Lyon
<[email protected]> wrote:
>
> On Thu, 21 Mar 2024 at 15:32, Christophe Lyon
> <[email protected]> wrote:
> >
> > On Wed, 20 Mar 2024 at 16:34, Simon Marchi <[email protected]> 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