* [Buildroot] [RFC PATCH] package/sdbusplus: fix the build @ 2020-02-10 19:11 Michael Walle 2020-02-10 20:57 ` Yann E. MORIN 2020-02-19 20:03 ` Thomas Petazzoni 0 siblings, 2 replies; 4+ messages in thread From: Michael Walle @ 2020-02-10 19:11 UTC (permalink / raw) To: buildroot Commit d255b67972b4b7f27572581fe0c8c8aa03d850c8 fixed the handling of the a package local m4/ directory which might be missing. But this only works if it is the very first argument. But for this package this is not possible because we already occupy this with the extra include directory for autoconf-archive. Bring back the hook to create the m4/ directory to fix this. Signed-off-by: Michael Walle <michael@walle.cc> --- IMHO keeping this hook if the package has an include directory in its _AUTORECONF_OPTS variable is the least bad thing to do. Unfortunately, I've only been able to test this briefly with the host build. package/sdbusplus/sdbusplus.mk | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/package/sdbusplus/sdbusplus.mk b/package/sdbusplus/sdbusplus.mk index 9d3d1e8cf4..b771576a1d 100644 --- a/package/sdbusplus/sdbusplus.mk +++ b/package/sdbusplus/sdbusplus.mk @@ -22,5 +22,14 @@ SDBUSPLUS_INSTALL_STAGING = YES SDBUSPLUS_LICENSE = Apache-2.0 SDBUSPLUS_LICENSE_FILES = LICENSE +# Autoreconf is missing the m4/ directory, which might actually be missing +# iff it was the first argument, but unfortunately we are overriding the +# first include directory above. Thus we need that hook here. +define SDBUSPLUS_CREATE_M4 + mkdir -p $(@D)/m4 +endef +SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 +HOST_SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 + $(eval $(autotools-package)) $(eval $(host-autotools-package)) -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Buildroot] [RFC PATCH] package/sdbusplus: fix the build 2020-02-10 19:11 [Buildroot] [RFC PATCH] package/sdbusplus: fix the build Michael Walle @ 2020-02-10 20:57 ` Yann E. MORIN 2020-02-10 23:02 ` Michael Walle 2020-02-19 20:03 ` Thomas Petazzoni 1 sibling, 1 reply; 4+ messages in thread From: Yann E. MORIN @ 2020-02-10 20:57 UTC (permalink / raw) To: buildroot Michael, All, On 2020-02-10 20:11 +0100, Michael Walle spake thusly: > Commit d255b67972b4b7f27572581fe0c8c8aa03d850c8 fixed the handling of > the a package local m4/ directory which might be missing. But this only > works if it is the very first argument. But for this package this is not > possible because we already occupy this with the extra include directory > for autoconf-archive. Bring back the hook to create the m4/ directory to > fix this. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > > IMHO keeping this hook if the package has an include directory in its > _AUTORECONF_OPTS variable is the least bad thing to do. > > Unfortunately, I've only been able to test this briefly with the host > build. > > package/sdbusplus/sdbusplus.mk | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/package/sdbusplus/sdbusplus.mk b/package/sdbusplus/sdbusplus.mk > index 9d3d1e8cf4..b771576a1d 100644 > --- a/package/sdbusplus/sdbusplus.mk > +++ b/package/sdbusplus/sdbusplus.mk > @@ -22,5 +22,14 @@ SDBUSPLUS_INSTALL_STAGING = YES > SDBUSPLUS_LICENSE = Apache-2.0 > SDBUSPLUS_LICENSE_FILES = LICENSE > > +# Autoreconf is missing the m4/ directory, which might actually be missing > +# iff it was the first argument, but unfortunately we are overriding the > +# first include directory above. Thus we need that hook here. > +define SDBUSPLUS_CREATE_M4 > + mkdir -p $(@D)/m4 > +endef > +SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 > +HOST_SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 I have to say that I am not happy that we so fast resort to reintroducing such hooks, when the goal of the initial patch from you was that we should not have to meddle with the include directive to begin with... I would favour trying to fix it in a cleaner way, either my previous patchset (or something similar) to conditionally add the correct list of directories, or to mege back autoconf-archive back into the main include directory and see what package break and fix those instead. However, we currently have a single package that depends on autoconf-archive, so I would not put too much effort in it either... Thomas, Arnout, Peter: thoghts? Regards, Yann E. MORIN. > $(eval $(autotools-package)) > $(eval $(host-autotools-package)) > -- > 2.20.1 > -- .-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [RFC PATCH] package/sdbusplus: fix the build 2020-02-10 20:57 ` Yann E. MORIN @ 2020-02-10 23:02 ` Michael Walle 0 siblings, 0 replies; 4+ messages in thread From: Michael Walle @ 2020-02-10 23:02 UTC (permalink / raw) To: buildroot Am 10. Februar 2020 21:57:03 MEZ schrieb "Yann E. MORIN" <yann.morin.1998@free.fr>: >Michael, All, > >On 2020-02-10 20:11 +0100, Michael Walle spake thusly: >> Commit d255b67972b4b7f27572581fe0c8c8aa03d850c8 fixed the handling of >> the a package local m4/ directory which might be missing. But this >only >> works if it is the very first argument. But for this package this is >not >> possible because we already occupy this with the extra include >directory >> for autoconf-archive. Bring back the hook to create the m4/ directory >to >> fix this. >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> >> IMHO keeping this hook if the package has an include directory in its >> _AUTORECONF_OPTS variable is the least bad thing to do. >> >> Unfortunately, I've only been able to test this briefly with the host >> build. >> >> package/sdbusplus/sdbusplus.mk | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/package/sdbusplus/sdbusplus.mk >b/package/sdbusplus/sdbusplus.mk >> index 9d3d1e8cf4..b771576a1d 100644 >> --- a/package/sdbusplus/sdbusplus.mk >> +++ b/package/sdbusplus/sdbusplus.mk >> @@ -22,5 +22,14 @@ SDBUSPLUS_INSTALL_STAGING = YES >> SDBUSPLUS_LICENSE = Apache-2.0 >> SDBUSPLUS_LICENSE_FILES = LICENSE >> >> +# Autoreconf is missing the m4/ directory, which might actually be >missing >> +# iff it was the first argument, but unfortunately we are overriding >the >> +# first include directory above. Thus we need that hook here. >> +define SDBUSPLUS_CREATE_M4 >> + mkdir -p $(@D)/m4 >> +endef >> +SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 >> +HOST_SDBUSPLUS_POST_PATCH_HOOKS += SDBUSPLUS_CREATE_M4 > >I have to say that I am not happy that we so fast resort to >reintroducing >such hooks, when the goal of the initial patch from you was that we >should not have to meddle with the include directive to begin with... Haha ;) well lets say I would be all over for a more generic solution, you can ask Heiko; I can be a pain in the ass. But.. I don't know if it's worth the hassle to have that for just one package; or for packages which use an include path argument. Actually, I've learn from you (or you all) to be more pragmatic :p Therefore it was a RFC patch. I've already pointed out my alternative approach in the other thread. But that worked also just for that specific use case of having another include directory for ac macros. So we'd have to have an own variable where a package could specify an include path besides the _AUTORECONF_OPTS, something like _AUTORECONF_INCLUDES. But again, this could be a thing if there were more users for it. >I would favour trying to fix it in a cleaner way, either my previous >patchset (or something similar) to conditionally add the correct list >of directories, or to mege back autoconf-archive back into the main >include directory and see what package break and fix those instead. yes but then what if a package wants to use other include paths. Well, we could filter out the include arguments and put it in the ACLOCAL_PATH. But IMHO this is too much magic. -michael >However, we currently have a single package that depends on >autoconf-archive, so I would not put too much effort in it either... > >Thomas, Arnout, Peter: thoghts? > >Regards, >Yann E. MORIN. > >> $(eval $(autotools-package)) >> $(eval $(host-autotools-package)) >> -- >> 2.20.1 >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [RFC PATCH] package/sdbusplus: fix the build 2020-02-10 19:11 [Buildroot] [RFC PATCH] package/sdbusplus: fix the build Michael Walle 2020-02-10 20:57 ` Yann E. MORIN @ 2020-02-19 20:03 ` Thomas Petazzoni 1 sibling, 0 replies; 4+ messages in thread From: Thomas Petazzoni @ 2020-02-19 20:03 UTC (permalink / raw) To: buildroot On Mon, 10 Feb 2020 20:11:30 +0100 Michael Walle <michael@walle.cc> wrote: > Commit d255b67972b4b7f27572581fe0c8c8aa03d850c8 fixed the handling of > the a package local m4/ directory which might be missing. But this only > works if it is the very first argument. But for this package this is not > possible because we already occupy this with the extra include directory > for autoconf-archive. Bring back the hook to create the m4/ directory to > fix this. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- The commit log was missing a reference to the autobuilder failure that this commit fixes, so I've added it and applied to master. I looked at the discussion with Yann, and yes some more elaborate solution could perhaps be introduced to avoid the hook. But for now the work done on autoconf allows to avoid the hook in most situations, except when autoconf-archive is needed, which is only a small minority of our packages. For these packages, it is good enough for now to keep an explicit hook to create the m4 directory. Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-19 20:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-02-10 19:11 [Buildroot] [RFC PATCH] package/sdbusplus: fix the build Michael Walle 2020-02-10 20:57 ` Yann E. MORIN 2020-02-10 23:02 ` Michael Walle 2020-02-19 20:03 ` Thomas Petazzoni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox