All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
@ 2016-03-09 13:16 Gustavo Zacarias
  2016-03-09 13:46 ` Peter Korsgaard
  0 siblings, 1 reply; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 13:16 UTC (permalink / raw)
  To: buildroot

Use shell ${STAGING_DIR} to expand at run time instead of build time
hence avoiding hardcoding the staging directory in it and making it
relocatable.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/pkgconf/pkg-config.in | 2 +-
 package/pkgconf/pkgconf.mk    | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
index 4dec487..51ac8c1 100644
--- a/package/pkgconf/pkg-config.in
+++ b/package/pkgconf/pkg-config.in
@@ -1,2 +1,2 @@
 #!/bin/sh
-PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:- at PKG_CONFIG_LIBDIR@} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:- at STAGING_DIR@} $(dirname $0)/pkgconf @STATIC@ $@
+PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${STAGING_DIR}/usr/lib/pkgconfig:${STAGING_DIR}/usr/share/pkgconfig} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${STAGING_DIR}} $(dirname $0)/pkgconf @STATIC@ $@
diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
index c8b0cba..3cfcc2a 100644
--- a/package/pkgconf/pkgconf.mk
+++ b/package/pkgconf/pkgconf.mk
@@ -19,9 +19,6 @@ endef
 define HOST_PKGCONF_INSTALL_WRAPPER
 	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
 		$(HOST_DIR)/usr/bin/pkg-config
-	$(SED) 's, at PKG_CONFIG_LIBDIR@,$(STAGING_DIR)/usr/lib/pkgconfig:$(STAGING_DIR)/usr/share/pkgconfig,' \
-		-e 's, at STAGING_DIR@,$(STAGING_DIR),' \
-		$(HOST_DIR)/usr/bin/pkg-config
 endef
 
 define HOST_PKGCONF_STATIC
-- 
2.4.10

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 13:16 [Buildroot] [PATCH] pkgconf: make wrapper relocatable Gustavo Zacarias
@ 2016-03-09 13:46 ` Peter Korsgaard
  2016-03-09 13:51   ` Gustavo Zacarias
  0 siblings, 1 reply; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 13:46 UTC (permalink / raw)
  To: buildroot

>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

 > Use shell ${STAGING_DIR} to expand at run time instead of build time
 > hence avoiding hardcoding the staging directory in it and making it
 > relocatable.

 > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

Ehh, doesn't this break using pkg-config outside buildroot? 

> ---
 >  package/pkgconf/pkg-config.in | 2 +-
 >  package/pkgconf/pkgconf.mk    | 3 ---
 >  2 files changed, 1 insertion(+), 4 deletions(-)

 > diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
 > index 4dec487..51ac8c1 100644
 > --- a/package/pkgconf/pkg-config.in
 > +++ b/package/pkgconf/pkg-config.in
 > @@ -1,2 +1,2 @@
 >  #!/bin/sh
 > -PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:- at PKG_CONFIG_LIBDIR@} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:- at STAGING_DIR@} $(dirname $0)/pkgconf @STATIC@ $@
 > +PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${STAGING_DIR}/usr/lib/pkgconfig:${STAGING_DIR}/usr/share/pkgconfig} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${STAGING_DIR}} $(dirname $0)/pkgconf @STATIC@ $@
 > diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
 > index c8b0cba..3cfcc2a 100644
 > --- a/package/pkgconf/pkgconf.mk
 > +++ b/package/pkgconf/pkgconf.mk
 > @@ -19,9 +19,6 @@ endef
 >  define HOST_PKGCONF_INSTALL_WRAPPER
 >  	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
 >  		$(HOST_DIR)/usr/bin/pkg-config
 > -	$(SED) 's, at PKG_CONFIG_LIBDIR@,$(STAGING_DIR)/usr/lib/pkgconfig:$(STAGING_DIR)/usr/share/pkgconfig,' \
 > -		-e 's, at STAGING_DIR@,$(STAGING_DIR),' \
 > -		$(HOST_DIR)/usr/bin/pkg-config
 >  endef
 
 >  define HOST_PKGCONF_STATIC
 > -- 
 > 2.4.10

 > _______________________________________________
 > buildroot mailing list
 > buildroot at busybox.net
 > http://lists.busybox.net/mailman/listinfo/buildroot


-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 13:46 ` Peter Korsgaard
@ 2016-03-09 13:51   ` Gustavo Zacarias
  2016-03-09 14:00     ` Peter Korsgaard
  2016-03-09 14:03     ` Thomas Petazzoni
  0 siblings, 2 replies; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 13:51 UTC (permalink / raw)
  To: buildroot

On 09/03/16 10:46, Peter Korsgaard wrote:

>>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:
>
>   > Use shell ${STAGING_DIR} to expand at run time instead of build time
>   > hence avoiding hardcoding the staging directory in it and making it
>   > relocatable.
>
>   > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
>
> Ehh, doesn't this break using pkg-config outside buildroot?

Hi.
Indeed it does, but you can't have the cake and eat it at once can you?
Arnout in the famous vala-wrapper thread pointed that we shouldn't 
hardcode the staging directory in it and i've adjusted accordingly, so 
which way is it? (same logic would apply to both cases). Re:
http://lists.busybox.net/pipermail/buildroot/2016-February/153671.html
Regards.

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 13:51   ` Gustavo Zacarias
@ 2016-03-09 14:00     ` Peter Korsgaard
  2016-03-09 14:04       ` Gustavo Zacarias
  2016-03-09 14:03     ` Thomas Petazzoni
  1 sibling, 1 reply; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 14:00 UTC (permalink / raw)
  To: buildroot

>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

Hi,

 >> Ehh, doesn't this break using pkg-config outside buildroot?

 > Hi.
 > Indeed it does, but you can't have the cake and eat it at once can you?

Well, we cannot really break existing features to add new ones. The
relative location between the pkg-config wrapper script and the staging
directory is constant (or rather known at build time), so I guess we can
do something with /proc/self/exe. We might need to implement the wrapper
in C instead for that to work as /proc/self/exe for a shell script seems
to return /bin/dash here.

 > Arnout in the famous vala-wrapper thread pointed that we shouldn't
 > hardcode the staging directory in it and i've adjusted accordingly, so
 > which way is it? (same logic would apply to both cases). Re:
 > http://lists.busybox.net/pipermail/buildroot/2016-February/153671.html
 > Regards.

I know people today are already using the pkg-config wrapper outside
buildroot (E.G. the output/host dir is used as a sdk), so we imho cannot
break that.

It would be nice to also fix it for vala-wrapper, but as it hasn't
worked before it is atleast not a regression if it doesn't work.

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 13:51   ` Gustavo Zacarias
  2016-03-09 14:00     ` Peter Korsgaard
@ 2016-03-09 14:03     ` Thomas Petazzoni
  2016-03-09 14:28       ` Gustavo Zacarias
                         ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Thomas Petazzoni @ 2016-03-09 14:03 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 9 Mar 2016 10:51:52 -0300, Gustavo Zacarias wrote:

> Indeed it does, but you can't have the cake and eat it at once can you?
> Arnout in the famous vala-wrapper thread pointed that we shouldn't 
> hardcode the staging directory in it and i've adjusted accordingly, so 
> which way is it? (same logic would apply to both cases). Re:
> http://lists.busybox.net/pipermail/buildroot/2016-February/153671.html

What about doing this relatively to the wrapper location?

WRAPPER_DIR=$(dirname $0)
STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)

and there you are, it is both relocatable and doesn't rely on
STAGING_DIR being defined in the environment.

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

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:00     ` Peter Korsgaard
@ 2016-03-09 14:04       ` Gustavo Zacarias
  2016-03-09 14:31         ` Peter Korsgaard
  0 siblings, 1 reply; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 14:04 UTC (permalink / raw)
  To: buildroot

