Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>, buildroot@buildroot.org
Subject: Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive
Date: Sat, 29 Jul 2023 23:03:35 +0200	[thread overview]
Message-ID: <20230729230335.3cd59db6@windsurf> (raw)
In-Reply-To: <CABGWkvoEvH=4=OmPQHm5T1XMAQ11x-SixLpAxkUL1CUtUrF2_A@mail.gmail.com>

On Sat, 29 Jul 2023 16:47:14 +0200
Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:

> > > diff --git a/package/automake/automake.mk b/package/automake/automake.mk
> > > index fd7933dd74f9..fc235784641a 100644
> > > --- a/package/automake/automake.mk
> > > +++ b/package/automake/automake.mk
> > > @@ -34,5 +34,6 @@ $(eval $(host-autotools-package))
> > >  AUTOMAKE = $(HOST_DIR)/bin/automake
> > >  ACLOCAL_DIR = $(STAGING_DIR)/usr/share/aclocal
> > >  ACLOCAL = $(HOST_DIR)/bin/aclocal
> > > -ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR)
> > > +ACLOCAL_HOST_ARCHIVE_DIR = $(HOST_DIR)/share/autoconf-archive
> > > +ACLOCAL_PATH = $(ACLOCAL_DIR):$(ACLOCAL_HOST_DIR):$(ACLOCAL_HOST_ARCHIVE_DIR)
> > >  export ACLOCAL_PATH  
> >
> > Could you clarify the motivation? Do you have a specific case for which
> > this is needed?
> >
> Following the suggestions of Yann E. MORIN in [1], I tried to fix the
> issue by submitting a
> patch to the ethtool project. During the various review stages, I sent
> a version (much more
> complicated than the one that was eventually accepted) that required a
> specific M4
> macro ([2]). From there, the idea for this patch was born.

What's confusing to me is: how is autoconf-archive useful/working today
in Buildroot if aclocal isn't told that
$(HOST_DIR)/share/autoconf-archive contains a bunch of m4 macros?

Ah, I see:

LIBGPIOD_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
NEARD_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
PYTHON3_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
etc.

So it would indeed probably make sense to factorize this. But the
problem with your change is that $(HOST_DIR)/share/autoconf-archive
gets added to ACLOCAL_PATH even if autoconf-archive isn't
used/installed. We could decide it is OK, but it should be mentioned.
Another side effect is that once autoconf-archive has been installed,
*all* packages will see the m4 macros from autoconf-archive, not only
the few packages that explicitly need autoconf-archive. I don't know if
this is a problem, but it can potentially have some side effects.

Also your patch should drop all the <pkg>_AUTORECONF_OPTS =
--include=$(HOST_DIR)/share/autoconf-archive, because they become
redundant.

Full list is:

package/libgpiod/libgpiod.mk:LIBGPIOD_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
package/neard/neard.mk:NEARD_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
package/python3/python3.mk:PYTHON3_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
package/sdbusplus/sdbusplus.mk:SDBUSPLUS_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
package/sox/sox.mk:SOX_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive
package/thermald/thermald.mk:THERMALD_AUTORECONF_OPTS = --include=$(HOST_DIR)/share/autoconf-archive

Yann, thoughts?

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot