Skip to contentSkip to conversation

infra: simplify .la/.pc fixup commands

PATCH New Thomas De Schampheleire 2 participants 6 messages Jun 29, 2014 – Jun 30, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

The commands fixing up .la and .pc files in the pkg-generic and pkg-autotools infrastructures can be simplified a little, since STAGING_DIR already contains BASE_DIR.

Additionally, this patch rewords the comment in pkg-autotools that explains the replacements.

---
 package/pkg-autotools.mk |  19 ++++++++-----------
 package/pkg-generic.mk   |   7 +++----
 2 files changed, 11 insertions(+), 15 deletions(-)
package/pkg-autotools.mk +8 -11
diff -r 4d6d2ac6a601 -r 6d38fb9f8bb5 package/pkg-autotools.mk
@@ -267,25 +267,22 @@
 # 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 we
-# configure with --prefix=/usr, such absolute paths start with
-# /usr. So we add $(STAGING_DIR) in front of any path that starts with
-# /usr.
+# 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 directory itself is
 # under /usr, we first substitute away any occurences of the output
-# directory to @BASE_DIR at .
+# directory as @BASE_DIR at .
 #
 ifndef $(2)_INSTALL_STAGING_CMDS
 define $(2)_INSTALL_STAGING_CMDS
 	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_SRCDIR)
-	for i in $$$$(find $$(STAGING_DIR)/usr/lib* -name "*.la"); do \
+	find $$(STAGING_DIR)/usr/lib* -name "*.la" | xargs \
 		$$(SED) "s:$$(BASE_DIR):@BASE_DIR@:g" \
-			-e "s:\(['= ]\)/usr:\\1 at STAGING_DIR@/usr:g" \
-			-e "s:@STAGING_DIR@:$$(STAGING_DIR):g" \
-			-e "s:@BASE_DIR@:$$(BASE_DIR):g" \
-			$$$$i; \
-	done
+			-e "s:\(['= ]\)/usr:\\1$$(STAGING_DIR)/usr:g" \
+			-e "s:@BASE_DIR@:$$(BASE_DIR):g"
 endef
 endif
 
package/pkg-generic.mk +3 -4
diff -r 4d6d2ac6a601 -r 6d38fb9f8bb5 package/pkg-generic.mk
@@ -202,10 +202,9 @@
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(call MESSAGE,"Fixing package configuration files") ;\
 			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
-				-e "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
-				-e "s,-I/usr/,-I at STAGING_DIR@/usr/,g" \
-				-e "s,-L/usr/,-L at STAGING_DIR@/usr/,g" \
-				-e "s, at STAGING_DIR@,$(STAGING_DIR),g" \
+				-e "s,^\(exec_\)\?prefix=.*,\1prefix=$(STAGING_DIR)/usr,g" \
+				-e "s,-I/usr/,-I$(STAGING_DIR)/usr/,g" \
+				-e "s,-L/usr/,-L$(STAGING_DIR)/usr/,g" \
 				-e "s, at BASE_DIR@,$(BASE_DIR),g" \
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
#1 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

Dear Thomas De Schampheleire,

Quoted 3 lines · Thomas De Schampheleire

The commands fixing up .la and .pc files in the pkg-generic and pkg-autotools infrastructures can be simplified a little, since STAGING_DIR already contains BASE_DIR.

Is this assumption really correct? If the user decides to move $(HOST_DIR) in some custom location using BR2_HOST_DIR, it also means that $(STAGING_DIR) is moved to this custom location. So you can have a case where:

BASE_DIR = /home/joe/buildroot/output
HOST_DIR = /opt/arm-build
STAGING_DIR = /opt/arm-build/usr/arm-unknown-linux-uclibcgnueabi/sysroot/

No ?

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

Thomas Petazzoni <[email protected]> schreef:

Quoted 13 lines

Dear Thomas De Schampheleire,

Quoted 3 lines · Thomas De Schampheleire

The commands fixing up .la and .pc files in the pkg-generic and pkg-autotools infrastructures can be simplified a little, since STAGING_DIR already contains BASE_DIR.

Is this assumption really correct? If the user decides to move $(HOST_DIR) in some custom location using BR2_HOST_DIR, it also means that $(STAGING_DIR) is moved to this custom location. So you can have a case where:

Quoted 5 lines

BASE_DIR = /home/joe/buildroot/output
HOST_DIR = /opt/arm-build
STAGING_DIR = /opt/arm-build/usr/arm-unknown-linux-uclibcgnueabi/sysroot/

