Re: [RFC] add regenerate Makefile target

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

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.
>

As another follow-up on this topic, while working on a tentative GCC
patch to implement this, I realized an obvious issue: all target
libraries configure steps depend on 'all-gcc' (of course, we need a
compiler to build the libs...)

So they idea of doing roughly:
- configure --enable-maintainer-mode
- make regenerate -j1  (to avoid current race conditions in maintainer-mode)
- make all -jXXX

means that the regenerate step will trigger the configure step for all
host and target subdirs as needed, and configuring target-libs
requires building 'all-gcc', which would happen at -j1 !

sigh :-)

Looks like we should handle binutils, gdb, and gcc differently for the
sake of precommit CI.

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