All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
@ 2014-06-29 12:16 Thomas De Schampheleire
  2014-06-29 14:15 ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2014-06-29 12:16 UTC (permalink / raw)
  To: buildroot

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.

Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 package/pkg-autotools.mk |  19 ++++++++-----------
 package/pkg-generic.mk   |   7 +++----
 2 files changed, 11 insertions(+), 15 deletions(-)

diff -r 4d6d2ac6a601 -r 6d38fb9f8bb5 package/pkg-autotools.mk
--- a/package/pkg-autotools.mk	Thu Jun 19 18:13:36 2014 +0200
+++ b/package/pkg-autotools.mk	Sun Jun 29 13:44:48 2014 +0200
@@ -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
 
diff -r 4d6d2ac6a601 -r 6d38fb9f8bb5 package/pkg-generic.mk
--- a/package/pkg-generic.mk	Thu Jun 19 18:13:36 2014 +0200
+++ b/package/pkg-generic.mk	Sun Jun 29 13:44:48 2014 +0200
@@ -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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
  2014-06-29 12:16 [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands Thomas De Schampheleire
@ 2014-06-29 14:15 ` Thomas Petazzoni
  2014-06-29 14:47   ` Thomas De Schampheleire
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-06-29 14:15 UTC (permalink / raw)
  To: buildroot

Dear Thomas De Schampheleire,

On Sun, 29 Jun 2014 14:16:25 +0200, Thomas De Schampheleire wrote:

> 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
  2014-06-29 14:15 ` Thomas Petazzoni
@ 2014-06-29 14:47   ` Thomas De Schampheleire
  2014-06-29 14:48     ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2014-06-29 14:47 UTC (permalink / raw)
  To: buildroot

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>Dear Thomas De Schampheleire,
>
>On Sun, 29 Jun 2014 14:16:25 +0200, Thomas De Schampheleire wrote:
>
>> 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 ?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
  2014-06-29 14:47   ` Thomas De Schampheleire
@ 2014-06-29 14:48     ` Thomas Petazzoni
  2014-06-29 15:42       ` Thomas De Schampheleire
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-06-29 14:48 UTC (permalink / raw)
  To: buildroot

Dear Thomas De Schampheleire,

On Sun, 29 Jun 2014 16:47:27 +0200, Thomas De Schampheleire wrote:

> >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 ;-)

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
  2014-06-29 14:48     ` Thomas Petazzoni
@ 2014-06-29 15:42       ` Thomas De Schampheleire
  2014-06-30  7:57         ` Thomas De Schampheleire
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2014-06-29 15:42 UTC (permalink / raw)
  To: buildroot

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>Dear Thomas De Schampheleire,
>
>On Sun, 29 Jun 2014 16:47:27 +0200, Thomas De Schampheleire wrote:
>
>> >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 ;-)
>
>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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands
  2014-06-29 15:42       ` Thomas De Schampheleire
@ 2014-06-30  7:57         ` Thomas De Schampheleire
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas De Schampheleire @ 2014-06-30  7:57 UTC (permalink / raw)
  To: buildroot

Hi,

On Sun, Jun 29, 2014 at 5:42 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>>Dear Thomas De Schampheleire,
>>
>>On Sun, 29 Jun 2014 16:47:27 +0200, Thomas De Schampheleire wrote:
>>
>>> >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 ;-)
>>
>>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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-06-30  7:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-29 12:16 [Buildroot] [PATCH] infra: simplify .la/.pc fixup commands Thomas De Schampheleire
2014-06-29 14:15 ` Thomas Petazzoni
2014-06-29 14:47   ` Thomas De Schampheleire
2014-06-29 14:48     ` Thomas Petazzoni
2014-06-29 15:42       ` Thomas De Schampheleire
2014-06-30  7:57         ` Thomas De Schampheleire

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.