No ?

Hmm, ok, but I'm this case the already applied patch is not complete either and we should go back to v2 I think... I'll resubmit...

...and I blame Arnout ;-)

Best regards,
Thomas

#3 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

Dear Thomas De Schampheleire,

Quoted 9 lines · Thomas De Schampheleire
Quoted 9 lines

Is this assumption really correct? If the user decides to move $(HOST_DIR) in some custom location using BR2_HOST_DIR, it also means that $(STAGING_DIR) is moved to this custom location. So you can have a case where:

BASE_DIR = /home/joe/buildroot/output
HOST_DIR = /opt/arm-build
STAGING_DIR = /opt/arm-build/usr/arm-unknown-linux-uclibcgnueabi/sysroot/

Quoted 5 lines
Quoted 1 line

No ?

Hmm, ok, but I'm this case the already applied patch is not complete either and we should go back to v2 I think... I'll resubmit...

...and I blame Arnout ;-)

Well, I don't know, I haven't tested nor mentally executed your sed expressions with the use case above in mind, so I don't know if the patch is actually broken or not. But if you assume that STAGING_DIR is a subdir of BASE_DIR, I believe it's not a valid assumption, as explained above.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

Thomas Petazzoni <[email protected]> schreef:

Quoted 17 lines

Dear Thomas De Schampheleire,

Quoted 13 lines · Thomas De Schampheleire
Quoted 10 lines

Is this assumption really correct? If the user decides to move $(HOST_DIR) in some custom location using BR2_HOST_DIR, it also means that $(STAGING_DIR) is moved to this custom location. So you can have a case where:

BASE_DIR = /home/joe/buildroot/output
HOST_DIR = /opt/arm-build
STAGING_DIR = /opt/arm-build/usr/arm-unknown-linux-uclibcgnueabi/sysroot/

No ?

Hmm, ok, but I'm this case the already applied patch is not complete either and we should go back to v2 I think... I'll resubmit...

Quoted 7 lines
Quoted 1 line

...and I blame Arnout ;-)

Well, I don't know, I haven't tested nor mentally executed your sed expressions with the use case above in mind, so I don't know if the patch is actually broken or not. But if you assume that STAGING_DIR is a subdir of BASE_DIR, I believe it's not a valid assumption, as explained above.

I think the patch is effectively broken due to this assumption. I will send a more detailed mail later with some sed commands on examples. This should clarify things...

Best regards,
Thomas

#5 lore raw

PATCH 1/1 infra: simplify .la/.pc fixup commands

Hi,

Quoted 20 lines · Thomas De Schampheleire

Thomas Petazzoni <[email protected]> schreef:

Quoted 19 lines

Dear Thomas De Schampheleire,

Quoted 14 lines · Thomas De Schampheleire
Quoted 10 lines

Is this assumption really correct? If the user decides to move $(HOST_DIR) in some custom location using BR2_HOST_DIR, it also means that $(STAGING_DIR) is moved to this custom location. So you can have a case where:

BASE_DIR = /home/joe/buildroot/output
HOST_DIR = /opt/arm-build
STAGING_DIR = /opt/arm-build/usr/arm-unknown-linux-uclibcgnueabi/sysroot/

No ?

Hmm, ok, but I'm this case the already applied patch is not complete either and we should go back to v2 I think... I'll resubmit...

...and I blame Arnout ;-)

Quoted 7 lines
Quoted 5 lines

Well, I don't know, I haven't tested nor mentally executed your sed expressions with the use case above in mind, so I don't know if the patch is actually broken or not. But if you assume that STAGING_DIR is a subdir of BASE_DIR, I believe it's not a valid assumption, as explained above.

I think the patch is effectively broken due to this assumption. I will send a more detailed mail later with some sed commands on examples. This should clarify things...

Let me try to clarify the different sed commands on an example.

tdescham at argentina ~ $ cat /tmp/testfile

1=/usr/foo                                 # unprefixed
2=/usr/buildroot/output/sysroot/usr/foo    # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

The goal of the sed command is to change the first line to include the staging dir, and not touch any of the others.
There are two versions of my patch relevant here: v3 is the currently committed version and implicitly assumes that STAGING_DIR always contains BASE_DIR (which is so by default but as ThomasP pointed out is not always true). The earlier v2 did not make this assumption and treated BASE_DIR and STAGING_DIR separately.

For the record:
BASE_DIR = <buildroot>/output
but can be overwritten with O=xxx

HOST_DIR := $(BASE_DIR)/host
HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
STAGING_SUBDIR = usr/$(GNU_TARGET_NAME)/sysroot
STAGING_DIR = $(HOST_DIR)/$(STAGING_SUBDIR)

So STAGING_DIR is by default under BASE_DIR, but this can be changed with BR2_HOST_DIR.

The sed commands in the two patch versions:

tdescham at argentina ~ $ cat /tmp/sedv2
#!/bin/sh

sed -e "s:$BASE_DIR:@BASE_DIR@:g" \
    -e "s:$STAGING_DIR:@STAGING_DIR@:g" \
    -e "s:\(['= ]\)/usr:\\1 at STAGING_DIR@/usr:g" \
    -e "s:@STAGING_DIR@:$STAGING_DIR:g" \
    -e "s:@BASE_DIR@:$BASE_DIR:g"
tdescham at argentina ~ $ cat /tmp/sedv3
#!/bin/sh
sed -e "s:$BASE_DIR:@BASE_DIR@:g" \
    -e "s:\(['= ]\)/usr:\\1$STAGING_DIR/usr:g" \
    -e "s:@BASE_DIR@:$BASE_DIR:g"

When buildroot is not in /usr, there was never a problem, not with the original code nor with the adapted (v2 or v3).

tdescham at argentina ~ $ BASE_DIR=/home/buildroot/output STAGING_DIR=$BASE_DIR/sysroot /tmp/sedv2 < /tmp/testfile 1=/home/buildroot/output/sysroot/usr/foo

  # unprefixed
2=/home/buildroot/output/sysroot/usr/buildroot/output/sysroot/usr/foo
  # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

tdescham at argentina ~ $ BASE_DIR=/home/buildroot/output STAGING_DIR=$BASE_DIR/sysroot /tmp/sedv3 < /tmp/testfile 1=/home/buildroot/output/sysroot/usr/foo

  # unprefixed
2=/home/buildroot/output/sysroot/usr/buildroot/output/sysroot/usr/foo
  # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

In both cases, line 1 is correctly prefixed, line 2 is double-prefixed which is normal because it starts with /usr but does not start with BASE_DIR nor STAGING_DIR, line 3 and 4 are correctly untouched.

When buildroot is placed in /usr, things went wrong originally, but both patch v2 and v3 solve this:

tdescham at argentina ~ $ BASE_DIR=/usr/buildroot/output STAGING_DIR=$BASE_DIR/sysroot /tmp/sedv2 < /tmp/testfile 1=/usr/buildroot/output/sysroot/usr/foo

 # unprefixed
2=/usr/buildroot/output/sysroot/usr/foo    # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

tdescham at argentina ~ $ BASE_DIR=/usr/buildroot/output STAGING_DIR=$BASE_DIR/sysroot /tmp/sedv3 < /tmp/testfile 1=/usr/buildroot/output/sysroot/usr/foo

 # unprefixed
2=/usr/buildroot/output/sysroot/usr/foo    # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

In both cases, line 1 is correctly prefixed, and all other lines are left untouched.

Now, what happens if STAGING_DIR is not inside BASE_DIR, for example BASE_DIR is in home but STAGING_DIR is in /usr:

tdescham at argentina ~ $ BASE_DIR=/home/buildroot/output STAGING_DIR=/usr/buildroot/output/sysroot /tmp/sedv2 < /tmp/testfile 1=/usr/buildroot/output/sysroot/usr/foo

 # unprefixed
2=/usr/buildroot/output/sysroot/usr/foo    # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

tdescham@argentina ~ $ BASE_DIR=/home/buildroot/output STAGING_DIR=/usr/buildroot/output/sysroot /tmp/sedv3 < /tmp/testfile 1=/usr/buildroot/output/sysroot/usr/foo

 # unprefixed
2=/usr/buildroot/output/sysroot/usr/buildroot/output/sysroot/usr/foo
 # prefixed, usr path
3=/home/buildroot/output/sysroot/usr/foo   # prefixed, 'normal' path
4=/bar                                     # dummy

v2 correctly prefixes line 1 and leaves the other untouched. v3 falls back to the original incorrect case and double-prefixes line 2.

So the bottom line of this is that we should go back to the v2 handling. I'll submit a new patch...

Best regards,
Thomas