On 09/03/16 11:00, Peter Korsgaard wrote:

> Well, we cannot really break existing features to add new ones. The
> relative location between the pkg-config wrapper script and the staging
> directory is constant (or rather known at build time), so I guess we can
> do something with /proc/self/exe. We might need to implement the wrapper
> in C instead for that to work as /proc/self/exe for a shell script seems
> to return /bin/dash here.

That's overly complicated for no value at all.
You know the sysroot is in the host directory as well, so you just need 
to filter out /usr/bin from $0 and hardcode the tuple in the wrapper - 
that's guaranteed to not change, otherwise you're in serious trouble.
Regards.

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:03     ` Thomas Petazzoni
@ 2016-03-09 14:28       ` Gustavo Zacarias
  2016-03-09 14:40         ` Arnout Vandecappelle
  2016-03-09 14:30       ` Peter Korsgaard
  2016-03-09 15:22       ` Samuel Martin
  2 siblings, 1 reply; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 14:28 UTC (permalink / raw)
  To: buildroot

On 09/03/16 11:03, Thomas Petazzoni wrote:

> What about doing this relatively to the wrapper location?
>
> WRAPPER_DIR=$(dirname $0)
> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
>
> and there you are, it is both relocatable and doesn't rely on
> STAGING_DIR being defined in the environment.
>
> Thomas

Hi.
That won't work, since staging is an absolute symlink, so when moving 
the SDK around it will be broken and fail.
You need something like:

WRAPPER_DIR=$(dirname $0)
STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../@TUPLE@/sysroot)

And hardcode tuple which isn't likely to change.
Regards.

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:03     ` Thomas Petazzoni
  2016-03-09 14:28       ` Gustavo Zacarias
@ 2016-03-09 14:30       ` Peter Korsgaard
  2016-03-09 14:50         ` Thomas Petazzoni
  2016-03-09 15:22       ` Samuel Martin
  2 siblings, 1 reply; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 14:30 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Wed, 9 Mar 2016 10:51:52 -0300, Gustavo Zacarias wrote:

 >> Indeed it does, but you can't have the cake and eat it at once can you?
 >> Arnout in the famous vala-wrapper thread pointed that we shouldn't 
 >> hardcode the staging directory in it and i've adjusted accordingly, so 
 >> which way is it? (same logic would apply to both cases). Re:
 >> http://lists.busybox.net/pipermail/buildroot/2016-February/153671.html

 > What about doing this relatively to the wrapper location?

 > WRAPPER_DIR=$(dirname $0)

