pkg-infra: better fix libtool files wrt. /usr (branch yem/libtool)

PATCH New Yann E. MORIN 3 participants 5 messages May 3, 2015 – Jul 19, 2015

lore

Share & tools

lore

Yann E. MORIN

author

2015-05-03 21:30 UTC #0 lore raw

v2 cover pkg-infra: better fix libtool files wrt. /usr (branch yem/libtool)

Hello All!

This small series aims at better fixing libtool's .la files.

Currently, we fix .la files by per-prending the path to the sysroot to any path that starts with /usr . However, if either or both of the base directory (basically, $(O) ) or the staging directory are themselves sub-directories of /usr , we avoid doing that transformation, by carefully 'protecting' those two paths, to keep them intact.

However, if the toolchain itself is present as a sub-directory of /usr (i.e. a pre-installed custom toolchain), it may also contain .la files with paths starting with /usr . This means we would currently prepend the path to the staging directory to such paths in those .la files.

Of course, that breaks. :-(

So this series adds two fixes for that:

- first, it properly protects the path to the custom pre-installed toolchain, the same way we handle the base and staging directories, (patch by Arnout),

- second, it moves .la munging into the generic package infrastructure, as even non-autotools-based packages may also install .la files (patch by me).

Changes v1 -> v2:
- add the second patch

Regards,
Yann E. MORIN.

The following changes since commit 6e55cd5d604b51a5f6658d4bf2f602a360d9c887:

libfreeimage: disable on ARC and Blackfin (2015-05-03 19:33:52 +0200)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/libtool

for you to fetch changes up to 2498f2352f5b65d77d91f6405d39e6349de640dc:

pkg-infra: munging .la files is not limited to autotools packages (2015-05-03 23:17:44 +0200)

----------------------------------------------------------------
Arnout Vandecappelle (1):
autotools-package: also handle pre-installed external toolchain in .la fixup

Yann E. MORIN (1):
pkg-infra: munging .la files is not limited to autotools packages

 package/pkg-autotools.mk | 20 --------------------
 package/pkg-generic.mk   | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 20 deletions(-)

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2015-05-03 21:30 UTC #1 lore raw

PATCH v2 1/2 autotools-package: also handle pre-installed external toolchain in .la fixup

From: Arnout Vandecappelle <[email protected]>

The .la fixup handling looks for paths starting with /usr and assumes that they are missing the installation prefix (i.e. $(STAGING_DIR)). It already handles the cases that $(STAGING_DIR) itself and $(BASE_DIR) are under /usr, but it does not yet handle the case that a pre-installed external toolchain is under /usr (and tracks that fact in some .la file). For instance, if you use buildroot to generate a toolchain with HOST_DIR=/usr/local/some_path, this problem will occur.

Fix this in the same way as $(STAGING_DIR) and $(BASE_DIR), but in addition check that TOOLCHAIN_EXTERNAL_INSTALL_DIR is non-empty. For internal toolchains, it is empty and the sed expression would fail.

Signed-off-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Reported-by Carlos Soto <[email protected]>

Cc Carlos Soto <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

Signed-off-by Yann E. MORIN <[email protected]>

---
I haven't been able to test this very extensively because it's not so easy to find .la files where it goes wrong.

Carlos, can you check if this patch solves the problem for you?

---
 package/pkg-autotools.mk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

package/pkg-autotools.mk +11 -4

index 49b42d4..9dea08a 100644
@@ -304,10 +304,13 @@ endif
 # needs to be applied to any path that starts with /usr.
 #
 # To protect against the case that the output or staging directories
-# themselves are under /usr, we first substitute away any occurrences
-# of these directories as @BASE_DIR@ and @STAGING_DIR at . Note that
-# STAGING_DIR can be outside BASE_DIR when the user sets BR2_HOST_DIR
-# to a custom value.
+# or the pre-installed external toolchain themselves are under /usr,
+# we first substitute away any occurrences of these directories as
+# @BASE_DIR@, @STAGING_DIR@ and @TOOLCHAIN_EXTERNAL_INSTALL_DIR@ respectively.
+# Note that STAGING_DIR can be outside BASE_DIR when the user sets
+# BR2_HOST_DIR to a custom value. Note that TOOLCHAIN_EXTERNAL_INSTALL_DIR
+# can be under @BASE_DIR@ when it's a downloaded toolchain, and can be empty
+# when we use an internal toolchain.
 #
 ifndef $(2)_INSTALL_STAGING_CMDS
 define $(2)_INSTALL_STAGING_CMDS

@@ -315,7 +318,11 @@ define $(2)_INSTALL_STAGING_CMDS
 	find $$(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
 		$$(SED) "s:$$(BASE_DIR):@BASE_DIR@:g" \
 			-e "s:$$(STAGING_DIR):@STAGING_DIR@:g" \
+			$$(if $$(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
+				-e "s:$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:g") \
 			-e "s:\(['= ]\)/usr:\\1 at STAGING_DIR@/usr:g" \
+			$$(if $$(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
+				-e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
 			-e "s:@STAGING_DIR@:$$(STAGING_DIR):g" \
 			-e "s:@BASE_DIR@:$$(BASE_DIR):g"
 endef

Signature · 1 line

1.9.1

Peter Korsgaard

2015-05-04 13:35 UTC #2 lore raw

PATCH v2 1/2 autotools-package: also handle pre-installed external toolchain in .la fixup

Quoted 21 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
From: Arnout Vandecappelle <[email protected]>
The .la fixup handling looks for paths starting with /usr and assumes that they are missing the installation prefix (i.e. $(STAGING_DIR)). It already handles the cases that $(STAGING_DIR) itself and $(BASE_DIR) are under /usr, but it does not yet handle the case that a pre-installed external toolchain is under /usr (and tracks that fact in some .la file). For instance, if you use buildroot to generate a toolchain with HOST_DIR=/usr/local/some_path, this problem will occur.
Fix this in the same way as $(STAGING_DIR) and $(BASE_DIR), but in addition check that TOOLCHAIN_EXTERNAL_INSTALL_DIR is non-empty. For internal toolchains, it is empty and the sed expression would fail.
Signed-off-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Reported-by Carlos Soto <[email protected]>
Cc Carlos Soto <[email protected]>
Reviewed-by "Yann E. MORIN" <[email protected]>
Signed-off-by Yann E. MORIN <[email protected]>

Quoted 5 lines

---
I haven't been able to test this very extensively because it's not so easy to find .la files where it goes wrong.
Carlos, can you check if this patch solves the problem for you?

Committed, thanks.

Signature · 1 line

Bye, Peter Korsgaard

Yann E. MORIN

author

2015-05-03 21:30 UTC #3 lore raw

PATCH v2 2/2 pkg-infra: munging .la files is not limited to autotools packages

In reply to Yann E. MORIN

Currently, we're only fixing libtool's .la files for autotools packages, and even more so, only for those that do not define their own _INSTALL_STAGING_CMDS.

However, .la files might also be installed by non-autotools packages, like the NVidia GPU binary blob (libGL.la).

So, move that code into the global build rule, so all packages get their .la files fixed, whether they be autotools packages or not, and whether they define their own _INSTALL_STAGING_CMDS or not.

Also print a message.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 package/pkg-autotools.mk | 27 ---------------------------
 package/pkg-generic.mk   | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 27 deletions(-)

package/pkg-autotools.mk +0 ~~-27~~

index 9dea08a..bf65247 100644
@@ -295,36 +295,9 @@ endif
 # Staging installation step. Only define it if not already defined by
 # the package .mk file.
 #
-# Most autotools packages install libtool .la files alongside any
-# installed libraries. These .la files sometimes refer to paths
-# relative to the sysroot, which libtool will interpret as absolute
-# paths to host libraries instead of the target libraries. Since this
-# is not what we want, these paths are fixed by prefixing them with
-# $(STAGING_DIR).  As we configure with --prefix=/usr, this fix
-# needs to be applied to any path that starts with /usr.
-#
-# To protect against the case that the output or staging directories
-# or the pre-installed external toolchain themselves are under /usr,
-# we first substitute away any occurrences of these directories as
-# @BASE_DIR@, @STAGING_DIR@ and @TOOLCHAIN_EXTERNAL_INSTALL_DIR@ respectively.
-# Note that STAGING_DIR can be outside BASE_DIR when the user sets
-# BR2_HOST_DIR to a custom value. Note that TOOLCHAIN_EXTERNAL_INSTALL_DIR
-# can be under @BASE_DIR@ when it's a downloaded toolchain, and can be empty
-# when we use an internal toolchain.
-#
 ifndef $(2)_INSTALL_STAGING_CMDS
 define $(2)_INSTALL_STAGING_CMDS
 	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_INSTALL_STAGING_OPTS) -C $$($$(PKG)_SRCDIR)
-	find $$(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
-		$$(SED) "s:$$(BASE_DIR):@BASE_DIR@:g" \
-			-e "s:$$(STAGING_DIR):@STAGING_DIR@:g" \
-			$$(if $$(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
-				-e "s:$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:g") \
-			-e "s:\(['= ]\)/usr:\\1 at STAGING_DIR@/usr:g" \
-			$$(if $$(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
-				-e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
-			-e "s:@STAGING_DIR@:$$(STAGING_DIR):g" \
-			-e "s:@BASE_DIR@:$$(BASE_DIR):g"
 endef
 endif

package/pkg-generic.mk +30 -0

index d5b29f0..bd6fa5c 100644
@@ -172,6 +172,25 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call step_end,install-host)
 
 # Install to staging dir
+#
+# Some packages install libtool .la files alongside any installed
+# libraries. These .la files sometimes refer to paths relative to the
+# sysroot, which libtool will interpret as absolute paths to host
+# libraries instead of the target libraries. Since this is not what we
+# want, these paths are fixed by prefixing them with $(STAGING_DIR).
+# As we configure with --prefix=/usr, this fix needs to be applied to
+# any path that starts with /usr.
+#
+# To protect against the case that the output or staging directories or
+# the pre-installed external toolchain themselves are under /usr, we first
+# substitute away any occurrences of these directories with @BASE_DIR@,
+# @STAGING_DIR@ and @TOOLCHAIN_EXTERNAL_INSTALL_DIR@ respectively.
+#
+# Note that STAGING_DIR can be outside BASE_DIR when the user sets
+# BR2_HOST_DIR to a custom value. Note that TOOLCHAIN_EXTERNAL_INSTALL_DIR
+# can be under @BASE_DIR@ when it's a downloaded toolchain, and can be
+# empty when we use an internal toolchain.
+#
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")

@@ -189,6 +208,17 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 				-e "s, at BASE_DIR@,$(BASE_DIR),g" \
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
+	@$(call MESSAGE,"Fixing libtool files")
+	$(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+		$(SED) "s:$(BASE_DIR):@BASE_DIR@:g" \
+			-e "s:$(STAGING_DIR):@STAGING_DIR@:g" \
+			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
+				-e "s:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:g") \
+			-e "s:\(['= ]\)/usr:\\1 at STAGING_DIR@/usr:g" \
+			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
+				-e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
+			-e "s:@STAGING_DIR@:$(STAGING_DIR):g" \
+			-e "s:@BASE_DIR@:$(BASE_DIR):g"
 	$(Q)touch $@
 	@$(call step_end,install-staging)

Signature · 1 line

1.9.1

Thomas Petazzoni

Applied

2015-07-19 09:53 UTC #4 lore raw

PATCH v2 2/2 pkg-infra: munging .la files is not limited to autotools packages

Dear Yann E. MORIN,

Quoted 11 lines · Yann E. MORIN

Currently, we're only fixing libtool's .la files for autotools packages, and even more so, only for those that do not define their own _INSTALL_STAGING_CMDS.
However, .la files might also be installed by non-autotools packages, like the NVidia GPU binary blob (libGL.la).
So, move that code into the global build rule, so all packages get their .la files fixed, whether they be autotools packages or not, and whether they define their own _INSTALL_STAGING_CMDS or not.

Quoted 8 lines

Also print a message.
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 package/pkg-autotools.mk | 27 ---------------------------
 package/pkg-generic.mk   | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 27 deletions(-)

Applied, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com