Message ID | 20181120163853.5040-2-thomas.petazzoni@bootlin.com |
---|---|
State | Changes Requested |
Headers | show |
Series | apr-util/apache fixes for per-package folders | expand |
Thomas, All, On 2018-11-20 17:38 +0100, Thomas Petazzoni spake thusly: > With per-package folder support enable, the build of apr-util fails, > for two reasons: > > - The rules.mk file is generated by the 'apr' package, and then > copied into the 'apr-util' source directory. This is done by the > 'apr-util' build process. Unfortunately, this rules.mk file has a > number of hardcoded paths: to the compiler and to the libtool > script. > > Due to this, the compiler from the 'apr' per-package folder gets > used. But this compiler uses the 'apr' package sysroot, which does > not have all the dependencies of the 'apr-util' package, causing > the build to fail because <expat.h> is not found. > > - Similarly, the libtool script itself has some hardcoded paths, > which make it use the compiler/linker from the 'apr' per-package > folder, so it does not find the expat library. > > We fix both issues by doing the necessary replacement in both rules.mk > and libtool. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > --- > Changes since v1: > - Drop the useless logic to copy the libtool script before doing the > replacement: as noted by Yann E. Morin, sed is properly breaking > hardlinks when doing an in-place replacement. > --- > package/apr-util/apr-util.mk | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/package/apr-util/apr-util.mk b/package/apr-util/apr-util.mk > index b006964ccb..379dfa692e 100644 > --- a/package/apr-util/apr-util.mk > +++ b/package/apr-util/apr-util.mk > @@ -15,6 +15,13 @@ APR_UTIL_CONF_OPTS = \ > --with-apr=$(STAGING_DIR)/usr/bin/apr-1-config > APR_UTIL_CONFIG_SCRIPTS = apu-1-config > > +define APR_UTIL_FIX_RULES_MK_LIBTOOL > + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk > + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \ I know we do not have a strong policy on this, but can you use '@' as the separator, instead of the comma ',' as we are sure that the build directory will not contain any '@': https://git.buildroot.org/buildroot/commit/Makefile?id=7007dc2bc99ad191c418c468707cdc3980273cda > + $(STAGING_DIR)/usr/build-1/libtool So, you're patching a file in STAGING_DIR in a post-configure hook? Also, PER_PACKAGE_DIR does yet exist in master (or next, for that matters), so this patch will have to leave in your branch for a little while still, I'm afraid.. :-/ > +endef > +APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK_LIBTOOL > + > # When iconv is available, then use it to provide charset conversion > # features. > APR_UTIL_DEPENDENCIES += $(if $(BR2_PACKAGE_LIBICONV),libiconv) > -- > 2.19.1 >
Hello, On Tue, 20 Nov 2018 21:09:44 +0100, Yann E. MORIN wrote: > > +define APR_UTIL_FIX_RULES_MK_LIBTOOL > > + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk > > + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \ > > I know we do not have a strong policy on this, but can you use '@' as > the separator, instead of the comma ',' as we are sure that the build > directory will not contain any '@': > https://git.buildroot.org/buildroot/commit/Makefile?id=7007dc2bc99ad191c418c468707cdc3980273cda ACK. > > + $(STAGING_DIR)/usr/build-1/libtool > > So, you're patching a file in STAGING_DIR in a post-configure hook? Yes. With per-package folder, we are preparing the target and host (including staging) folders with the package dependencies during the configure step. It is what we copy that contains references to other per-package folders that should be fixed up before the build. Hence, this needs to be done during the configure step. > Also, PER_PACKAGE_DIR does yet exist in master (or next, for that > matters), so this patch will have to leave in your branch for a little > while still, I'm afraid.. :-/ Perhaps I should include this logic inside a BR2_PER_PACKAGE_FOLDERS condition. Best regards, Thomas
diff --git a/package/apr-util/apr-util.mk b/package/apr-util/apr-util.mk index b006964ccb..379dfa692e 100644 --- a/package/apr-util/apr-util.mk +++ b/package/apr-util/apr-util.mk @@ -15,6 +15,13 @@ APR_UTIL_CONF_OPTS = \ --with-apr=$(STAGING_DIR)/usr/bin/apr-1-config APR_UTIL_CONFIG_SCRIPTS = apu-1-config +define APR_UTIL_FIX_RULES_MK_LIBTOOL + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' $(@D)/build/rules.mk + $(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \ + $(STAGING_DIR)/usr/build-1/libtool +endef +APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK_LIBTOOL + # When iconv is available, then use it to provide charset conversion # features. APR_UTIL_DEPENDENCIES += $(if $(BR2_PACKAGE_LIBICONV),libiconv)
With per-package folder support enable, the build of apr-util fails, for two reasons: - The rules.mk file is generated by the 'apr' package, and then copied into the 'apr-util' source directory. This is done by the 'apr-util' build process. Unfortunately, this rules.mk file has a number of hardcoded paths: to the compiler and to the libtool script. Due to this, the compiler from the 'apr' per-package folder gets used. But this compiler uses the 'apr' package sysroot, which does not have all the dependencies of the 'apr-util' package, causing the build to fail because <expat.h> is not found. - Similarly, the libtool script itself has some hardcoded paths, which make it use the compiler/linker from the 'apr' per-package folder, so it does not find the expat library. We fix both issues by doing the necessary replacement in both rules.mk and libtool. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- Changes since v1: - Drop the useless logic to copy the libtool script before doing the replacement: as noted by Yann E. Morin, sed is properly breaking hardlinks when doing an in-place replacement. --- package/apr-util/apr-util.mk | 7 +++++++ 1 file changed, 7 insertions(+)