That relies on argv[0] containing the full path, which it normally will
do when you run it from the shell, but isn't guaranteed to work. If we
want to support that then we should use /proc/self/exe

> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)

This will only work if you keep the entire buildroot output around (or
rather output/staging) and don't customize the BR2_HOST_DIR.

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:04       ` Gustavo Zacarias
@ 2016-03-09 14:31         ` Peter Korsgaard
  2016-03-09 14:37           ` Arnout Vandecappelle
  0 siblings, 1 reply; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 14:31 UTC (permalink / raw)
  To: buildroot

>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:

 > On 09/03/16 11:00, Peter Korsgaard wrote:
 >> Well, we cannot really break existing features to add new ones. The
 >> relative location between the pkg-config wrapper script and the staging
 >> directory is constant (or rather known at build time), so I guess we can
 >> do something with /proc/self/exe. We might need to implement the wrapper
 >> in C instead for that to work as /proc/self/exe for a shell script seems
 >> to return /bin/dash here.

 > That's overly complicated for no value at all.
 > You know the sysroot is in the host directory as well, so you just
 > need to filter out /usr/bin from $0 and hardcode the tuple in the
 > wrapper - 
 > that's guaranteed to not change, otherwise you're in serious trouble.

Yes, that's basically what I'm saying +/- the question if you want to
rely on $0 containing the full path to the wrapper or if you use
/proc/self/exe instead.

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:31         ` Peter Korsgaard
@ 2016-03-09 14:37           ` Arnout Vandecappelle
  2016-03-09 14:40             ` Gustavo Zacarias
  2016-03-09 14:46             ` Peter Korsgaard
  0 siblings, 2 replies; 21+ messages in thread
From: Arnout Vandecappelle @ 2016-03-09 14:37 UTC (permalink / raw)
  To: buildroot



On 03/09/16 15:31, Peter Korsgaard wrote:
>>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:
>
>   > On 09/03/16 11:00, Peter Korsgaard wrote:
>   >> Well, we cannot really break existing features to add new ones. The
>   >> relative location between the pkg-config wrapper script and the staging
>   >> directory is constant (or rather known at build time), so I guess we can
>   >> do something with /proc/self/exe. We might need to implement the wrapper
>   >> in C instead for that to work as /proc/self/exe for a shell script seems
>   >> to return /bin/dash here.
>
>   > That's overly complicated for no value at all.
>   > You know the sysroot is in the host directory as well, so you just
>   > need to filter out /usr/bin from $0 and hardcode the tuple in the
>   > wrapper -
>   > that's guaranteed to not change, otherwise you're in serious trouble.
>
> Yes, that's basically what I'm saying +/- the question if you want to
> rely on $0 containing the full path to the wrapper or if you use
> /proc/self/exe instead.

  $0 will always contain the full relative path to the wrapper, no? Even if it 
was found through PATH, the PATH search is done by libc so the shell will be 
invoked with the full (absolute or relative) path to the wrapper and use that as 
$0. Or am I missing something?

  I think the /proc/self/exe thing was something to deal with symlinks, which 
isn't the issue here.

  Regards,
  Arnout

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:37           ` Arnout Vandecappelle
@ 2016-03-09 14:40             ` Gustavo Zacarias
  2016-03-09 14:46             ` Peter Korsgaard
  1 sibling, 0 replies; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 14:40 UTC (permalink / raw)
  To: buildroot

