* [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive @ 2023-05-28 11:30 Dario Binacchi 2023-07-28 21:01 ` Thomas Petazzoni via buildroot 0 siblings, 1 reply; 7+ messages in thread From: Dario Binacchi @ 2023-05-28 11:30 UTC (permalink / raw) To: buildroot; +Cc: Dario Binacchi The patch expands the number of usable .m4 files by including those contained in the autoconf-archive. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- package/automake/automake.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 -- 2.32.0 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-05-28 11:30 [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive Dario Binacchi @ 2023-07-28 21:01 ` Thomas Petazzoni via buildroot 2023-07-29 14:47 ` Dario Binacchi 0 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni via buildroot @ 2023-07-28 21:01 UTC (permalink / raw) To: Dario Binacchi; +Cc: buildroot Hello Dario, On Sun, 28 May 2023 13:30:42 +0200 Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > The patch expands the number of usable .m4 files by including those > contained in the autoconf-archive. > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > --- > package/automake/automake.mk | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > 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? Thanks! 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-07-28 21:01 ` Thomas Petazzoni via buildroot @ 2023-07-29 14:47 ` Dario Binacchi 2023-07-29 21:03 ` Thomas Petazzoni via buildroot 0 siblings, 1 reply; 7+ messages in thread From: Dario Binacchi @ 2023-07-29 14:47 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot Hello Thomas, On Fri, Jul 28, 2023 at 11:02 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Dario, > > On Sun, 28 May 2023 13:30:42 +0200 > Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote: > > > The patch expands the number of usable .m4 files by including those > > contained in the autoconf-archive. > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > --- > > package/automake/automake.mk | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > 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? > > Thanks! > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com 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. [1] https://patchwork.ozlabs.org/project/buildroot/patch/20230520211246.3950131-1-dario.binacchi@amarulasolutions.com/ [2] https://marc.info/?l=linux-netdev&m=168486237131560&w=2 Thanks and regards, Dario -- Dario Binacchi Senior Embedded Linux Developer dario.binacchi@amarulasolutions.com __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-07-29 14:47 ` Dario Binacchi @ 2023-07-29 21:03 ` Thomas Petazzoni via buildroot 2023-08-13 12:40 ` Yann E. MORIN 0 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni via buildroot @ 2023-07-29 21:03 UTC (permalink / raw) To: Dario Binacchi; +Cc: Yann E. MORIN, buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-07-29 21:03 ` Thomas Petazzoni via buildroot @ 2023-08-13 12:40 ` Yann E. MORIN 2023-08-13 20:39 ` Thomas Petazzoni via buildroot 0 siblings, 1 reply; 7+ messages in thread From: Yann E. MORIN @ 2023-08-13 12:40 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: Dario Binacchi, buildroot Thomas, All, On 2023-07-29 23:03 +0200, Thomas Petazzoni via buildroot spake thusly: > 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. After thinking a bit on this, here's what I think we should try; For packages that have _AUTORECONF = YES, we forcibly add host-autoconf-archive to their _DEPENDENCIES, and always set the macros search path as proposed here. Supposedly, having macros from autoconf-archive available should not be a cause for failure to successfully autorconf, otherwise that would not work on random systems which have it installed for other reasons, like native builds on standard distros; also, a missing directorry in the macro search list should not be a cause for failure. Consequently, we can drop the ad-hoc dependencies in the individual packages, and drop the ad-hoc include directove as well. host-autoconf-archive is a plain autocnf package, without dependencies (save for the autoconf machinery), and it only ever installs a buncha files, does not compile anything, so it is pretty fast. Adding it to all autoreconfigured packages should have a minimal, barely noticeable impact on the build time. If the above causes too much breakage, then even this patch was going to be incorrect, as it would unconditionally add the autocon-archive path to the search list for all packages that indirectly have autoconf-archive in their dependencies. Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-08-13 12:40 ` Yann E. MORIN @ 2023-08-13 20:39 ` Thomas Petazzoni via buildroot 2023-08-29 17:09 ` Dario Binacchi 0 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni via buildroot @ 2023-08-13 20:39 UTC (permalink / raw) To: Yann E. MORIN; +Cc: Dario Binacchi, buildroot Hello, On Sun, 13 Aug 2023 14:40:37 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > After thinking a bit on this, here's what I think we should try; > > For packages that have _AUTORECONF = YES, we forcibly add > host-autoconf-archive to their _DEPENDENCIES, and always set the macros > search path as proposed here. Supposedly, having macros from > autoconf-archive available should not be a cause for failure to > successfully autorconf, otherwise that would not work on random systems > which have it installed for other reasons, like native builds on > standard distros; also, a missing directorry in the macro search list > should not be a cause for failure. > > Consequently, we can drop the ad-hoc dependencies in the individual > packages, and drop the ad-hoc include directove as well. > > host-autoconf-archive is a plain autocnf package, without dependencies > (save for the autoconf machinery), and it only ever installs a buncha > files, does not compile anything, so it is pretty fast. Adding it to all > autoreconfigured packages should have a minimal, barely noticeable > impact on the build time. > > If the above causes too much breakage, then even this patch was going to > be incorrect, as it would unconditionally add the autocon-archive path > to the search list for all packages that indirectly have > autoconf-archive in their dependencies. We have 336 packages that set AUTORECONF = YES. Out of these 336 packages, only 8 need host-autoconf-archive. To me, it makes no sense to add host-autoconf-archive to those 336-8 packages that need autoreconf, but do not use any of the macros provided by host-autoconf-archive. One of Buildroot's beauty is its minimalism: it builds only what's needed, and every time it builds something, there is a solid reason for it. I would really dislike if we were to start building useless dependencies, even if admittedly host-autoconf-archive is small and quick to build. But small and quick to build is not the only thing, it's also about whether it makes sense. I regularly stare at my build going on, and when something gets built that I don't understand why its gets pulled into the build, I check with "make graph-depends" why it is there, and sometimes investigate further to make sure there's a good justification. To me, this is an important property of Buildroot, and I would really like to keep this aspect of Buildroot. Especially, I don't see what problem we would solve by making host-autoconf-archive a dependency of all packages that do AUTORECONF = YES. What problem would this solve? 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive 2023-08-13 20:39 ` Thomas Petazzoni via buildroot @ 2023-08-29 17:09 ` Dario Binacchi 0 siblings, 0 replies; 7+ messages in thread From: Dario Binacchi @ 2023-08-29 17:09 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: Yann E. MORIN, buildroot Hi Yann and Thomas, On Sun, Aug 13, 2023 at 10:41 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Sun, 13 Aug 2023 14:40:37 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > After thinking a bit on this, here's what I think we should try; > > > > For packages that have _AUTORECONF = YES, we forcibly add > > host-autoconf-archive to their _DEPENDENCIES, and always set the macros > > search path as proposed here. Supposedly, having macros from > > autoconf-archive available should not be a cause for failure to > > successfully autorconf, otherwise that would not work on random systems > > which have it installed for other reasons, like native builds on > > standard distros; also, a missing directorry in the macro search list > > should not be a cause for failure. > > > > Consequently, we can drop the ad-hoc dependencies in the individual > > packages, and drop the ad-hoc include directove as well. > > > > host-autoconf-archive is a plain autocnf package, without dependencies > > (save for the autoconf machinery), and it only ever installs a buncha > > files, does not compile anything, so it is pretty fast. Adding it to all > > autoreconfigured packages should have a minimal, barely noticeable > > impact on the build time. > > > > If the above causes too much breakage, then even this patch was going to > > be incorrect, as it would unconditionally add the autocon-archive path > > to the search list for all packages that indirectly have > > autoconf-archive in their dependencies. > > We have 336 packages that set AUTORECONF = YES. Out of these 336 > packages, only 8 need host-autoconf-archive. > > To me, it makes no sense to add host-autoconf-archive to those 336-8 > packages that need autoreconf, but do not use any of the macros > provided by host-autoconf-archive. One of Buildroot's beauty is its > minimalism: it builds only what's needed, and every time it builds > something, there is a solid reason for it. I would really dislike if we > were to start building useless dependencies, even if admittedly > host-autoconf-archive is small and quick to build. But small and quick > to build is not the only thing, it's also about whether it makes sense. > I regularly stare at my build going on, and when something gets built > that I don't understand why its gets pulled into the build, I check > with "make graph-depends" why it is there, and sometimes investigate > further to make sure there's a good justification. To me, this is an > important property of Buildroot, and I would really like to keep this > aspect of Buildroot. > > Especially, I don't see what problem we would solve by making > host-autoconf-archive a dependency of all packages that do AUTORECONF = > YES. What problem would this solve? > I am available to apply the changes suggested by Thomas in order to submit version 2 of the patch. Alternatively, do you think it's better to stop here and not proceed further? As far as I'm concerned, this solution convinces me, but I also admit that I don't have a complete understanding of the topic and its implications. Thanks and regards, Dario > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com -- Dario Binacchi Senior Embedded Linux Developer dario.binacchi@amarulasolutions.com __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@amarulasolutions.com www.amarulasolutions.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-08-29 17:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-28 11:30 [Buildroot] [RFC PATCH 1/1] package/automake: include .m4 files of autoconf-archive Dario Binacchi 2023-07-28 21:01 ` Thomas Petazzoni via buildroot 2023-07-29 14:47 ` Dario Binacchi 2023-07-29 21:03 ` Thomas Petazzoni via buildroot 2023-08-13 12:40 ` Yann E. MORIN 2023-08-13 20:39 ` Thomas Petazzoni via buildroot 2023-08-29 17:09 ` Dario Binacchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox