| Message ID | 20181123145815.13008-10-thomas.petazzoni@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Per-package host/target directory support | expand |
Thomas, All, On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly: > Libtool .la files unfortunately contain a number of absolute paths, > which now refer to per-package directories. Due to this, when building > package A, .la files may contain absolute paths referring to > directories in package B per-package sysroot. This causes some -L > flags referring to other sysroot from being added, which doesn't work > as the linker no longer realizes that such paths are within its > sysroot. > > To fix this, we introduce a replacement step of .la files in the > configure step, to make sure all paths refer to this package > per-package directory. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Since I really want this topic to come to a conclusion sooner than later, and since the solution is good-enough for now: Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Howerver, see below... > --- > package/pkg-generic.mk | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index b1f0cdf34a..17909f4a61 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -222,6 +222,12 @@ $(BUILD_DIR)/%/.stamp_configured: > @$(call step_start,configure) > @$(call MESSAGE,"Configuring") > $(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES)) > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y) > + # Make sure .la files only reference the current per-package > + # directory > + $(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \ > + $(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(NAME)/:g" > +endif I don't find it very clean that we shoe-horn such preparation in the generic rules; I'd rather we use a hook mechanism, which we can define for the prepare-per-package-directory macro to call. For example: define PER_PKG_CONFIGURE_FIXUP_LA_FILES Blabla your big find command endef PER_PACKAGE_CONFIGURE_PREPARE_HOOKS += PER_PKG_CONFIGURE_FIXUP_LA_FILES And then: # $(1): step to prepare for # $(2): space-separated list of dependencies to grab define prepare-per-package-directory mkdir -p $(HOST_DIR) $(TARGET_DIR) $(foreach pkg,$(2),\ rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \ $(PER_PACKAGE_DIR)/$(pkg)/host/ \ $(HOST_DIR)$(sep)) $(foreach pkg,$(2),\ rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \ $(PER_PACKAGE_DIR)/$(pkg)/target/ \ $(TARGET_DIR)$(sep)) $(foreach hook,$(PER_PACKAGE_$(call UPPER,$(1))_PREPARE_HOOKS),$(call $(hook))$(sep)) endef And then you'd just have to call prepare-per-package-directory with one of: $(call prepare-per-package-directory,source,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) $(call prepare-per-package-directory,extract,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES)) $(call prepare-per-package-directory,patch,$($(PKG)_FINAL_PATCH_DEPENDENCIES)) $(call prepare-per-package-directory,configure,$($(PKG)_FINAL_DEPENDENCIES)) > $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep)) ternatively, we could change this line to be able to run hooks before the package-registered hooks: $(foreach hook,\ $($(PKG)_EARLY_PRE_CONFIGURE_HOOKS) $($(PKG)_PRE_CONFIGURE_HOOKS),\ $(call $(hook))$(sep)) and earlier: $(2)_EARLY_PRE_CONFIGURE_HOOKS += \ PER_PKG_PREPARE_DIRECTORY_FOR_CONFIGURE \ PER_PKG_CONFIGURE_FIXUP_LA_FILES And so on for each steps, no? But again: I'm pretty much OK with the patch as is, because it is better than nothing, and it would be awesome to have that in early December, so it is part of the next LTS. (Unrelated note: why do we $(call $(hook)) instead of just $($(hook)) ?) Regards, Yann E. MORIN. > $($(PKG)_CONFIGURE_CMDS) > $(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep)) > @@ -875,6 +881,7 @@ $$($(2)_TARGET_INSTALL_IMAGES): PKG=$(2) > $$($(2)_TARGET_INSTALL_HOST): PKG=$(2) > $$($(2)_TARGET_BUILD): PKG=$(2) > $$($(2)_TARGET_CONFIGURE): PKG=$(2) > +$$($(2)_TARGET_CONFIGURE): NAME=$(1) > $$($(2)_TARGET_RSYNC): SRCDIR=$$($(2)_OVERRIDE_SRCDIR) > $$($(2)_TARGET_RSYNC): PKG=$(2) > $$($(2)_TARGET_PATCH): PKG=$(2) > -- > 2.19.1 >
Hello, On Sun, 25 Nov 2018 18:57:02 +0100, Yann E. MORIN wrote: > Since I really want this topic to come to a conclusion sooner than > later, and since the solution is good-enough for now: > > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Thanks. However, since I have changed a bit this patch for v7, I will not include your Reviewed-by. > I don't find it very clean that we shoe-horn such preparation in the > generic rules; I'd rather we use a hook mechanism, which we can define > for the prepare-per-package-directory macro to call. I understand your proposal. However, I think that it's premature to introduce such a hook mechanism: for now there's only one such fixup to do. Let's wait until we have a few. I have moved the code outside of the package step, into a helper function. It looks a bit nicer, and perhaps if we need to do other fixups this helper function can be extended to be the one doing all the per-package related fixups. What do you think ? Thomas
Thomas, All, On 2018-11-30 11:20 +0100, Thomas Petazzoni spake thusly: > On Sun, 25 Nov 2018 18:57:02 +0100, Yann E. MORIN wrote: > > Since I really want this topic to come to a conclusion sooner than > > later, and since the solution is good-enough for now: > > > > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > Thanks. However, since I have changed a bit this patch for v7, I will > not include your Reviewed-by. > > > I don't find it very clean that we shoe-horn such preparation in the > > generic rules; I'd rather we use a hook mechanism, which we can define > > for the prepare-per-package-directory macro to call. > > I understand your proposal. However, I think that it's premature to > introduce such a hook mechanism: for now there's only one such fixup to > do. Let's wait until we have a few. > > I have moved the code outside of the package step, into a helper > function. It looks a bit nicer, and perhaps if we need to do other > fixups this helper function can be extended to be the one doing all the > per-package related fixups. > > What do you think ? If you were speaking of: https://git.bootlin.com/users/thomas-petazzoni/buildroot/commit/?h=ppsh-v7-work&id=5fd4ce2bbed14478d49afcd775f37eb8c4e487a4 ... then you can keep my reviewed-by tag when you submit that new iteration. Thanks! :-) Regards, Yann E. MORIN. > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index b1f0cdf34a..17909f4a61 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -222,6 +222,12 @@ $(BUILD_DIR)/%/.stamp_configured: @$(call step_start,configure) @$(call MESSAGE,"Configuring") $(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES)) +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y) + # Make sure .la files only reference the current per-package + # directory + $(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \ + $(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(NAME)/:g" +endif $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep)) $($(PKG)_CONFIGURE_CMDS) $(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep)) @@ -875,6 +881,7 @@ $$($(2)_TARGET_INSTALL_IMAGES): PKG=$(2) $$($(2)_TARGET_INSTALL_HOST): PKG=$(2) $$($(2)_TARGET_BUILD): PKG=$(2) $$($(2)_TARGET_CONFIGURE): PKG=$(2) +$$($(2)_TARGET_CONFIGURE): NAME=$(1) $$($(2)_TARGET_RSYNC): SRCDIR=$$($(2)_OVERRIDE_SRCDIR) $$($(2)_TARGET_RSYNC): PKG=$(2) $$($(2)_TARGET_PATCH): PKG=$(2)
Libtool .la files unfortunately contain a number of absolute paths, which now refer to per-package directories. Due to this, when building package A, .la files may contain absolute paths referring to directories in package B per-package sysroot. This causes some -L flags referring to other sysroot from being added, which doesn't work as the linker no longer realizes that such paths are within its sysroot. To fix this, we introduce a replacement step of .la files in the configure step, to make sure all paths refer to this package per-package directory. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- package/pkg-generic.mk | 7 +++++++ 1 file changed, 7 insertions(+)