On 09/03/16 11:37, Arnout Vandecappelle wrote:

>> Yes, that's basically what I'm saying +/- the question if you want to
>> rely on $0 containing the full path to the wrapper or if you use
>> /proc/self/exe instead.
>
>   $0 will always contain the full relative path to the wrapper, no? Even
> if it was found through PATH, the PATH search is done by libc so the
> shell will be invoked with the full (absolute or relative) path to the
> wrapper and use that as $0. Or am I missing something?
>
>   I think the /proc/self/exe thing was something to deal with symlinks,
> which isn't the issue here.

Indeed, take the following example:

---
#!/bin/sh

TUPLE=powerpc-buildroot-linux-uclibcspe
WRAPPER_DIR=$(dirname $0)
STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../${TUPLE}/sysroot)

echo Invocation: $0
echo Wrapper dir: $WRAPPER_DIR
echo Staging dir: $STAGING_DIR
---

Run full pathspec /home/gustavoz/b/router03/output/host/usr/bin/ja:

Invocation: /home/gustavoz/b/router03/output/host/usr/bin/ja
Wrapper dir: /home/gustavoz/b/router03/output/host/usr/bin
Staging dir: 
/home/gustavoz/b/router03/output/host/usr/powerpc-buildroot-linux-uclibcspe/sysroot

Run in-place ./ja:

Invocation: ./ja
Wrapper dir: .
Staging dir: 
/home/gustavoz/b/router03/output/host/usr/powerpc-buildroot-linux-uclibcspe/sysroot

We can add a check to see if ${STAGING_DIR} is defined on and avoid 
(re)setting it when it is.

if [ -z ${STAGING_DIR} ]; then magic; fi

Regards.

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:28       ` Gustavo Zacarias
@ 2016-03-09 14:40         ` Arnout Vandecappelle
  2016-03-09 14:41           ` Gustavo Zacarias
  2016-03-09 14:42           ` Arnout Vandecappelle
  0 siblings, 2 replies; 21+ messages in thread
From: Arnout Vandecappelle @ 2016-03-09 14:40 UTC (permalink / raw)
  To: buildroot

On 03/09/16 15:28, Gustavo Zacarias wrote:
> On 09/03/16 11:03, Thomas Petazzoni wrote:
>
>> What about doing this relatively to the wrapper location?
>>
>> WRAPPER_DIR=$(dirname $0)
>> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
>>
>> and there you are, it is both relocatable and doesn't rely on
>> STAGING_DIR being defined in the environment.
>>
>> Thomas
>
> Hi.
> That won't work, since staging is an absolute symlink, so when moving the SDK
> around it will be broken and fail.
> You need something like:
>
> WRAPPER_DIR=$(dirname $0)
> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../@TUPLE@/sysroot)
>
> And hardcode tuple which isn't likely to change.

  Not just unlikely to change: if it changes, the sysroot is completely broken.

  So this seems like the best solution to me. Thanks!

  Regards,
  Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:40         ` Arnout Vandecappelle
@ 2016-03-09 14:41           ` Gustavo Zacarias
  2016-03-09 14:42           ` Arnout Vandecappelle
  1 sibling, 0 replies; 21+ messages in thread
From: Gustavo Zacarias @ 2016-03-09 14:41 UTC (permalink / raw)
  To: buildroot

On 09/03/16 11:40, Arnout Vandecappelle wrote:

>   Not just unlikely to change: if it changes, the sysroot is completely
> broken.
>
>   So this seems like the best solution to me. Thanks!

Yes, that's why i said at the beginning "you're in serious trouble" (if 
it changes) ;)
Regards.

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:40         ` Arnout Vandecappelle
  2016-03-09 14:41           ` Gustavo Zacarias
@ 2016-03-09 14:42           ` Arnout Vandecappelle
  1 sibling, 0 replies; 21+ messages in thread
From: Arnout Vandecappelle @ 2016-03-09 14:42 UTC (permalink / raw)
  To: buildroot

On 03/09/16 15:40, Arnout Vandecappelle wrote:
> On 03/09/16 15:28, Gustavo Zacarias wrote:
>> On 09/03/16 11:03, Thomas Petazzoni wrote:
>>
>>> What about doing this relatively to the wrapper location?
>>>
>>> WRAPPER_DIR=$(dirname $0)
>>> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
>>>
>>> and there you are, it is both relocatable and doesn't rely on
>>> STAGING_DIR being defined in the environment.
>>>
>>> Thomas
>>
>> Hi.
>> That won't work, since staging is an absolute symlink, so when moving the SDK
>> around it will be broken and fail.
>> You need something like:
>>
>> WRAPPER_DIR=$(dirname $0)
>> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../@TUPLE@/sysroot)
>>
>> And hardcode tuple which isn't likely to change.
>
>   Not just unlikely to change: if it changes, the sysroot is completely broken.
>
>   So this seems like the best solution to me. Thanks!

  One small thing: you should use $(STAGING_SUBDIR), which is already defined.

  Regards,
  Arnout

>
>   Regards,
>   Arnout
>
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:37           ` Arnout Vandecappelle
  2016-03-09 14:40             ` Gustavo Zacarias
@ 2016-03-09 14:46             ` Peter Korsgaard
  2016-03-09 14:52               ` Arnout Vandecappelle
  1 sibling, 1 reply; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 14:46 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 03/09/16 15:31, Peter Korsgaard wrote:
 >>>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:
 >> 
 >> > On 09/03/16 11:00, Peter Korsgaard wrote:
 >> >> Well, we cannot really break existing features to add new ones. The
 >> >> relative location between the pkg-config wrapper script and the staging
 >> >> directory is constant (or rather known at build time), so I guess we can
 >> >> do something with /proc/self/exe. We might need to implement the wrapper
 >> >> in C instead for that to work as /proc/self/exe for a shell script seems
 >> >> to return /bin/dash here.
 >> 
 >> > That's overly complicated for no value at all.
 >> > You know the sysroot is in the host directory as well, so you just
 >> > need to filter out /usr/bin from $0 and hardcode the tuple in the
 >> > wrapper -
 >> > that's guaranteed to not change, otherwise you're in serious trouble.
 >> 
 >> Yes, that's basically what I'm saying +/- the question if you want to
 >> rely on $0 containing the full path to the wrapper or if you use
 >> /proc/self/exe instead.

 >  $0 will always contain the full relative path to the wrapper, no?
 > Even if it was found through PATH, the PATH search is done by libc so
 > the shell will be invoked with the full (absolute or relative) path to
 > the wrapper and use that as $0. Or am I missing something?

well, if executed throyh a shell it will - But if I write code to call
execve I get to chose what argv[0] should be.

Some software (like upx) wants to handle situations like this, but it
might not be necessary for our pkg-config / vala wrappers.

>  I think the /proc/self/exe thing was something to deal with symlinks,
 > which isn't the issue here.

That's another reason. If somebody does a symlink to our wrapper, then
$0 will contain the full path to the link, not to the wrapper
itself.

If somebody does a hardlink, /proc/self/exe won't even fix it for you -
But that is probably quite unlikely.

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:30       ` Peter Korsgaard
@ 2016-03-09 14:50         ` Thomas Petazzoni
  2016-03-09 14:55           ` Peter Korsgaard
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Petazzoni @ 2016-03-09 14:50 UTC (permalink / raw)
  To: buildroot

Dear Peter Korsgaard,

On Wed, 09 Mar 2016 15:30:18 +0100, Peter Korsgaard wrote:

>  > WRAPPER_DIR=$(dirname $0)
> 
> That relies on argv[0] containing the full path, which it normally will
> do when you run it from the shell, but isn't guaranteed to work. If we
> want to support that then we should use /proc/self/exe

Well, OK.

> > STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
> 
> This will only work if you keep the entire buildroot output around (or
> rather output/staging) and don't customize the BR2_HOST_DIR.

Indeed, so we need to do like Arnout suggests, hardcoding the tuple
inside the script, so that we really only on the contents of host/, and
not on the staging symlink.

Best regards,

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

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:46             ` Peter Korsgaard
@ 2016-03-09 14:52               ` Arnout Vandecappelle
  2016-03-09 15:06                 ` Peter Korsgaard
  0 siblings, 1 reply; 21+ messages in thread
From: Arnout Vandecappelle @ 2016-03-09 14:52 UTC (permalink / raw)
  To: buildroot



On 03/09/16 15:46, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
>
>   > On 03/09/16 15:31, Peter Korsgaard wrote:
>   >>>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:
>   >>
>   >> > On 09/03/16 11:00, Peter Korsgaard wrote:
>   >> >> Well, we cannot really break existing features to add new ones. The
>   >> >> relative location between the pkg-config wrapper script and the staging
>   >> >> directory is constant (or rather known at build time), so I guess we can
>   >> >> do something with /proc/self/exe. We might need to implement the wrapper
>   >> >> in C instead for that to work as /proc/self/exe for a shell script seems
>   >> >> to return /bin/dash here.
>   >>
>   >> > That's overly complicated for no value at all.
>   >> > You know the sysroot is in the host directory as well, so you just
>   >> > need to filter out /usr/bin from $0 and hardcode the tuple in the
>   >> > wrapper -
>   >> > that's guaranteed to not change, otherwise you're in serious trouble.
>   >>
>   >> Yes, that's basically what I'm saying +/- the question if you want to
>   >> rely on $0 containing the full path to the wrapper or if you use
>   >> /proc/self/exe instead.
>
>   >  $0 will always contain the full relative path to the wrapper, no?
>   > Even if it was found through PATH, the PATH search is done by libc so
>   > the shell will be invoked with the full (absolute or relative) path to
>   > the wrapper and use that as $0. Or am I missing something?
>
> well, if executed throyh a shell it will - But if I write code to call
> execve I get to chose what argv[0] should be.

  Read the section "Interpreter scripts" of execve(2). argv[0] is discarded.

>
> Some software (like upx) wants to handle situations like this, but it
> might not be necessary for our pkg-config / vala wrappers.
>
>>   I think the /proc/self/exe thing was something to deal with symlinks,
>   > which isn't the issue here.
>
> That's another reason. If somebody does a symlink to our wrapper, then
> $0 will contain the full path to the link, not to the wrapper
> itself.

  So we need to readlink it first.

> If somebody does a hardlink, /proc/self/exe won't even fix it for you -
> But that is probably quite unlikely.

  For the hardlink case there is no solution.


  Regards,
  Arnout

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:50         ` Thomas Petazzoni
@ 2016-03-09 14:55           ` Peter Korsgaard
  0 siblings, 0 replies; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 14:55 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 >> > STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
 >> 
 >> This will only work if you keep the entire buildroot output around (or
 >> rather output/staging) and don't customize the BR2_HOST_DIR.

 > Indeed, so we need to do like Arnout suggests, hardcoding the tuple
 > inside the script, so that we really only on the contents of host/, and
 > not on the staging symlink.

Yes, like I proposed ;)

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:52               ` Arnout Vandecappelle
@ 2016-03-09 15:06                 ` Peter Korsgaard
  0 siblings, 0 replies; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 15:06 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

>> well, if executed throyh a shell it will - But if I write code to call
 >> execve I get to chose what argv[0] should be.

 >  Read the section "Interpreter scripts" of execve(2). argv[0] is discarded.

Ahh yes, for a script this would be true because of the shebang handling
in the kernel, but it is not true for normal ELF binaries.

>> That's another reason. If somebody does a symlink to our wrapper, then
 >> $0 will contain the full path to the link, not to the wrapper
 >> itself.

 >  So we need to readlink it first.

Yes, or read /proc/self/exec.

 >> If somebody does a hardlink, /proc/self/exe won't even fix it for you -
 >> But that is probably quite unlikely.

 >  For the hardlink case there is no solution.

Indeed.

-- 
Venlig hilsen,
Peter Korsgaard 

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 14:03     ` Thomas Petazzoni
  2016-03-09 14:28       ` Gustavo Zacarias
  2016-03-09 14:30       ` Peter Korsgaard
@ 2016-03-09 15:22       ` Samuel Martin
  2016-03-09 15:55         ` Peter Korsgaard
  2 siblings, 1 reply; 21+ messages in thread
From: Samuel Martin @ 2016-03-09 15:22 UTC (permalink / raw)
  To: buildroot

On Wed, Mar 9, 2016 at 3:03 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 9 Mar 2016 10:51:52 -0300, Gustavo Zacarias wrote:
>
>> Indeed it does, but you can't have the cake and eat it at once can you?
>> Arnout in the famous vala-wrapper thread pointed that we shouldn't
>> hardcode the staging directory in it and i've adjusted accordingly, so
>> which way is it? (same logic would apply to both cases). Re:
>> http://lists.busybox.net/pipermail/buildroot/2016-February/153671.html
>
> What about doing this relatively to the wrapper location?
>
> WRAPPER_DIR=$(dirname $0)
> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
>
> and there you are, it is both relocatable and doesn't rely on
> STAGING_DIR being defined in the environment.

Funny that's close to what I've done in my relocatable sdk branch few
days ago...
https://github.com/tSed/buildroot/commit/cebeb4f43a44eda8e9c1d2fd9629e9f9deea3f28



>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



-- 
Samuel

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

* [Buildroot] [PATCH] pkgconf: make wrapper relocatable
  2016-03-09 15:22       ` Samuel Martin
@ 2016-03-09 15:55         ` Peter Korsgaard
  0 siblings, 0 replies; 21+ messages in thread
From: Peter Korsgaard @ 2016-03-09 15:55 UTC (permalink / raw)
  To: buildroot

>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

Hi,

 >> What about doing this relatively to the wrapper location?
 >> 
 >> WRAPPER_DIR=$(dirname $0)
 >> STAGING_DIR=$(readlink -f ${WRAPPER_DIR}/../../../staging/)
 >> 
 >> and there you are, it is both relocatable and doesn't rely on
 >> STAGING_DIR being defined in the environment.

 > Funny that's close to what I've done in my relocatable sdk branch few
 > days ago...
 > https://github.com/tSed/buildroot/commit/cebeb4f43a44eda8e9c1d2fd9629e9f9deea3f28

Heh, great minds think alike ;)

Something else more-or-less related that I think we should
fix/workaround until the relocatable sdk rpath handling gets integrated
is the issue about the toolchain linking to gmp/mpc/mpfr, and not
finding our libraries when the toolchain is moved.

That is currently quite painful, especially when you want to reuse the
toolchain on different machines/distributions, that may not have
compatible libraries in /usr/lib.

The easiest workaround is just to build our host-gmp/mpc/mpfr statically
for now. This has minimal size impact on the toolchain (< 1MB for a
.tar.gz of output/host).

-- 
Bye, Peter Korsgaard

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

end of thread, other threads:[~2016-03-09 15:55 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 13:16 [Buildroot] [PATCH] pkgconf: make wrapper relocatable Gustavo Zacarias
2016-03-09 13:46 ` Peter Korsgaard
2016-03-09 13:51   ` Gustavo Zacarias
2016-03-09 14:00     ` Peter Korsgaard
2016-03-09 14:04       ` Gustavo Zacarias
2016-03-09 14:31         ` Peter Korsgaard
2016-03-09 14:37           ` Arnout Vandecappelle
2016-03-09 14:40             ` Gustavo Zacarias
2016-03-09 14:46             ` Peter Korsgaard
2016-03-09 14:52               ` Arnout Vandecappelle
2016-03-09 15:06                 ` Peter Korsgaard
2016-03-09 14:03     ` Thomas Petazzoni
2016-03-09 14:28       ` Gustavo Zacarias
2016-03-09 14:40         ` Arnout Vandecappelle
2016-03-09 14:41           ` Gustavo Zacarias
2016-03-09 14:42           ` Arnout Vandecappelle
2016-03-09 14:30       ` Peter Korsgaard
2016-03-09 14:50         ` Thomas Petazzoni
2016-03-09 14:55           ` Peter Korsgaard
2016-03-09 15:22       ` Samuel Martin
2016-03-09 15:55         ` Peter Korsgaard

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.