[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Wolfgang Grandegger]

Hello,

this is v5 of my patch series to make the buildroot SDK (HOST_DIR)
relocatable. It sanitizes the RPATH of all ELF files in the "target",
"staging" and "host" tree using "patchelf --make-rpath-relative". I
have started the mainlining process to implement "--make-rpath-relative"
using GitHub pull request [1]... till now, no answer!

In contrast to v4, this series now does RPATH sanitation per package
after package installation into the host, staging or target tree
using GLOBAL_INSTRUMENTATION_HOOKS, similar to "check-bin-arch" or
"check-host-rpath". This is a first hack to demonstrate feasibility.
The scripts in "pkg-generic.mk" needs some more polish to be more
efficient and elegant. See also "Changes since v4" below.

Furthermore this patch creates the script "relocate-sdk.sh" in the top
directory of the "host" tree allowing to relocate the SDK after it has
been moved to a new location. It replaces the old path with the new
one in all text files identified by "file --mime-type". The location
is stored in "usr/share/buildroot/sdk-location".

Unfortunately, "qmake" uses hard-coded pathes compiled into the QT5
libraries. To overcome this problem, "qt5pase" now creates "qt.conf".


Other Questions:

Things not yet addressed:

- "make toolchain" creates a toolchain tree which still has references
  to the build system (in ELF and text files).

Changes since v4:

- RPATH sanitation is now done per package and installation step
  in "pkg-generic.mk".
- After the installation step, the list of the installed files is
  stored in "<package-build-dir>/.br_[host|staging|target]_filelist".
- The GLOBAL_INSTRUMENTATION_HOOKS "step_sanitize_rpath" then calls
  "fix-rpath" to do the sanitation.
- DEPENDENCIES_HOST_PREREQ += host-patchelf is set early in the
  Makefile as we need it for RPATH sanitation. As soon it's available
  sanitation can start (currently missing some packages).
- The patchelf "file busy" issue is now worked-around differently.

Changes since v3:

- The patchelf patch implementing " --make-rpath-relative" now supports
  the option "--relative-to-file" instructing to use "$ORIGIN" in
  RPATHs. Otherwise an absolute path relative to the root directory will
  be used.
- The staging tree is now sanitized as well using the options
  "--relative-to-file" and "--no-standard-libs".
- For the "target" tree, relative RPATHs do not use "$ORIGIN" any
  longer. An absolute path relative to the root directory is used
  instead.

Changes since v2:

- provide "qt.conf" to make "qmake" relocatable
- sed now uses the separator "\" to substitute the directory path.
  It's one of the few characters not allowed in file names. To
  avoid interpreting it as escape character, the "read -r" is used.
- The paranoia substituion check is done before doing the real
  substituion.

Changes since v1:

- The name SDK has been chosen for the relocatabed "HOST_DIR" (instead
  of toolchanin).
- The patchelf version bump and patching are now done by 2 patches
- No more helper functions are used in the Makefile to call "fix-rpath"
  but added directly.
- The staging tree is not touched any more... until we have a good
  reason to do so. 
- The sanitation is now performed by an optimized "fix-rpath" script.
- The relocate-sdk script is now copied for support/misc to the
  top directory of the host tree.

[1] https://github.com/NixOS/patchelf/pull/118

Samuel Martin (1):
  support/scripts: add fix-rpath script to sanitize the rpath

Wolfgang Grandegger (10):
  package/patchelf: use most recent version as a base for rpath
    sanitation
  package/patchelf: add patch for rpath sanitation under a root
    directory
  pkg-generic: create a list af installed files per step and package
  core: we need host-patchelf as the very first package for RPATH
    sanitation
  pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation
  support/scripts: add reloacte-sdk.sh script for SDK relocation
  core: install relocation script and location at the end of the build
  external-toolchain: check if a buildroot SDK has already been
    relocated
  support/scripts: check-host-rpath now handles $ORIGIN as well
  package/qt5base: provide "qt.conf" to make "qmake" relocatable

 Makefile                                           |   5 +
 ...to-make-the-rpath-relative-under-a-specif.patch | 326 +++++++++++++++++++++
 package/patchelf/patchelf.hash                     |   2 +-
 package/patchelf/patchelf.mk                       |   8 +-
 package/pkg-generic.mk                             |  56 +++-
 package/qt5/qt5base/qt.conf.in                     |   6 +
 package/qt5/qt5base/qt5base.mk                     |   8 +
 support/misc/relocate-sdk.sh                       |  47 +++
 support/scripts/check-host-rpath                   |   2 +-
 support/scripts/fix-rpath                          | 100 +++++++
 toolchain/helpers.mk                               |  15 +
 .../toolchain-external/pkg-toolchain-external.mk   |   1 +
 12 files changed, 556 insertions(+), 20 deletions(-)
 create mode 100644 package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
 create mode 100644 package/qt5/qt5base/qt.conf.in
 create mode 100755 support/misc/relocate-sdk.sh
 create mode 100755 support/scripts/fix-rpath

-- 
2.7.4

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Wolfgang Grandegger]

We would like to use "patchelf" to do rpath sanitation of all ELF files
in the "host", "staging" and "target" directory, mainly because a script
based solutions is too complex and slow.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 package/patchelf/patchelf.hash | 2 +-
 package/patchelf/patchelf.mk   | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/package/patchelf/patchelf.hash b/package/patchelf/patchelf.hash
index 653eb46..95b067b 100644
--- a/package/patchelf/patchelf.hash
+++ b/package/patchelf/patchelf.hash
@@ -1,2 +1,2 @@
 # Locally calculated
-sha256	a0f65c1ba148890e9f2f7823f4bedf7ecad5417772f64f994004f59a39014f83	patchelf-0.9.tar.bz2
+sha256  c8f1e4d2d41d5b390931e9876ccab39050182dc0003e865d3edd06684dbf9d8d  patchelf-29c085fd9d3fc972f75b3961905d6b4ecce7eb2b.tar.gz
diff --git a/package/patchelf/patchelf.mk b/package/patchelf/patchelf.mk
index 74e6ccc..9750351 100644
--- a/package/patchelf/patchelf.mk
+++ b/package/patchelf/patchelf.mk
@@ -4,10 +4,10 @@
 #
 ################################################################################
 
-PATCHELF_VERSION = 0.9
-PATCHELF_SITE = http://releases.nixos.org/patchelf/patchelf-$(PATCHELF_VERSION)
-PATCHELF_SOURCE = patchelf-$(PATCHELF_VERSION).tar.bz2
-PATCHELF_LICENSE = GPL-3.0+
+PATCHELF_VERSION = 29c085fd9d3fc972f75b3961905d6b4ecce7eb2b
+PATCHELF_SITE = $(call github,NixOS,patchelf,$(PATCHELF_VERSION))
+PATCHELF_AUTORECONF = YES
+PATCHELF_LICENSE = GPLv3+
 PATCHELF_LICENSE_FILES = COPYING
 
 $(eval $(host-autotools-package))
-- 
2.7.4

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 30-06-17 10:36, Wolfgang Grandegger wrote:
> We would like to use "patchelf" to do rpath sanitation of all ELF files
> in the "host", "staging" and "target" directory, mainly because a script
> based solutions is too complex and slow.

 Note that the bump is needed because some features were introduced since 0.9
that we need for out rpath sanitization.

> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  package/patchelf/patchelf.hash | 2 +-
>  package/patchelf/patchelf.mk   | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/package/patchelf/patchelf.hash b/package/patchelf/patchelf.hash
> index 653eb46..95b067b 100644
> --- a/package/patchelf/patchelf.hash
> +++ b/package/patchelf/patchelf.hash
> @@ -1,2 +1,2 @@
>  # Locally calculated
> -sha256	a0f65c1ba148890e9f2f7823f4bedf7ecad5417772f64f994004f59a39014f83	patchelf-0.9.tar.bz2
> +sha256  c8f1e4d2d41d5b390931e9876ccab39050182dc0003e865d3edd06684dbf9d8d  patchelf-29c085fd9d3fc972f75b3961905d6b4ecce7eb2b.tar.gz
> diff --git a/package/patchelf/patchelf.mk b/package/patchelf/patchelf.mk
> index 74e6ccc..9750351 100644
> --- a/package/patchelf/patchelf.mk
> +++ b/package/patchelf/patchelf.mk
> @@ -4,10 +4,10 @@
>  #
>  ################################################################################
>  
> -PATCHELF_VERSION = 0.9
> -PATCHELF_SITE = http://releases.nixos.org/patchelf/patchelf-$(PATCHELF_VERSION)
> -PATCHELF_SOURCE = patchelf-$(PATCHELF_VERSION).tar.bz2
> -PATCHELF_LICENSE = GPL-3.0+
> +PATCHELF_VERSION = 29c085fd9d3fc972f75b3961905d6b4ecce7eb2b
> +PATCHELF_SITE = $(call github,NixOS,patchelf,$(PATCHELF_VERSION))
> +PATCHELF_AUTORECONF = YES

 Please add a comment above that:
# Fetched from Github, with no configure script

> +PATCHELF_LICENSE = GPLv3+

 This was not the intention I guess?

 Regards,
 Arnout

>  PATCHELF_LICENSE_FILES = COPYING
>  
>  $(eval $(host-autotools-package))
> 

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

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Wolfgang Grandegger]

Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
> 
> 
> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>> We would like to use "patchelf" to do rpath sanitation of all ELF files
>> in the "host", "staging" and "target" directory, mainly because a script
>> based solutions is too complex and slow.
> 
>   Note that the bump is needed because some features were introduced since 0.9
> that we need for out rpath sanitization.

OK. Strictly speaking, we don't need/use the new stuff. The patch could also be
adapted to 0.0. I mainly used the most recent version because of the mainlining
process.

>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   package/patchelf/patchelf.hash | 2 +-
>>   package/patchelf/patchelf.mk   | 8 ++++----
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/patchelf/patchelf.hash b/package/patchelf/patchelf.hash
>> index 653eb46..95b067b 100644
>> --- a/package/patchelf/patchelf.hash
>> +++ b/package/patchelf/patchelf.hash
>> @@ -1,2 +1,2 @@
>>   # Locally calculated
>> -sha256	a0f65c1ba148890e9f2f7823f4bedf7ecad5417772f64f994004f59a39014f83	patchelf-0.9.tar.bz2
>> +sha256  c8f1e4d2d41d5b390931e9876ccab39050182dc0003e865d3edd06684dbf9d8d  patchelf-29c085fd9d3fc972f75b3961905d6b4ecce7eb2b.tar.gz
>> diff --git a/package/patchelf/patchelf.mk b/package/patchelf/patchelf.mk
>> index 74e6ccc..9750351 100644
>> --- a/package/patchelf/patchelf.mk
>> +++ b/package/patchelf/patchelf.mk
>> @@ -4,10 +4,10 @@
>>   #
>>   ################################################################################
>>   
>> -PATCHELF_VERSION = 0.9
>> -PATCHELF_SITE = http://releases.nixos.org/patchelf/patchelf-$(PATCHELF_VERSION)
>> -PATCHELF_SOURCE = patchelf-$(PATCHELF_VERSION).tar.bz2
>> -PATCHELF_LICENSE = GPL-3.0+
>> +PATCHELF_VERSION = 29c085fd9d3fc972f75b3961905d6b4ecce7eb2b
>> +PATCHELF_SITE = $(call github,NixOS,patchelf,$(PATCHELF_VERSION))
>> +PATCHELF_AUTORECONF = YES
> 
>   Please add a comment above that:
> # Fetched from Github, with no configure script

OK.

>> +PATCHELF_LICENSE = GPLv3+
> 
>   This was not the intention I guess?

Well, that's the licence used for the new version:

https://github.com/NixOS/patchelf/commit/e75749b84f1511fb14a4027d25a75ad9f867e25f

Wolfgang.

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 30-06-17 19:33, Wolfgang Grandegger wrote:
> Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
>>
>>
>> On 30-06-17 10:36, Wolfgang Grandegger wrote:

[snip]
>>> -PATCHELF_LICENSE = GPL-3.0+
[snip]
>>> +PATCHELF_LICENSE = GPLv3+
>>
>>   This was not the intention I guess?
> 
> Well, that's the licence used for the new version:

 It was already the license of the old version, but we switched to SPDX license
strings and you reverted to the old custom way.

 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

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 30-06-17 19:33, Wolfgang Grandegger wrote:
> Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
>>
>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>> We would like to use "patchelf" to do rpath sanitation of all ELF files
>>> in the "host", "staging" and "target" directory, mainly because a script
>>> based solutions is too complex and slow.
>>   Note that the bump is needed because some features were introduced since 0.9
>> that we need for out rpath sanitization.
> OK. Strictly speaking, we don't need/use the new stuff. The patch could also be
> adapted to 0.0. I mainly used the most recent version because of the mainlining
> process.

 Perhaps we should stick to 0.9 then. Since then, they have started using C++11
features, and I am not sure they are supported by the gcc in RHEL5. Since we
will always use patchelf, it will have to be buildable on any host that we
support...

 Adding ThomasDS in Cc who I believe is still using RHEL5. Thomas, can you clone
https://github.com/nixos/patchelf and see if it builds? Perhaps you have to
install gcc44, would that work for you? I've tried to do it myself but I wasn't
able to run a 'yum install' in a CentOs5.6 VM... It seems none of the mirrors
work anymore.

 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

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 01-07-17 11:40, Arnout Vandecappelle wrote:
> 
> 
> On 30-06-17 19:33, Wolfgang Grandegger wrote:
>> Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
>>>
>>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>>> We would like to use "patchelf" to do rpath sanitation of all ELF files
>>>> in the "host", "staging" and "target" directory, mainly because a script
>>>> based solutions is too complex and slow.
>>>   Note that the bump is needed because some features were introduced since 0.9
>>> that we need for out rpath sanitization.
>> OK. Strictly speaking, we don't need/use the new stuff. The patch could also be
>> adapted to 0.0. I mainly used the most recent version because of the mainlining
>> process.
> 
>  Perhaps we should stick to 0.9 then. Since then, they have started using C++11
> features, and I am not sure they are supported by the gcc in RHEL5. Since we
> will always use patchelf, it will have to be buildable on any host that we
> support...

 OK, so we have a problem here: current patchelf master doesn't build with gcc
4.4 because of the use of C++11 constructs. Here are our options:

1. Base our patchelf on 0.9 still instead of current master.

2. Base on master and add patches to make it build again on gcc 4.4 (which at
least has *some* C++11 support). Most of these patches would not be upstreamable
though.

3. Like 1/2 but fork patchelf entirely, i.e. keep the patching we do outside of
Buildroot, in the forked repository. This is a bit easier to maintain than
patches within Buildroot, and is as easy for the users.

4. Require gcc 4.6 on the host (if that is indeed sufficient to build patchelf).
This means abandoning support for Debian squeeze (i.e. three releases before
current stable) and RHEL5 (Ubuntu 12.04, the previous-previous LTS, is still OK).

5. Replace patchelf with something else that fits better. Two options:
pyelftools and chrpath. Neither looks to promising: chrpath is simply worse than
patchelf (doesn't really abbreviate RPATH, it just fills it with zeroes, and
doesn't support making it longer). pyelftools looks interesting, but it's Python
so we would normally install it as a host-python-package which would complicate
things enormously.


 My feeling is that we should stick to option 1 and revisit when it turns out
that we really want some upstream features added later.


 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

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 01-07-17 17:24, Arnout Vandecappelle wrote:
> 
> 
> On 01-07-17 11:40, Arnout Vandecappelle wrote:
>>
>>
>> On 30-06-17 19:33, Wolfgang Grandegger wrote:
>>> Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
>>>>
>>>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>>>> We would like to use "patchelf" to do rpath sanitation of all ELF files
>>>>> in the "host", "staging" and "target" directory, mainly because a script
>>>>> based solutions is too complex and slow.
>>>>   Note that the bump is needed because some features were introduced since 0.9
>>>> that we need for out rpath sanitization.
>>> OK. Strictly speaking, we don't need/use the new stuff. The patch could also be
>>> adapted to 0.0. I mainly used the most recent version because of the mainlining
>>> process.
>>
>>  Perhaps we should stick to 0.9 then. Since then, they have started using C++11
>> features, and I am not sure they are supported by the gcc in RHEL5. Since we
>> will always use patchelf, it will have to be buildable on any host that we
>> support...
> 
>  OK, so we have a problem here: current patchelf master doesn't build with gcc
> 4.4 because of the use of C++11 constructs. Here are our options:
> 
> 1. Base our patchelf on 0.9 still instead of current master.

 After discussion here at the BR Summer Camp, we decided to stick with option 1
(stay on patchelf 0.9) and revisit later when either patchelf grows interesting
additional features upstream, or we anyway decide to drop old Debian and RHEL.

 Regards,
 Arnout

> 
> 2. Base on master and add patches to make it build again on gcc 4.4 (which at
> least has *some* C++11 support). Most of these patches would not be upstreamable
> though.
> 
> 3. Like 1/2 but fork patchelf entirely, i.e. keep the patching we do outside of
> Buildroot, in the forked repository. This is a bit easier to maintain than
> patches within Buildroot, and is as easy for the users.
> 
> 4. Require gcc 4.6 on the host (if that is indeed sufficient to build patchelf).
> This means abandoning support for Debian squeeze (i.e. three releases before
> current stable) and RHEL5 (Ubuntu 12.04, the previous-previous LTS, is still OK).
> 
> 5. Replace patchelf with something else that fits better. Two options:
> pyelftools and chrpath. Neither looks to promising: chrpath is simply worse than
> patchelf (doesn't really abbreviate RPATH, it just fills it with zeroes, and
> doesn't support making it longer). pyelftools looks interesting, but it's Python
> so we would normally install it as a host-python-package which would complicate
> things enormously.
> 
> 
>  My feeling is that we should stick to option 1 and revisit when it turns out
> that we really want some upstream features added later.
> 
> 
>  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

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Wolfgang Grandegger]

Am 02.07.2017 um 18:10 schrieb Arnout Vandecappelle:
> 
> 
> On 01-07-17 17:24, Arnout Vandecappelle wrote:
>>
>>
>> On 01-07-17 11:40, Arnout Vandecappelle wrote:
>>>
>>>
>>> On 30-06-17 19:33, Wolfgang Grandegger wrote:
>>>> Am 30.06.2017 um 19:15 schrieb Arnout Vandecappelle:
>>>>>
>>>>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>>>>> We would like to use "patchelf" to do rpath sanitation of all ELF files
>>>>>> in the "host", "staging" and "target" directory, mainly because a script
>>>>>> based solutions is too complex and slow.
>>>>>    Note that the bump is needed because some features were introduced since 0.9
>>>>> that we need for out rpath sanitization.
>>>> OK. Strictly speaking, we don't need/use the new stuff. The patch could also be
>>>> adapted to 0.0. I mainly used the most recent version because of the mainlining
>>>> process.
>>>
>>>   Perhaps we should stick to 0.9 then. Since then, they have started using C++11
>>> features, and I am not sure they are supported by the gcc in RHEL5. Since we
>>> will always use patchelf, it will have to be buildable on any host that we
>>> support...
>>
>>   OK, so we have a problem here: current patchelf master doesn't build with gcc
>> 4.4 because of the use of C++11 constructs. Here are our options:
>>
>> 1. Base our patchelf on 0.9 still instead of current master.
> 
>   After discussion here at the BR Summer Camp, we decided to stick with option 1
> (stay on patchelf 0.9) and revisit later when either patchelf grows interesting
> additional features upstream, or we anyway decide to drop old Debian and RHEL.

OK, I will use v0.9 plus a few patches (relevant for rpath 
sanitization). I see at least:

   a365bcb Remove apparently incorrect usage of "static"


Wolfgang.

[Buildroot] [RFC PATCH v5 01/11] package/patchelf: use most recent version as a base for rpath sanitation

[Arnout Vandecappelle]

On 03-07-17 09:04, Wolfgang Grandegger wrote:
[snip]
> OK, I will use v0.9 plus a few patches (relevant for rpath sanitization). I see
> at least:
> 
>   a365bcb Remove apparently incorrect usage of "static"

 Well, actually, if you keep it static, that resolves the issue that you have
with libs occurring twice. But it's certainly OK for me if you keep this patch
and then resolve it by adding neededLibFound as an argument.

 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

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Wolfgang Grandegger]

The patch allows to use patchelf to sanitize the rpath of the buildroot
libraries and binaries using the option "--make-rpath-relative <rootdir>".

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 ...to-make-the-rpath-relative-under-a-specif.patch | 326 +++++++++++++++++++++
 1 file changed, 326 insertions(+)
 create mode 100644 package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch

diff --git a/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
new file mode 100644
index 0000000..609eacf
--- /dev/null
+++ b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
@@ -0,0 +1,326 @@
+From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
+From: Wolfgang Grandegger <wg@grandegger.com>
+Date: Mon, 20 Feb 2017 16:29:24 +0100
+Subject: [PATCH] Add option to make the rpath relative under a specified root
+ directory
+
+Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will
+modify or delete the RPATHDIRs according the following rules
+similar to Martin's patches [1] making the Buildroot toolchaing/SDK
+relocatable.
+
+RPATHDIR starts with "$ORIGIN":
+    The original build-system already took care of setting a relative
+    RPATH, resolve it and test if it's valid (does exist)
+
+RPATHDIR starts with ROOTDIR:
+    The original build-system added some absolute RPATH (absolute on
+    the build machine). Test if it's valid (does exist).
+
+ROOTDIR/RPATHDIR exists:
+    The original build-system already took care of setting an absolute
+    RPATH (absolute in the final rootfs), resolve it and test if it's
+    valid (does exist).
+
+RPATHDIR points somewhere else:
+    (can be anywhere: build trees, staging tree, host location,
+    non-existing location, etc.). Just discard such a path.
+
+The option "--no-standard-libs" will discard RPATHDIRs ROOTDIR/lib and
+ROOTDIR/usr/lib. Like "--shrink-rpath", RPATHDIRs are also discarded
+if the directories do not contain a library referenced by the
+DT_NEEDED fields.
+If the option "--relative-to-file" is given, the rpath will start
+with "$ORIGIN" making it relative to the ELF file, otherwise an
+absolute path relative to ROOTDIR will be used.
+
+[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html
+
+Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
+---
+ src/patchelf.cc | 187 ++++++++++++++++++++++++++++++++++++++++++++++++--------
+ 1 file changed, 161 insertions(+), 26 deletions(-)
+
+diff --git a/src/patchelf.cc b/src/patchelf.cc
+index 55b38e3..60295b3 100644
+--- a/src/patchelf.cc
++++ b/src/patchelf.cc
+@@ -50,6 +50,9 @@ static int pageSize = PAGESIZE;
+ 
+ typedef std::shared_ptr<std::vector<unsigned char>> FileContents;
+ 
++#define MODIFY_FLAG_NO_STD_LIB_DIRS 0x1
++#define MODIFY_FLAG_RELATIVE_TO_FILE 0x2
++static int modifyFlags;
+ 
+ #define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym, class Elf_Verneed
+ #define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym, Elf_Verneed
+@@ -84,6 +87,36 @@ static unsigned int getPageSize()
+     return pageSize;
+ }
+ 
++static bool absolutePathExists(const std::string & path, std::string & canonicalPath)
++{
++    char *cpath = realpath(path.c_str(), NULL);
++    if (cpath) {
++        canonicalPath = cpath;
++        free(cpath);
++        return true;
++    } else {
++        return false;
++    }
++}
++
++static std::string makePathRelative(const std::string & path,
++    const std::string & refPath, const std::string & rootDir)
++{
++    std::string relPath = "$ORIGIN";
++
++    /* Strip root path first */
++    std::string p = path.substr(rootDir.length());
++    std::string refP = refPath.substr(rootDir.length());
++
++    std::size_t pos = refP.find_first_of('/');
++    while (pos != std::string::npos) {
++        pos =refP.find_first_of('/', pos + 1);
++        relPath.append("/..");
++    }
++    relPath.append(p);
++
++    return relPath;
++}
+ 
+ template<ElfFileParams>
+ class ElfFile
+@@ -194,9 +227,14 @@ public:
+ 
+     void setInterpreter(const std::string & newInterpreter);
+ 
+-    typedef enum { rpPrint, rpShrink, rpSet, rpRemove } RPathOp;
++    typedef enum { rpPrint, rpShrink, rpMakeRelative, rpSet, rpRemove} RPathOp;
+ 
+-    void modifyRPath(RPathOp op, const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath);
++    bool libFoundInRPath(const std::string & dirName,
++                         const std::vector<std::string> neededLibs);
++
++    void modifyRPath(RPathOp op,
++                     const std::vector<std::string> & allowedRpathPrefixes,
++                     std::string rootDir, int flags, std::string newRPath);
+ 
+     void addNeeded(const std::set<std::string> & libs);
+ 
+@@ -1108,10 +1146,35 @@ static void concatToRPath(std::string & rpath, const std::string & path)
+     rpath += path;
+ }
+ 
++template<ElfFileParams>
++bool ElfFile<ElfFileParamNames>::libFoundInRPath(const std::string & dirName,
++    const std::vector<std::string> neededLibs)
++{
++    std::vector<bool> neededLibFound(neededLibs.size(), false);
++
++    /* For each library that we haven't found yet, see if it
++       exists in this directory. */
++    bool libFound = false;
++    for (unsigned int j = 0; j < neededLibs.size(); ++j)
++        if (!neededLibFound[j]) {
++            std::string libName = dirName + "/" + neededLibs[j];
++            try {
++                if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
++                    neededLibFound[j] = true;
++                    libFound = true;
++                } else
++                    debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
++            } catch (SysError & e) {
++                if (e.errNo != ENOENT) throw;
++            }
++        }
++    return libFound;
++}
+ 
+ template<ElfFileParams>
+ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
+-    const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath)
++    const std::vector<std::string> & allowedRpathPrefixes,
++    std::string rootDir, int flags, std::string newRPath)
+ {
+     Elf_Shdr & shdrDynamic = findSection(".dynamic");
+ 
+@@ -1162,11 +1225,14 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
+         return;
+     }
+ 
++    if (op == rpMakeRelative && !rpath) {
++        debug("no RPATH to make relative\n");
++        return;
++    }
+ 
+     /* For each directory in the RPATH, check if it contains any
+        needed library. */
+     if (op == rpShrink) {
+-        std::vector<bool> neededLibFound(neededLibs.size(), false);
+ 
+         newRPath = "";
+ 
+@@ -1186,30 +1252,81 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
+                 continue;
+             }
+ 
+-            /* For each library that we haven't found yet, see if it
+-               exists in this directory. */
+-            bool libFound = false;
+-            for (unsigned int j = 0; j < neededLibs.size(); ++j)
+-                if (!neededLibFound[j]) {
+-                    std::string libName = dirName + "/" + neededLibs[j];
+-                    try {
+-                        if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
+-                            neededLibFound[j] = true;
+-                            libFound = true;
+-                        } else
+-                            debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
+-                    } catch (SysError & e) {
+-                        if (e.errNo != ENOENT) throw;
+-                    }
+-                }
+-
+-            if (!libFound)
++            if (!libFoundInRPath(dirName, neededLibs))
+                 debug("removing directory '%s' from RPATH\n", dirName.c_str());
+             else
+                 concatToRPath(newRPath, dirName);
+         }
+     }
+ 
++    /* Make the the RPATH relative to the specified path */
++    if (op == rpMakeRelative) {
++        std::string fileDir = fileName.substr(0, fileName.find_last_of("/"));
++        newRPath = "";
++
++        for (auto & dirName : splitColonDelimitedString(rpath)) {
++            std::string canonicalPath;
++
++            /* Figure out if we should keep or discard the path. There are several
++               cases to be handled:
++               "dirName" starts with "$ORIGIN":
++                   The original build-system already took care of setting a relative
++                   RPATH. Resolve it and test if it's valid (does exist).
++               "dirName" start with "rootDir":
++                   The original build-system added some absolute RPATH (absolute on
++                   the build machine). Test if it's valid (does exist).
++               "rootDir"/"dirName" exists:
++                    The original build-system already took care of setting an absolute
++                    RPATH (absolute in the final rootfs). Resolve it and test if it's
++                    valid (does exist).
++               "dirName" points somewhere else:
++                    (can be anywhere: build trees, staging tree, host location,
++                    non-existing location, etc.). Just discard such a path. */
++            if (!dirName.compare(0, 7, "$ORIGIN")) {
++                std::string path = fileDir + dirName.substr(7);
++                if (!absolutePathExists(path, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because '%s' doesn't exist\n",
++			  dirName.c_str(), path.c_str());
++                    continue;
++                }
++            } else if (!dirName.compare(0, rootDir.length(), rootDir)) {
++                if (!absolutePathExists(dirName, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because it doesn't exist\n", dirName.c_str());
++                    continue;
++                }
++            } else {
++                std::string path = rootDir + dirName;
++                if (!absolutePathExists(path, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because it's not in rootdir\n",
++			  dirName.c_str());
++                    continue;
++                }
++            }
++
++            if (flags & MODIFY_FLAG_NO_STD_LIB_DIRS) {
++                if (!canonicalPath.compare(rootDir + "/lib") ||
++                    !canonicalPath.compare(rootDir + "/usr/lib")) {
++                    debug("removing directory '%s' from RPATH because it's a standard library directory\n",
++                         dirName.c_str());
++                    continue;
++                }
++            }
++
++            if (!libFoundInRPath(canonicalPath, neededLibs)) {
++                debug("removing directory '%s' from RPATH because it does not contain needed libs\n",
++		      dirName.c_str());
++                continue;
++            }
++
++            /* Finally make "canonicalPath" relative to "filedir" in "rootDir" */
++	    if (flags & MODIFY_FLAG_RELATIVE_TO_FILE)
++		concatToRPath(newRPath, makePathRelative(canonicalPath, fileDir, rootDir));
++	    else
++		concatToRPath(newRPath, canonicalPath.substr(rootDir.length()));
++	    debug("keeping relative path of %s\n", canonicalPath.c_str());
++	}
++    }
++
+     if (op == rpRemove) {
+         if (!rpath) {
+             debug("no RPATH to delete\n");
+@@ -1538,7 +1655,9 @@ static std::vector<std::string> allowedRpathPrefixes;
+ static bool removeRPath = false;
+ static bool setRPath = false;
+ static bool printRPath = false;
++static bool makeRPathRelative = false;
+ static std::string newRPath;
++static std::string rootDir;
+ static std::set<std::string> neededLibsToRemove;
+ static std::map<std::string, std::string> neededLibsToReplace;
+ static std::set<std::string> neededLibsToAdd;
+@@ -1561,14 +1680,16 @@ static void patchElf2(ElfFile && elfFile)
+         elfFile.setInterpreter(newInterpreter);
+ 
+     if (printRPath)
+-        elfFile.modifyRPath(elfFile.rpPrint, {}, "");
++        elfFile.modifyRPath(elfFile.rpPrint, {}, {}, modifyFlags, "");
+ 
+     if (shrinkRPath)
+-        elfFile.modifyRPath(elfFile.rpShrink, allowedRpathPrefixes, "");
++        elfFile.modifyRPath(elfFile.rpShrink, allowedRpathPrefixes, "", modifyFlags, "");
+     else if (removeRPath)
+-        elfFile.modifyRPath(elfFile.rpRemove, {}, "");
++        elfFile.modifyRPath(elfFile.rpRemove, {}, "", modifyFlags, "");
+     else if (setRPath)
+-        elfFile.modifyRPath(elfFile.rpSet, {}, newRPath);
++        elfFile.modifyRPath(elfFile.rpSet, {}, "", modifyFlags, newRPath);
++    else if (makeRPathRelative)
++        elfFile.modifyRPath(elfFile.rpMakeRelative, {}, rootDir, modifyFlags, "");
+ 
+     if (printNeeded) elfFile.printNeededLibs();
+ 
+@@ -1614,6 +1735,9 @@ void showHelp(const std::string & progName)
+   [--remove-rpath]\n\
+   [--shrink-rpath]\n\
+   [--allowed-rpath-prefixes PREFIXES]\t\tWith '--shrink-rpath', reject rpath entries not starting with the allowed prefix\n\
++  [--make-rpath-relative ROOTDIR]\n\
++  [--no-standard-lib-dirs]\n\
++  [--relative-to-file]\n\
+   [--print-rpath]\n\
+   [--force-rpath]\n\
+   [--add-needed LIBRARY]\n\
+@@ -1674,6 +1798,17 @@ int mainWrapped(int argc, char * * argv)
+             setRPath = true;
+             newRPath = argv[i];
+         }
++        else if (arg == "--make-rpath-relative") {
++            if (++i == argc) error("missing argument to --make-rpath-relative");
++            makeRPathRelative = true;
++            rootDir = argv[i];
++        }
++        else if (arg == "--no-standard-lib-dirs") {
++            modifyFlags |= MODIFY_FLAG_NO_STD_LIB_DIRS;
++        }
++        else if (arg == "--relative-to-file") {
++            modifyFlags |= MODIFY_FLAG_RELATIVE_TO_FILE;
++        }
+         else if (arg == "--print-rpath") {
+             printRPath = true;
+         }
+-- 
+1.9.1
+
-- 
2.7.4

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Arnout Vandecappelle]

 Hi Wolfgang,

On 30-06-17 10:36, Wolfgang Grandegger wrote:
> The patch allows to use patchelf to sanitize the rpath of the buildroot
> libraries and binaries using the option "--make-rpath-relative <rootdir>".
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

 I still have quite a few remarks, one of which is important: when the same
libfoo.so is present in two directories of the rpath, the second directory
should be removed but it isn't. See below for details.

> ---
>  ...to-make-the-rpath-relative-under-a-specif.patch | 326 +++++++++++++++++++++
>  1 file changed, 326 insertions(+)
>  create mode 100644 package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
> 
> diff --git a/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
> new file mode 100644
> index 0000000..609eacf
> --- /dev/null
> +++ b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
> @@ -0,0 +1,326 @@
> +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
> +From: Wolfgang Grandegger <wg@grandegger.com>
> +Date: Mon, 20 Feb 2017 16:29:24 +0100
> +Subject: [PATCH] Add option to make the rpath relative under a specified root
> + directory
> +
> +Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will
> +modify or delete the RPATHDIRs according the following rules
> +similar to Martin's patches [1] making the Buildroot toolchaing/SDK
> +relocatable.
> +
> +RPATHDIR starts with "$ORIGIN":
> +    The original build-system already took care of setting a relative
> +    RPATH, resolve it and test if it's valid (does exist)
> +
> +RPATHDIR starts with ROOTDIR:
> +    The original build-system added some absolute RPATH (absolute on
> +    the build machine). Test if it's valid (does exist).
> +
> +ROOTDIR/RPATHDIR exists:
> +    The original build-system already took care of setting an absolute
> +    RPATH (absolute in the final rootfs), resolve it and test if it's
> +    valid (does exist).
> +
> +RPATHDIR points somewhere else:
> +    (can be anywhere: build trees, staging tree, host location,
> +    non-existing location, etc.). Just discard such a path.
> +
> +The option "--no-standard-libs" will discard RPATHDIRs ROOTDIR/lib and
> +ROOTDIR/usr/lib. Like "--shrink-rpath", RPATHDIRs are also discarded
> +if the directories do not contain a library referenced by the
> +DT_NEEDED fields.
> +If the option "--relative-to-file" is given, the rpath will start
> +with "$ORIGIN" making it relative to the ELF file, otherwise an
> +absolute path relative to ROOTDIR will be used.
> +
> +[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html
> +
> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> +---
> + src/patchelf.cc | 187 ++++++++++++++++++++++++++++++++++++++++++++++++--------
> + 1 file changed, 161 insertions(+), 26 deletions(-)
> +
> +diff --git a/src/patchelf.cc b/src/patchelf.cc
> +index 55b38e3..60295b3 100644
> +--- a/src/patchelf.cc
> ++++ b/src/patchelf.cc
> +@@ -50,6 +50,9 @@ static int pageSize = PAGESIZE;
> + 
> + typedef std::shared_ptr<std::vector<unsigned char>> FileContents;
> + 
> ++#define MODIFY_FLAG_NO_STD_LIB_DIRS 0x1
> ++#define MODIFY_FLAG_RELATIVE_TO_FILE 0x2
> ++static int modifyFlags;

 I still find it a bad idea to use flags here rather than two separate booleans,
but I guess that's bikeshedding.

> + 
> + #define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym, class Elf_Verneed
> + #define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym, Elf_Verneed
> +@@ -84,6 +87,36 @@ static unsigned int getPageSize()
> +     return pageSize;
> + }
> + 
> ++static bool absolutePathExists(const std::string & path, std::string & canonicalPath)
> ++{
> ++    char *cpath = realpath(path.c_str(), NULL);
> ++    if (cpath) {
> ++        canonicalPath = cpath;
> ++        free(cpath);
> ++        return true;
> ++    } else {
> ++        return false;
> ++    }
> ++}
> ++
> ++static std::string makePathRelative(const std::string & path,
> ++    const std::string & refPath, const std::string & rootDir)
> ++{
> ++    std::string relPath = "$ORIGIN";
> ++
> ++    /* Strip root path first */
> ++    std::string p = path.substr(rootDir.length());
> ++    std::string refP = refPath.substr(rootDir.length());
> ++
> ++    std::size_t pos = refP.find_first_of('/');
> ++    while (pos != std::string::npos) {
> ++        pos =refP.find_first_of('/', pos + 1);
                 ^ missing space

> ++        relPath.append("/..");
> ++    }
> ++    relPath.append(p);

 I probably asked this before already, but why do we need to pass rootDir
instead of just eliminating the common part of path and refPath? Something like:

    while (true) {
        pos = path.find_first_of('/');
        refPos = refPath.find_first_of('/');
        std::string chunk = path.substr(0, pos);
        std::string refChunk = refPath.substr(0, refPos);
        if (chunk != refChunk)
            break;
        path = path.substr(pos);
        refPath = refPath.substr(refPos);
    }
...followed by the loop you already had.

> ++
> ++    return relPath;
> ++}
> + 
> + template<ElfFileParams>
> + class ElfFile
> +@@ -194,9 +227,14 @@ public:
> + 
> +     void setInterpreter(const std::string & newInterpreter);
> + 
> +-    typedef enum { rpPrint, rpShrink, rpSet, rpRemove } RPathOp;
> ++    typedef enum { rpPrint, rpShrink, rpMakeRelative, rpSet, rpRemove} RPathOp;
> + 
> +-    void modifyRPath(RPathOp op, const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath);
> ++    bool libFoundInRPath(const std::string & dirName,
> ++                         const std::vector<std::string> neededLibs);
> ++
> ++    void modifyRPath(RPathOp op,
> ++                     const std::vector<std::string> & allowedRpathPrefixes,
> ++                     std::string rootDir, int flags, std::string newRPath);
> + 
> +     void addNeeded(const std::set<std::string> & libs);
> + 
> +@@ -1108,10 +1146,35 @@ static void concatToRPath(std::string & rpath, const std::string & path)
> +     rpath += path;
> + }
> + 
> ++template<ElfFileParams>
> ++bool ElfFile<ElfFileParamNames>::libFoundInRPath(const std::string & dirName,
> ++    const std::vector<std::string> neededLibs)
> ++{
> ++    std::vector<bool> neededLibFound(neededLibs.size(), false);
> ++
> ++    /* For each library that we haven't found yet, see if it
> ++       exists in this directory. */
> ++    bool libFound = false;
> ++    for (unsigned int j = 0; j < neededLibs.size(); ++j)
> ++        if (!neededLibFound[j]) {
> ++            std::string libName = dirName + "/" + neededLibs[j];
> ++            try {
> ++                if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
> ++                    neededLibFound[j] = true;
> ++                    libFound = true;
> ++                } else
> ++                    debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
> ++            } catch (SysError & e) {
> ++                if (e.errNo != ENOENT) throw;
> ++            }
> ++        }
> ++    return libFound;
> ++}
> + 
> + template<ElfFileParams>
> + void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
> +-    const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath)
> ++    const std::vector<std::string> & allowedRpathPrefixes,
> ++    std::string rootDir, int flags, std::string newRPath)

 Since the modifyFlags are anyway global, and since this function is already
using the other global parameter forceRPath, I don't think it makes a lot of
sense to pass flags as an argument. Also, for consistency, it would be better to
still call it modifyFlags IMO.

> + {
> +     Elf_Shdr & shdrDynamic = findSection(".dynamic");
> + 
> +@@ -1162,11 +1225,14 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
> +         return;
> +     }
> + 
> ++    if (op == rpMakeRelative && !rpath) {
> ++        debug("no RPATH to make relative\n");
> ++        return;
> ++    }
> + 
> +     /* For each directory in the RPATH, check if it contains any
> +        needed library. */
> +     if (op == rpShrink) {
> +-        std::vector<bool> neededLibFound(neededLibs.size(), false);

 Darn, I hadn't noticed this before: I think this breaks rpShrink...

 When you have an rpath /xxx:/yyy, and there is a needed library libfoo.so that
exists in both /xxx and /yyy, then rpShrink should still remove /yyy. Indeed,
the one in /xxx gets precedence, and /yyy isn't needed for anything else.

 However, with your refactoring, the neededLibfound array is reset for every
iteration of the rpath loop. So when evaluating /yyy, neededLibFound will again
be false, and /yyy will be kept.

 Actually, come to think of it, the same problem will occur for the
rpMakeRelative, no?

> + 
> +         newRPath = "";
> + 
> +@@ -1186,30 +1252,81 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
> +                 continue;
> +             }
> + 
> +-            /* For each library that we haven't found yet, see if it
> +-               exists in this directory. */
> +-            bool libFound = false;
> +-            for (unsigned int j = 0; j < neededLibs.size(); ++j)
> +-                if (!neededLibFound[j]) {
> +-                    std::string libName = dirName + "/" + neededLibs[j];
> +-                    try {
> +-                        if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
> +-                            neededLibFound[j] = true;
> +-                            libFound = true;
> +-                        } else
> +-                            debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
> +-                    } catch (SysError & e) {
> +-                        if (e.errNo != ENOENT) throw;
> +-                    }
> +-                }
> +-
> +-            if (!libFound)
> ++            if (!libFoundInRPath(dirName, neededLibs))
> +                 debug("removing directory '%s' from RPATH\n", dirName.c_str());

 For consistency, we should probably honour --no-standard-lib-dirs here as well.

> +             else
> +                 concatToRPath(newRPath, dirName);
> +         }
> +     }
> + 
> ++    /* Make the the RPATH relative to the specified path */
> ++    if (op == rpMakeRelative) {
> ++        std::string fileDir = fileName.substr(0, fileName.find_last_of("/"));
> ++        newRPath = "";
> ++
> ++        for (auto & dirName : splitColonDelimitedString(rpath)) {
> ++            std::string canonicalPath;
> ++
> ++            /* Figure out if we should keep or discard the path. There are several
> ++               cases to be handled:
> ++               "dirName" starts with "$ORIGIN":
> ++                   The original build-system already took care of setting a relative
> ++                   RPATH. Resolve it and test if it's valid (does exist).
> ++               "dirName" start with "rootDir":
> ++                   The original build-system added some absolute RPATH (absolute on
> ++                   the build machine). Test if it's valid (does exist).
> ++               "rootDir"/"dirName" exists:
> ++                    The original build-system already took care of setting an absolute
> ++                    RPATH (absolute in the final rootfs). Resolve it and test if it's
> ++                    valid (does exist).
> ++               "dirName" points somewhere else:
> ++                    (can be anywhere: build trees, staging tree, host location,
> ++                    non-existing location, etc.). Just discard such a path. */
> ++            if (!dirName.compare(0, 7, "$ORIGIN")) {
> ++                std::string path = fileDir + dirName.substr(7);
> ++                if (!absolutePathExists(path, canonicalPath)) {
> ++                    debug("removing directory '%s' from RPATH because '%s' doesn't exist\n",
> ++			  dirName.c_str(), path.c_str());
> ++                    continue;
> ++                }
> ++            } else if (!dirName.compare(0, rootDir.length(), rootDir)) {
> ++                if (!absolutePathExists(dirName, canonicalPath)) {
> ++                    debug("removing directory '%s' from RPATH because it doesn't exist\n", dirName.c_str());
> ++                    continue;
> ++                }
> ++            } else {
> ++                std::string path = rootDir + dirName;
> ++                if (!absolutePathExists(path, canonicalPath)) {
> ++                    debug("removing directory '%s' from RPATH because it's not in rootdir\n",
> ++			  dirName.c_str());
> ++                    continue;
> ++                }
> ++            }
> ++
> ++            if (flags & MODIFY_FLAG_NO_STD_LIB_DIRS) {
> ++                if (!canonicalPath.compare(rootDir + "/lib") ||
> ++                    !canonicalPath.compare(rootDir + "/usr/lib")) {
> ++                    debug("removing directory '%s' from RPATH because it's a standard library directory\n",
> ++                         dirName.c_str());
> ++                    continue;
> ++                }
> ++            }
> ++
> ++            if (!libFoundInRPath(canonicalPath, neededLibs)) {
> ++                debug("removing directory '%s' from RPATH because it does not contain needed libs\n",
> ++		      dirName.c_str());
> ++                continue;
> ++            }
> ++
> ++            /* Finally make "canonicalPath" relative to "filedir" in "rootDir" */
> ++	    if (flags & MODIFY_FLAG_RELATIVE_TO_FILE)

 Indentation is wrong here (and on the following lines): line starts with tab
instead of spaces.

 Regards,
 Arnout


> ++		concatToRPath(newRPath, makePathRelative(canonicalPath, fileDir, rootDir));
> ++	    else
> ++		concatToRPath(newRPath, canonicalPath.substr(rootDir.length()));
> ++	    debug("keeping relative path of %s\n", canonicalPath.c_str());
> ++	}
> ++    }
> ++
> +     if (op == rpRemove) {
> +         if (!rpath) {
> +             debug("no RPATH to delete\n");
> +@@ -1538,7 +1655,9 @@ static std::vector<std::string> allowedRpathPrefixes;
> + static bool removeRPath = false;
> + static bool setRPath = false;
> + static bool printRPath = false;
> ++static bool makeRPathRelative = false;
> + static std::string newRPath;
> ++static std::string rootDir;
> + static std::set<std::string> neededLibsToRemove;
> + static std::map<std::string, std::string> neededLibsToReplace;
> + static std::set<std::string> neededLibsToAdd;
> +@@ -1561,14 +1680,16 @@ static void patchElf2(ElfFile && elfFile)
> +         elfFile.setInterpreter(newInterpreter);
> + 
> +     if (printRPath)
> +-        elfFile.modifyRPath(elfFile.rpPrint, {}, "");
> ++        elfFile.modifyRPath(elfFile.rpPrint, {}, {}, modifyFlags, "");
> + 
> +     if (shrinkRPath)
> +-        elfFile.modifyRPath(elfFile.rpShrink, allowedRpathPrefixes, "");
> ++        elfFile.modifyRPath(elfFile.rpShrink, allowedRpathPrefixes, "", modifyFlags, "");
> +     else if (removeRPath)
> +-        elfFile.modifyRPath(elfFile.rpRemove, {}, "");
> ++        elfFile.modifyRPath(elfFile.rpRemove, {}, "", modifyFlags, "");
> +     else if (setRPath)
> +-        elfFile.modifyRPath(elfFile.rpSet, {}, newRPath);
> ++        elfFile.modifyRPath(elfFile.rpSet, {}, "", modifyFlags, newRPath);
> ++    else if (makeRPathRelative)
> ++        elfFile.modifyRPath(elfFile.rpMakeRelative, {}, rootDir, modifyFlags, "");
> + 
> +     if (printNeeded) elfFile.printNeededLibs();
> + 
> +@@ -1614,6 +1735,9 @@ void showHelp(const std::string & progName)
> +   [--remove-rpath]\n\
> +   [--shrink-rpath]\n\
> +   [--allowed-rpath-prefixes PREFIXES]\t\tWith '--shrink-rpath', reject rpath entries not starting with the allowed prefix\n\
> ++  [--make-rpath-relative ROOTDIR]\n\
> ++  [--no-standard-lib-dirs]\n\
> ++  [--relative-to-file]\n\
> +   [--print-rpath]\n\
> +   [--force-rpath]\n\
> +   [--add-needed LIBRARY]\n\
> +@@ -1674,6 +1798,17 @@ int mainWrapped(int argc, char * * argv)
> +             setRPath = true;
> +             newRPath = argv[i];
> +         }
> ++        else if (arg == "--make-rpath-relative") {
> ++            if (++i == argc) error("missing argument to --make-rpath-relative");
> ++            makeRPathRelative = true;
> ++            rootDir = argv[i];
> ++        }
> ++        else if (arg == "--no-standard-lib-dirs") {
> ++            modifyFlags |= MODIFY_FLAG_NO_STD_LIB_DIRS;
> ++        }
> ++        else if (arg == "--relative-to-file") {
> ++            modifyFlags |= MODIFY_FLAG_RELATIVE_TO_FILE;
> ++        }
> +         else if (arg == "--print-rpath") {
> +             printRPath = true;
> +         }
> +-- 
> +1.9.1
> +
> 

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

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Wolfgang Grandegger]

Am 01.07.2017 um 00:13 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> 
>   I still have quite a few remarks, one of which is important: when the same
> libfoo.so is present in two directories of the rpath, the second directory
> should be removed but it isn't. See below for details.

Not sure if I understand you correctly. The rpath is for example 
"/usr/lib/dir1:/usr/lib/dir2" and libfoo.so is in both directories!?
>> ---
>>   ...to-make-the-rpath-relative-under-a-specif.patch | 326 +++++++++++++++++++++
>>   1 file changed, 326 insertions(+)
>>   create mode 100644 package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
>>
>> diff --git a/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
>> new file mode 100644
>> index 0000000..609eacf
>> --- /dev/null
>> +++ b/package/patchelf/0001-Add-option-to-make-the-rpath-relative-under-a-specif.patch
>> @@ -0,0 +1,326 @@
>> +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
>> +From: Wolfgang Grandegger <wg@grandegger.com>
>> +Date: Mon, 20 Feb 2017 16:29:24 +0100
>> +Subject: [PATCH] Add option to make the rpath relative under a specified root
>> + directory
>> +
>> +Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will
>> +modify or delete the RPATHDIRs according the following rules
>> +similar to Martin's patches [1] making the Buildroot toolchaing/SDK
>> +relocatable.
>> +
>> +RPATHDIR starts with "$ORIGIN":
>> +    The original build-system already took care of setting a relative
>> +    RPATH, resolve it and test if it's valid (does exist)
>> +
>> +RPATHDIR starts with ROOTDIR:
>> +    The original build-system added some absolute RPATH (absolute on
>> +    the build machine). Test if it's valid (does exist).
>> +
>> +ROOTDIR/RPATHDIR exists:
>> +    The original build-system already took care of setting an absolute
>> +    RPATH (absolute in the final rootfs), resolve it and test if it's
>> +    valid (does exist).
>> +
>> +RPATHDIR points somewhere else:
>> +    (can be anywhere: build trees, staging tree, host location,
>> +    non-existing location, etc.). Just discard such a path.
>> +
>> +The option "--no-standard-libs" will discard RPATHDIRs ROOTDIR/lib and
>> +ROOTDIR/usr/lib. Like "--shrink-rpath", RPATHDIRs are also discarded
>> +if the directories do not contain a library referenced by the
>> +DT_NEEDED fields.
>> +If the option "--relative-to-file" is given, the rpath will start
>> +with "$ORIGIN" making it relative to the ELF file, otherwise an
>> +absolute path relative to ROOTDIR will be used.
>> +
>> +[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html
>> +
>> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> +---
>> + src/patchelf.cc | 187 ++++++++++++++++++++++++++++++++++++++++++++++++--------
>> + 1 file changed, 161 insertions(+), 26 deletions(-)
>> +
>> +diff --git a/src/patchelf.cc b/src/patchelf.cc
>> +index 55b38e3..60295b3 100644
>> +--- a/src/patchelf.cc
>> ++++ b/src/patchelf.cc
>> +@@ -50,6 +50,9 @@ static int pageSize = PAGESIZE;
>> +
>> + typedef std::shared_ptr<std::vector<unsigned char>> FileContents;
>> +
>> ++#define MODIFY_FLAG_NO_STD_LIB_DIRS 0x1
>> ++#define MODIFY_FLAG_RELATIVE_TO_FILE 0x2
>> ++static int modifyFlags;
> 
>   I still find it a bad idea to use flags here rather than two separate booleans,
> but I guess that's bikeshedding.

Well, coming from the C world, I love the bit-wise flags. Could switch 
to more C++ like flags, no problem.

> 
>> +
>> + #define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym, class Elf_Verneed
>> + #define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym, Elf_Verneed
>> +@@ -84,6 +87,36 @@ static unsigned int getPageSize()
>> +     return pageSize;
>> + }
>> +
>> ++static bool absolutePathExists(const std::string & path, std::string & canonicalPath)
>> ++{
>> ++    char *cpath = realpath(path.c_str(), NULL);
>> ++    if (cpath) {
>> ++        canonicalPath = cpath;
>> ++        free(cpath);
>> ++        return true;
>> ++    } else {
>> ++        return false;
>> ++    }
>> ++}
>> ++
>> ++static std::string makePathRelative(const std::string & path,
>> ++    const std::string & refPath, const std::string & rootDir)
>> ++{
>> ++    std::string relPath = "$ORIGIN";
>> ++
>> ++    /* Strip root path first */
>> ++    std::string p = path.substr(rootDir.length());
>> ++    std::string refP = refPath.substr(rootDir.length());
>> ++
>> ++    std::size_t pos = refP.find_first_of('/');
>> ++    while (pos != std::string::npos) {
>> ++        pos =refP.find_first_of('/', pos + 1);
>                   ^ missing space
> 
>> ++        relPath.append("/..");
>> ++    }
>> ++    relPath.append(p);
> 
>   I probably asked this before already, but why do we need to pass rootDir
> instead of just eliminating the common part of path and refPath? Something like:
> 
>      while (true) {
>          pos = path.find_first_of('/');
>          refPos = refPath.find_first_of('/');
>          std::string chunk = path.substr(0, pos);
>          std::string refChunk = refPath.substr(0, refPos);
>          if (chunk != refChunk)
>              break;
>          path = path.substr(pos);
>          refPath = refPath.substr(refPos);
>      }
> ...followed by the loop you already had.

Could be done. In that case the path ($ORIGIN) is not relative to 
rootdir. But that's not required. The path must just be within rootdir.

> 
>> ++
>> ++    return relPath;
>> ++}
>> +
>> + template<ElfFileParams>
>> + class ElfFile
>> +@@ -194,9 +227,14 @@ public:
>> +
>> +     void setInterpreter(const std::string & newInterpreter);
>> +
>> +-    typedef enum { rpPrint, rpShrink, rpSet, rpRemove } RPathOp;
>> ++    typedef enum { rpPrint, rpShrink, rpMakeRelative, rpSet, rpRemove} RPathOp;
>> +
>> +-    void modifyRPath(RPathOp op, const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath);
>> ++    bool libFoundInRPath(const std::string & dirName,
>> ++                         const std::vector<std::string> neededLibs);
>> ++
>> ++    void modifyRPath(RPathOp op,
>> ++                     const std::vector<std::string> & allowedRpathPrefixes,
>> ++                     std::string rootDir, int flags, std::string newRPath);
>> +
>> +     void addNeeded(const std::set<std::string> & libs);
>> +
>> +@@ -1108,10 +1146,35 @@ static void concatToRPath(std::string & rpath, const std::string & path)
>> +     rpath += path;
>> + }
>> +
>> ++template<ElfFileParams>
>> ++bool ElfFile<ElfFileParamNames>::libFoundInRPath(const std::string & dirName,
>> ++    const std::vector<std::string> neededLibs)
>> ++{
>> ++    std::vector<bool> neededLibFound(neededLibs.size(), false);
>> ++
>> ++    /* For each library that we haven't found yet, see if it
>> ++       exists in this directory. */
>> ++    bool libFound = false;
>> ++    for (unsigned int j = 0; j < neededLibs.size(); ++j)
>> ++        if (!neededLibFound[j]) {
>> ++            std::string libName = dirName + "/" + neededLibs[j];
>> ++            try {
>> ++                if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
>> ++                    neededLibFound[j] = true;
>> ++                    libFound = true;
>> ++                } else
>> ++                    debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
>> ++            } catch (SysError & e) {
>> ++                if (e.errNo != ENOENT) throw;
>> ++            }
>> ++        }
>> ++    return libFound;
>> ++}
>> +
>> + template<ElfFileParams>
>> + void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
>> +-    const std::vector<std::string> & allowedRpathPrefixes, std::string newRPath)
>> ++    const std::vector<std::string> & allowedRpathPrefixes,
>> ++    std::string rootDir, int flags, std::string newRPath)
> 
>   Since the modifyFlags are anyway global, and since this function is already
> using the other global parameter forceRPath, I don't think it makes a lot of
> sense to pass flags as an argument. Also, for consistency, it would be better to
> still call it modifyFlags IMO.

I see, will change that.

>> + {
>> +     Elf_Shdr & shdrDynamic = findSection(".dynamic");
>> +
>> +@@ -1162,11 +1225,14 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
>> +         return;
>> +     }
>> +
>> ++    if (op == rpMakeRelative && !rpath) {
>> ++        debug("no RPATH to make relative\n");
>> ++        return;
>> ++    }
>> +
>> +     /* For each directory in the RPATH, check if it contains any
>> +        needed library. */
>> +     if (op == rpShrink) {
>> +-        std::vector<bool> neededLibFound(neededLibs.size(), false);
> 
>   Darn, I hadn't noticed this before: I think this breaks rpShrink...

Hm, I thought I didn't change rpShrink. I just use a common function to 
avoid duplication code, but ...

>   When you have an rpath /xxx:/yyy, and there is a needed library libfoo.so that
> exists in both /xxx and /yyy, then rpShrink should still remove /yyy. Indeed,
> the one in /xxx gets precedence, and /yyy isn't needed for anything else.
> 
>   However, with your refactoring, the neededLibfound array is reset for every
> iteration of the rpath loop. So when evaluating /yyy, neededLibFound will again
> be false, and /yyy will be kept.
> 
>   Actually, come to think of it, the same problem will occur for the
> rpMakeRelative, no?

... you a right. Good catch! I should pass "neededLibFound" as argument.

> 
>> +
>> +         newRPath = "";
>> +
>> +@@ -1186,30 +1252,81 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
>> +                 continue;
>> +             }
>> +
>> +-            /* For each library that we haven't found yet, see if it
>> +-               exists in this directory. */
>> +-            bool libFound = false;
>> +-            for (unsigned int j = 0; j < neededLibs.size(); ++j)
>> +-                if (!neededLibFound[j]) {
>> +-                    std::string libName = dirName + "/" + neededLibs[j];
>> +-                    try {
>> +-                        if (getElfType(readFile(libName, sizeof(Elf32_Ehdr))).machine == rdi(hdr->e_machine)) {
>> +-                            neededLibFound[j] = true;
>> +-                            libFound = true;
>> +-                        } else
>> +-                            debug("ignoring library '%s' because its machine type differs\n", libName.c_str());
>> +-                    } catch (SysError & e) {
>> +-                        if (e.errNo != ENOENT) throw;
>> +-                    }
>> +-                }
>> +-
>> +-            if (!libFound)
>> ++            if (!libFoundInRPath(dirName, neededLibs))
>> +                 debug("removing directory '%s' from RPATH\n", dirName.c_str());
> 
>   For consistency, we should probably honour --no-standard-lib-dirs here as well.

OK.

>> +             else
>> +                 concatToRPath(newRPath, dirName);
>> +         }
>> +     }
>> +
>> ++    /* Make the the RPATH relative to the specified path */
>> ++    if (op == rpMakeRelative) {
>> ++        std::string fileDir = fileName.substr(0, fileName.find_last_of("/"));
>> ++        newRPath = "";
>> ++
>> ++        for (auto & dirName : splitColonDelimitedString(rpath)) {
>> ++            std::string canonicalPath;
>> ++
>> ++            /* Figure out if we should keep or discard the path. There are several
>> ++               cases to be handled:
>> ++               "dirName" starts with "$ORIGIN":
>> ++                   The original build-system already took care of setting a relative
>> ++                   RPATH. Resolve it and test if it's valid (does exist).
>> ++               "dirName" start with "rootDir":
>> ++                   The original build-system added some absolute RPATH (absolute on
>> ++                   the build machine). Test if it's valid (does exist).
>> ++               "rootDir"/"dirName" exists:
>> ++                    The original build-system already took care of setting an absolute
>> ++                    RPATH (absolute in the final rootfs). Resolve it and test if it's
>> ++                    valid (does exist).
>> ++               "dirName" points somewhere else:
>> ++                    (can be anywhere: build trees, staging tree, host location,
>> ++                    non-existing location, etc.). Just discard such a path. */
>> ++            if (!dirName.compare(0, 7, "$ORIGIN")) {
>> ++                std::string path = fileDir + dirName.substr(7);
>> ++                if (!absolutePathExists(path, canonicalPath)) {
>> ++                    debug("removing directory '%s' from RPATH because '%s' doesn't exist\n",
>> ++			  dirName.c_str(), path.c_str());
>> ++                    continue;
>> ++                }
>> ++            } else if (!dirName.compare(0, rootDir.length(), rootDir)) {
>> ++                if (!absolutePathExists(dirName, canonicalPath)) {
>> ++                    debug("removing directory '%s' from RPATH because it doesn't exist\n", dirName.c_str());
>> ++                    continue;
>> ++                }
>> ++            } else {
>> ++                std::string path = rootDir + dirName;
>> ++                if (!absolutePathExists(path, canonicalPath)) {
>> ++                    debug("removing directory '%s' from RPATH because it's not in rootdir\n",
>> ++			  dirName.c_str());
>> ++                    continue;
>> ++                }
>> ++            }
>> ++
>> ++            if (flags & MODIFY_FLAG_NO_STD_LIB_DIRS) {
>> ++                if (!canonicalPath.compare(rootDir + "/lib") ||
>> ++                    !canonicalPath.compare(rootDir + "/usr/lib")) {
>> ++                    debug("removing directory '%s' from RPATH because it's a standard library directory\n",
>> ++                         dirName.c_str());
>> ++                    continue;
>> ++                }
>> ++            }
>> ++
>> ++            if (!libFoundInRPath(canonicalPath, neededLibs)) {
>> ++                debug("removing directory '%s' from RPATH because it does not contain needed libs\n",
>> ++		      dirName.c_str());
>> ++                continue;
>> ++            }
>> ++
>> ++            /* Finally make "canonicalPath" relative to "filedir" in "rootDir" */
>> ++	    if (flags & MODIFY_FLAG_RELATIVE_TO_FILE)
> 
>   Indentation is wrong here (and on the following lines): line starts with tab
> instead of spaces.

OK.

Wolfgang.

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Arnout Vandecappelle]

On 01-07-17 22:30, Wolfgang Grandegger wrote:
> 
> 
> Am 01.07.2017 um 00:13 schrieb Arnout Vandecappelle:
>>   Hi Wolfgang,
>>
>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>
>>   I still have quite a few remarks, one of which is important: when the same
>> libfoo.so is present in two directories of the rpath, the second directory
>> should be removed but it isn't. See below for details.
> 
> Not sure if I understand you correctly. The rpath is for example
> "/usr/lib/dir1:/usr/lib/dir2" and libfoo.so is in both directories!?

 Yes. If that happens then dir2 should be removed from rpath (unless of course
there is still another needed lib in there).

 I admit it's unlikely that such a situation will arise in Buildroot, but let's
do the right thing :-)

[snip]
>>> ++static std::string makePathRelative(const std::string & path,
>>> ++    const std::string & refPath, const std::string & rootDir)
>>> ++{
>>> ++    std::string relPath = "$ORIGIN";
>>> ++
>>> ++    /* Strip root path first */
>>> ++    std::string p = path.substr(rootDir.length());
>>> ++    std::string refP = refPath.substr(rootDir.length());
>>> ++
>>> ++    std::size_t pos = refP.find_first_of('/');
>>> ++    while (pos != std::string::npos) {
>>> ++        pos =refP.find_first_of('/', pos + 1);
>>                   ^ missing space
>>
>>> ++        relPath.append("/..");
>>> ++    }
>>> ++    relPath.append(p);
>>
>>   I probably asked this before already, but why do we need to pass rootDir
>> instead of just eliminating the common part of path and refPath? Something like:
>>
>>      while (true) {
>>          pos = path.find_first_of('/');
>>          refPos = refPath.find_first_of('/');
>>          std::string chunk = path.substr(0, pos);
>>          std::string refChunk = refPath.substr(0, refPos);
>>          if (chunk != refChunk)
>>              break;
>>          path = path.substr(pos);
>>          refPath = refPath.substr(refPos);
>>      }
>> ...followed by the loop you already had.
> 
> Could be done. In that case the path ($ORIGIN) is not relative to rootdir. But
> that's not required. The path must just be within rootdir.

 Exactly.

 Well, actually, for HOST_DIR, it doesn't need to be in rootdir. For example,
it's possible that some host tool optionally links to libusb but our
host-package doesn't depend on libusb and doesn't pass --without-libusb. If you
have libusb installed on your build system in a non-standard path, it will be
discovered through pkg-config and the package will link with it, but then the
rpath is needed to make sure the executable still runs.

 So in fact, for host tools, we shouldn't eliminate RPATHs just because they are
outside of rootDir.

 Regards,
 Arnout

[snip]

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

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Wolfgang Grandegger]

Hello,

browsing your comments...

Am 01.07.2017 um 00:13 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
> On 30-06-17 10:36, Wolfgang Grandegger wrote:
[... snip ...]
>> +-            if (!libFound)
>> ++            if (!libFoundInRPath(dirName, neededLibs))
>> +                 debug("removing directory '%s' from RPATH\n", dirName.c_str());
> 
>   For consistency, we should probably honour --no-standard-lib-dirs here as well.
> 

For "--shrink-rpath" it's difficult to decide what is a non standard 
library directory. A path ending with "/lib" or "/usr/lib"?

Wolfgang.

[Buildroot] [RFC PATCH v5 02/11] package/patchelf: add patch for rpath sanitation under a root directory

[Samuel Martin]

Wolfgang,

On Mon, Jul 3, 2017 at 9:38 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Hello,
>
> browsing your comments...
>
> Am 01.07.2017 um 00:13 schrieb Arnout Vandecappelle:
>>
>>   Hi Wolfgang,
>>
>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>
> [... snip ...]
>>>
>>> +-            if (!libFound)
>>> ++            if (!libFoundInRPath(dirName, neededLibs))
>>> +                 debug("removing directory '%s' from RPATH\n",
>>> dirName.c_str());
>>
>>
>>   For consistency, we should probably honour --no-standard-lib-dirs here
>> as well.
>>
>
> For "--shrink-rpath" it's difficult to decide what is a non standard library
> directory. A path ending with "/lib" or "/usr/lib"?

Yes, exactly.
By standard paths, we mean location browsed by default by the linux
loader (see [1])

[1] http://www.manpages.info/linux/ld.so.8.html

Regards,

-- 
Samuel

[Buildroot] [RFC PATCH v5 03/11] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

From: Samuel Martin <s.martin49@gmail.com>

The script "fix-rpath" can scan a list of files provided via
command line argument, identify ELF files, check their RPATH and
fix it in a proper way. The RPATH fixup is done by the patchelf
utility using the option "--make-rpath-relative <root-directory>".

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 support/scripts/fix-rpath | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100755 support/scripts/fix-rpath

diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
new file mode 100755
index 0000000..405108d
--- /dev/null
+++ b/support/scripts/fix-rpath
@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+usage() {
+  cat <<EOF >&2
+Usage:	${0} TREE_KIND FILES_LIST_BEFORE FILELIST_AFTER_AFTER
+
+Description:
+
+    This script scans a tree and sanitize ELF files' RPATH found in there.
+
+    Sanitization behaves the same whatever the kind of the processed tree,
+    but the resulting RPATH differs. The rpath sanitization is done using
+    "patchelf --make-rpath-relazive".
+
+Arguments:
+
+    TREE_KIND	Kind of tree to be processed.
+		Allowed values: host, target, staging
+
+    FILELIST    File with the list of files installed by the package
+
+
+Environment:
+
+    PATCHELF	patchelf program to use
+		(default: HOST_DIR/usr/bin/patchelf)
+EOF
+}
+
+: ${PATCHELF:=${HOST_DIR}/usr/bin/patchelf}
+
+main() {
+    local rootdir
+    local tree="${1}"
+    local filelist="${2}"
+    local sanitize_extra_args=( )
+
+    case "${tree}" in
+	host)
+	    rootdir="${HOST_DIR}"
+	    sanitize_extra_args+=( "--relative-to-file" )
+	    ;;
+
+	staging)
+	    rootdir="${STAGING_DIR}"
+	    sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )
+	    ;;
+
+	target)
+	    rootdir="${TARGET_DIR}"
+	    sanitize_extra_args+=( "--no-standard-lib-dirs" )
+	    ;;
+
+	*)
+	    usage
+	    exit 1
+	    ;;
+    esac
+
+    while read -r file ; do
+	# check if it's an ELF file
+	path=${rootdir}/${file}
+	if ${PATCHELF} --print-rpath "${path}" > /dev/null 2>&1; then
+	    # Work around file busy issue with patchelf binary
+	    if [ "${PATCHELF}" == "${path}" ]; then
+		cp "${PATCHELF}" "${PATCHELF}.__patched__"
+		${PATCHELF} --debug --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${PATCHELF}.__patched__"
+		mv "${PATCHELF}.__patched__" "${PATCHELF}"
+	    else
+		# make files writable if necessary
+		changed=$(chmod -c u+w "${path}")
+		# call patchelf to sanitize the rpath
+		${PATCHELF} --debug --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${path}"
+		# restore the original permission
+		test "${changed}" != "" && chmod u-w "${path}"
+	    fi
+	fi
+    done < ${filelist}
+
+    # ignore errors
+    return 0
+}
+
+main ${@}
-- 
2.7.4

[Buildroot] [RFC PATCH v5 03/11] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

 Hi Wolfgang,

 First of all, let me commend you on the large number of comments and the
general clarity of this script.

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> From: Samuel Martin <s.martin49@gmail.com>
> 
> The script "fix-rpath" can scan a list of files provided via
> command line argument, identify ELF files, check their RPATH and
> fix it in a proper way. The RPATH fixup is done by the patchelf
> utility using the option "--make-rpath-relative <root-directory>".
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  support/scripts/fix-rpath | 100 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100755 support/scripts/fix-rpath
> 
> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
> new file mode 100755
> index 0000000..405108d
> --- /dev/null
> +++ b/support/scripts/fix-rpath
> @@ -0,0 +1,100 @@
> +#!/usr/bin/env bash
> +
> +# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>

 Almost nothing is left of the original version by Samuel, so I think you can
safely add your own name to the Copyright statement :-)

> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +usage() {
> +  cat <<EOF >&2
> +Usage:	${0} TREE_KIND FILES_LIST_BEFORE FILELIST_AFTER_AFTER

 There is only one file list now.

> +
> +Description:
> +
> +    This script scans a tree and sanitize ELF files' RPATH found in there.

 It doesn't scan the tree anymore.

> +
> +    Sanitization behaves the same whatever the kind of the processed tree,
> +    but the resulting RPATH differs. The rpath sanitization is done using
> +    "patchelf --make-rpath-relazive".

 relazive? :-)

> +
> +Arguments:
> +
> +    TREE_KIND	Kind of tree to be processed.
> +		Allowed values: host, target, staging
> +
> +    FILELIST    File with the list of files installed by the package
> +
> +
> +Environment:
> +
> +    PATCHELF	patchelf program to use
> +		(default: HOST_DIR/usr/bin/patchelf)
> +EOF
> +}
> +
> +: ${PATCHELF:=${HOST_DIR}/usr/bin/patchelf}
> +
> +main() {
> +    local rootdir
> +    local tree="${1}"
> +    local filelist="${2}"
> +    local sanitize_extra_args=( )
> +
> +    case "${tree}" in
> +	host)
> +	    rootdir="${HOST_DIR}"
> +	    sanitize_extra_args+=( "--relative-to-file" )
> +	    ;;
> +
> +	staging)
> +	    rootdir="${STAGING_DIR}"
> +	    sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )

 I don't think this is the right thing to do. As I wrote before, the files in
staging_dir should never ever be executed, and they are not copied to the
target. So instead, we should just --remove-rpath.

> +	    ;;
> +
> +	target)
> +	    rootdir="${TARGET_DIR}"
> +	    sanitize_extra_args+=( "--no-standard-lib-dirs" )

 Since this is the only variation between the three, I think it would be easier
to handle that from the caller. So

${0} ROOT_DIR FILES_LIST PATCHELF_ARGS...

> +	    ;;
> +
> +	*)
> +	    usage
> +	    exit 1
> +	    ;;
> +    esac
> +
> +    while read -r file ; do
> +	# check if it's an ELF file
> +	path=${rootdir}/${file}
> +	if ${PATCHELF} --print-rpath "${path}" > /dev/null 2>&1; then
> +	    # Work around file busy issue with patchelf binary
> +	    if [ "${PATCHELF}" == "${path}" ]; then
> +		cp "${PATCHELF}" "${PATCHELF}.__patched__"
> +		${PATCHELF} --debug --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${PATCHELF}.__patched__"

 Do we want to keep the --debug?

> +		mv "${PATCHELF}.__patched__" "${PATCHELF}"
> +	    else
> +		# make files writable if necessary
> +		changed=$(chmod -c u+w "${path}")

 Good call on that one!

 Regards,
 Arnout

> +		# call patchelf to sanitize the rpath
> +		${PATCHELF} --debug --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${path}"
> +		# restore the original permission
> +		test "${changed}" != "" && chmod u-w "${path}"
> +	    fi
> +	fi
> +    done < ${filelist}
> +
> +    # ignore errors
> +    return 0
> +}
> +
> +main ${@}
> 

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

[Buildroot] [RFC PATCH v5 03/11] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Hello Arnout,

the latest patch was a rather quick hack to demonstrate the per package 
and installation step approach...

Am 01.07.2017 um 12:36 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
>   First of all, let me commend you on the large number of comments and the
> general clarity of this script.
> 
> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>> From: Samuel Martin <s.martin49@gmail.com>
>>
>> The script "fix-rpath" can scan a list of files provided via
>> command line argument, identify ELF files, check their RPATH and
>> fix it in a proper way. The RPATH fixup is done by the patchelf
>> utility using the option "--make-rpath-relative <root-directory>".
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   support/scripts/fix-rpath | 100 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 100 insertions(+)
>>   create mode 100755 support/scripts/fix-rpath
>>
>> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
>> new file mode 100755
>> index 0000000..405108d
>> --- /dev/null
>> +++ b/support/scripts/fix-rpath
>> @@ -0,0 +1,100 @@
>> +#!/usr/bin/env bash
>> +
>> +# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
> 
>   Almost nothing is left of the original version by Samuel, so I think you can
> safely add your own name to the Copyright statement :-)

Well, fine for me and nobody else complains.

>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +# General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> +
>> +usage() {
>> +  cat <<EOF >&2
>> +Usage:	${0} TREE_KIND FILES_LIST_BEFORE FILELIST_AFTER_AFTER
> 
>   There is only one file list now.

OK. Leftover from my first try.

>> +
>> +Description:
>> +
>> +    This script scans a tree and sanitize ELF files' RPATH found in there.
> 
>   It doesn't scan the tree anymore.
> 
>> +
>> +    Sanitization behaves the same whatever the kind of the processed tree,
>> +    but the resulting RPATH differs. The rpath sanitization is done using
>> +    "patchelf --make-rpath-relazive".
> 
>   relazive? :-)

Will fix the two issues above.

> 
>> +
>> +Arguments:
>> +
>> +    TREE_KIND	Kind of tree to be processed.
>> +		Allowed values: host, target, staging
>> +
>> +    FILELIST    File with the list of files installed by the package
>> +
>> +
>> +Environment:
>> +
>> +    PATCHELF	patchelf program to use
>> +		(default: HOST_DIR/usr/bin/patchelf)
>> +EOF
>> +}
>> +
>> +: ${PATCHELF:=${HOST_DIR}/usr/bin/patchelf}
>> +
>> +main() {
>> +    local rootdir
>> +    local tree="${1}"
>> +    local filelist="${2}"
>> +    local sanitize_extra_args=( )
>> +
>> +    case "${tree}" in
>> +	host)
>> +	    rootdir="${HOST_DIR}"
>> +	    sanitize_extra_args+=( "--relative-to-file" )
>> +	    ;;
>> +
>> +	staging)
>> +	    rootdir="${STAGING_DIR}"
>> +	    sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )
> 
>   I don't think this is the right thing to do. As I wrote before, the files in
> staging_dir should never ever be executed, and they are not copied to the
> target. So instead, we should just --remove-rpath.

OK, in the end we could do for this tree and others what is really 
needed. I already tried with deleting the rpath and it did not trigger 
any build issues (with my single usecase).

> 
>> +	    ;;
>> +
>> +	target)
>> +	    rootdir="${TARGET_DIR}"
>> +	    sanitize_extra_args+=( "--no-standard-lib-dirs" )
> 
>   Since this is the only variation between the three, I think it would be easier
> to handle that from the caller. So
> 
> ${0} ROOT_DIR FILES_LIST PATCHELF_ARGS...

Could be done but we need to pass then almost all args, including 
"--remove-rpath", "--make-rpath-rleative"... Maybe that's not that nice 
in "pkg-generic.mk". But we could drop $tree and  use the $ROOT_DIR instead.

> 
>> +	    ;;
>> +
>> +	*)
>> +	    usage
>> +	    exit 1
>> +	    ;;
>> +    esac
>> +
>> +    while read -r file ; do
>> +	# check if it's an ELF file
>> +	path=${rootdir}/${file}
>> +	if ${PATCHELF} --print-rpath "${path}" > /dev/null 2>&1; then
>> +	    # Work around file busy issue with patchelf binary
>> +	    if [ "${PATCHELF}" == "${path}" ]; then
>> +		cp "${PATCHELF}" "${PATCHELF}.__patched__"
>> +		${PATCHELF} --debug --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${PATCHELF}.__patched__"
> 
>   Do we want to keep the --debug?

No, in the end. But I left it for the first tries.

Wolfgang.

[Buildroot] [RFC PATCH v5 03/11] support/scripts: add fix-rpath script to sanitize the rpath

[Samuel Martin]

Hi Wolfgang,

On Sat, Jul 1, 2017 at 2:15 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Hello Arnout,
>
> the latest patch was a rather quick hack to demonstrate the per package and
> installation step approach...
>
>
> Am 01.07.2017 um 12:36 schrieb Arnout Vandecappelle:
>>
>>   Hi Wolfgang,
>>
>>   First of all, let me commend you on the large number of comments and the
>> general clarity of this script.
>>
>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>
>>> From: Samuel Martin <s.martin49@gmail.com>
>>>
>>> The script "fix-rpath" can scan a list of files provided via
>>> command line argument, identify ELF files, check their RPATH and
>>> fix it in a proper way. The RPATH fixup is done by the patchelf
>>> utility using the option "--make-rpath-relative <root-directory>".
>>>
>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> ---
>>>   support/scripts/fix-rpath | 100
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 100 insertions(+)
>>>   create mode 100755 support/scripts/fix-rpath
>>>
>>> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
>>> new file mode 100755
>>> index 0000000..405108d
>>> --- /dev/null
>>> +++ b/support/scripts/fix-rpath
>>> @@ -0,0 +1,100 @@
>>> +#!/usr/bin/env bash
>>> +
>>> +# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
>>
>>
>>   Almost nothing is left of the original version by Samuel, so I think you
>> can
>> safely add your own name to the Copyright statement :-)
>
>
> Well, fine for me and nobody else complains.

Please do, you contribute at least as much as i did on this ;-)

[...]
>
>>
>>> +           ;;
>>> +
>>> +       target)
>>> +           rootdir="${TARGET_DIR}"
>>> +           sanitize_extra_args+=( "--no-standard-lib-dirs" )
>>
>>
>>   Since this is the only variation between the three, I think it would be
>> easier
>> to handle that from the caller. So
>>
>> ${0} ROOT_DIR FILES_LIST PATCHELF_ARGS...
>
>
> Could be done but we need to pass then almost all args, including
> "--remove-rpath", "--make-rpath-rleative"... Maybe that's not that nice in
> "pkg-generic.mk". But we could drop $tree and  use the $ROOT_DIR instead.

Indeed. IIRC, this was done to avoid carrying to many arguments on the
script invocation, and correctly support any host location (since it
can be set by the user via menuconfig).
If everything is passed from the invocation in pkg-generic.mk, it is ok TM.

>
>>
>>> +           ;;
>>> +
>>> +       *)
>>> +           usage
>>> +           exit 1
>>> +           ;;
>>> +    esac
>>> +
>>> +    while read -r file ; do
>>> +       # check if it's an ELF file
>>> +       path=${rootdir}/${file}
>>> +       if ${PATCHELF} --print-rpath "${path}" > /dev/null 2>&1; then
>>> +           # Work around file busy issue with patchelf binary
>>> +           if [ "${PATCHELF}" == "${path}" ]; then
>>> +               cp "${PATCHELF}" "${PATCHELF}.__patched__"
>>> +               ${PATCHELF} --debug --make-rpath-relative "${rootdir}"
>>> ${sanitize_extra_args[@]} "${PATCHELF}.__patched__"
>>
>>
>>   Do we want to keep the --debug?
>
>
> No, in the end. But I left it for the first tries.
>
> Wolfgang.
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

[Buildroot] [RFC PATCH v5 03/11] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Am 01.07.2017 um 15:13 schrieb Samuel Martin:
> Hi Wolfgang,
> 
> On Sat, Jul 1, 2017 at 2:15 PM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Hello Arnout,
>>
>> the latest patch was a rather quick hack to demonstrate the per package and
>> installation step approach...
>>
>>
>> Am 01.07.2017 um 12:36 schrieb Arnout Vandecappelle:
>>>
>>>    Hi Wolfgang,
>>>
>>>    First of all, let me commend you on the large number of comments and the
>>> general clarity of this script.
>>>
>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>>
>>>> From: Samuel Martin <s.martin49@gmail.com>
>>>>
>>>> The script "fix-rpath" can scan a list of files provided via
>>>> command line argument, identify ELF files, check their RPATH and
>>>> fix it in a proper way. The RPATH fixup is done by the patchelf
>>>> utility using the option "--make-rpath-relative <root-directory>".
>>>>
>>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>> ---
>>>>    support/scripts/fix-rpath | 100
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 100 insertions(+)
>>>>    create mode 100755 support/scripts/fix-rpath
>>>>
>>>> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
>>>> new file mode 100755
>>>> index 0000000..405108d
>>>> --- /dev/null
>>>> +++ b/support/scripts/fix-rpath
>>>> @@ -0,0 +1,100 @@
>>>> +#!/usr/bin/env bash
>>>> +
>>>> +# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
>>>
>>>
>>>    Almost nothing is left of the original version by Samuel, so I think you
>>> can
>>> safely add your own name to the Copyright statement :-)
>>
>>
>> Well, fine for me and nobody else complains.
> 
> Please do, you contribute at least as much as i did on this ;-)

OK, I will just add my copyright (keeping yours as well).

Wolfgang.

[Buildroot] [RFC PATCH v5 04/11] pkg-generic: create a list af installed files per step and package

[Wolfgang Grandegger]

The GLOBAL_INSTRUMENTATION_HOOKS "step_pkg_size*" are extended to create
a file with the list of all installed files per step and package. The
file name is "<package-build-dir>/.br_<host|staging|target>_filelist".
This file could be used by other hooks to scan the installed files,
e.g. to check the bin arch, the host rpath or for rpath sanitation.

For a quick hack, the "packages-file-list.txt" is created as before.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 package/pkg-generic.mk | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f474704..825ab0c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -57,33 +57,51 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 
 # Hooks to collect statistics about installed files
 
-# This hook will be called before the target installation of a
-# package. We store in a file named .br_filelist_before the list of
-# files currently installed in the target. Note that the MD5 is also
-# stored, in order to identify if the files are overwritten.
+# This hook will be called before the installation of a host,
+# staging or target package.
+# We store in a file named .br_filelist_before the list of files
+# currently installed in the root directory. Note that the MD5 is
+# also stored, in order to identify if the files are overwritten.
+# Arguments:
+# $1: path to the root directory of host, staging or target tree
 define step_pkg_size_start
-	(cd $(TARGET_DIR) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
+	(cd $(1) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
 		$($(PKG)_DIR)/.br_filelist_before
 endef
 
-# This hook will be called after the target installation of a
-# package. We store in a file named .br_filelist_after the list of
-# files (and their MD5) currently installed in the target. We then do
-# a diff with the .br_filelist_before to compute the list of files
-# installed by this package.
+# This hook will be called before the installation of a host,
+# staging or target package.
+# We store in a file named .br_filelist_after the list of files
+# (and their MD5) currently installed in the root directory. We
+# then do a diff with the .br_filelist_before to compute the list
+# of files installed by this package.
+# Arguments:
+# $1: path to the root directory of host, staging or target tree
+# $2: name of the installation tree ("host"|"staging"|"target")
+# $3: name of the package
 define step_pkg_size_end
-	(cd $(TARGET_DIR); find . -type f -print0 | xargs -0 md5sum) | sort > \
+	(cd $(1); find . -type f -print0 | xargs -0 md5sum) | sort > \
 		$($(PKG)_DIR)/.br_filelist_after
+	: > $($(PKG)_DIR)/.br_$(2)_filelist; \
 	comm -13 $($(PKG)_DIR)/.br_filelist_before $($(PKG)_DIR)/.br_filelist_after | \
 		while read hash file ; do \
-			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
+			echo "$${file}" | sed -e 's/^\.\///' >> $($(PKG)_DIR)/.br_$(2)_filelist ; \
+			if [ $(2) = "target" ]; then \
+				echo "$(3),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
+			fi ; \
 		done
 endef
 
 define step_pkg_size
+	$(if $(filter install-host,$(2)),\
+		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(HOST_DIR))) \
+		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(HOST_DIR),"host",$(3))))
+	$(if $(filter install-staging,$(2)),\
+		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(STAGING_DIR))) \
+		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(STAGING_DIR),"staging",$(3))))
 	$(if $(filter install-target,$(2)),\
-		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(3))) \
-		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(3))))
+		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(TARGET_DIR))) \
+		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(TARGET_DIR),"target",$(3))))
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 
-- 
2.7.4

[Buildroot] [RFC PATCH v5 04/11] pkg-generic: create a list af installed files per step and package

[Samuel Martin]

Hi Wolfgang,

In the commit title: s/af/of/

On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> The GLOBAL_INSTRUMENTATION_HOOKS "step_pkg_size*" are extended to create
> a file with the list of all installed files per step and package. The
> file name is "<package-build-dir>/.br_<host|staging|target>_filelist".
> This file could be used by other hooks to scan the installed files,
> e.g. to check the bin arch, the host rpath or for rpath sanitation.
>
> For a quick hack, the "packages-file-list.txt" is created as before.

Nit: you could add few words why you needed this here, but I figured
out while reviewing the rest of the series ;-)

Regards,

-- 
Samuel

[Buildroot] [RFC PATCH v5 04/11] pkg-generic: create a list af installed files per step and package

[Wolfgang Grandegger]

Am 01.07.2017 um 16:00 schrieb Samuel Martin:
> Hi Wolfgang,
> 
> In the commit title: s/af/of/
> 
> On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> The GLOBAL_INSTRUMENTATION_HOOKS "step_pkg_size*" are extended to create
>> a file with the list of all installed files per step and package. The
>> file name is "<package-build-dir>/.br_<host|staging|target>_filelist".
>> This file could be used by other hooks to scan the installed files,
>> e.g. to check the bin arch, the host rpath or for rpath sanitation.
>>
>> For a quick hack, the "packages-file-list.txt" is created as before.
> 
> Nit: you could add few words why you needed this here, but I figured
> out while reviewing the rest of the series ;-)

For the moment it's just a quick hack. Maybe the file with the list of 
the installed packages could be used by other hooks as well 
(check-bin-arch, check-host-rpath, etc.

Wolfgang.

[Buildroot] [RFC PATCH v5 04/11] pkg-generic: create a list af installed files per step and package

[Arnout Vandecappelle]

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> The GLOBAL_INSTRUMENTATION_HOOKS "step_pkg_size*" are extended to create
> a file with the list of all installed files per step and package. The
> file name is "<package-build-dir>/.br_<host|staging|target>_filelist".
> This file could be used by other hooks to scan the installed files,
> e.g. to check the bin arch, the host rpath or for rpath sanitation.
> 
> For a quick hack, the "packages-file-list.txt" is created as before.

 And the intention would be to move the generation of packages-file-list.txt to
the finalize step, right?

> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  package/pkg-generic.mk | 46 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f474704..825ab0c 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -57,33 +57,51 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  
>  # Hooks to collect statistics about installed files
>  
> -# This hook will be called before the target installation of a
> -# package. We store in a file named .br_filelist_before the list of
> -# files currently installed in the target. Note that the MD5 is also
> -# stored, in order to identify if the files are overwritten.
> +# This hook will be called before the installation of a host,
> +# staging or target package.

 Just "package" - all packages are one of the three.

> +# We store in a file named .br_filelist_before the list of files
> +# currently installed in the root directory. Note that the MD5 is
> +# also stored, in order to identify if the files are overwritten.
> +# Arguments:
> +# $1: path to the root directory of host, staging or target tree

 Could you include http://patchwork.ozlabs.org/patch/724235/ in your series? It
cleans it up a little bit.

>  define step_pkg_size_start
> -	(cd $(TARGET_DIR) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
> +	(cd $(1) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
>  		$($(PKG)_DIR)/.br_filelist_before
>  endef
>  
> -# This hook will be called after the target installation of a
> -# package. We store in a file named .br_filelist_after the list of
> -# files (and their MD5) currently installed in the target. We then do
> -# a diff with the .br_filelist_before to compute the list of files
> -# installed by this package.
> +# This hook will be called before the installation of a host,
                              after                        package.

> +# staging or target package.
> +# We store in a file named .br_filelist_after the list of files
> +# (and their MD5) currently installed in the root directory. We
> +# then do a diff with the .br_filelist_before to compute the list
> +# of files installed by this package.
> +# Arguments:
> +# $1: path to the root directory of host, staging or target tree
> +# $2: name of the installation tree ("host"|"staging"|"target")
> +# $3: name of the package
>  define step_pkg_size_end
> -	(cd $(TARGET_DIR); find . -type f -print0 | xargs -0 md5sum) | sort > \
> +	(cd $(1); find . -type f -print0 | xargs -0 md5sum) | sort > \
>  		$($(PKG)_DIR)/.br_filelist_after
> +	: > $($(PKG)_DIR)/.br_$(2)_filelist; \
                                           ^^^Not needed

 But instead of doing this, I have a slight preference for doing the redirect
outside of the while loop instead of appending to the file inside the loop. It's
more efficient and also easier to read IMO.

>  	comm -13 $($(PKG)_DIR)/.br_filelist_before $($(PKG)_DIR)/.br_filelist_after | \
>  		while read hash file ; do \
> -			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
> +			echo "$${file}" | sed -e 's/^\.\///' >> $($(PKG)_DIR)/.br_$(2)_filelist ; \
                                              So remove
this:^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +			if [ $(2) = "target" ]; then \
> +				echo "$(3),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
> +			fi ; \
>  		done
   And insert here:  > $($(PKG)_DIR)/.br_$(2)_filelist

>  endef
>  
>  define step_pkg_size
> +	$(if $(filter install-host,$(2)),\
> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(HOST_DIR))) \
> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(HOST_DIR),"host",$(3))))
> +	$(if $(filter install-staging,$(2)),\
> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(STAGING_DIR))) \
> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(STAGING_DIR),"staging",$(3))))
>  	$(if $(filter install-target,$(2)),\
> -		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(3))) \
> -		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(3))))
> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(TARGET_DIR))) \
> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(TARGET_DIR),"target",$(3))))
>  endef

 This is getting quite complicated... I'm thinking the following:

define step_pkg_size_install-host
	$(call step_pkg_size_$(1),$(HOST_DIR),$(2),$(3))
endef

define step_pkg_size_install-staging
	$(call step_pkg_size_$(1),$(STAGING_DIR),$(2),$(3))
endef

define step_pkg_size_install-target
	$(call step_pkg_size_$(1),$(TARGET_DIR),$(2),$(3))
endef

define step_pkg_size
	$(call step_pkg_size_$(2),$(1),$(2),$(3))
endef

(untested, as usual) but I'm not entirely sure that it's better. Obviously, this
should be done in a separate patch.

 Regards,
 Arnout


>  GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
>  
> 

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

[Buildroot] [RFC PATCH v5 04/11] pkg-generic: create a list af installed files per step and package

[Wolfgang Grandegger]

Hello Arnount,

Am 01.07.2017 um 18:15 schrieb Arnout Vandecappelle:
> 
> 
> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>> The GLOBAL_INSTRUMENTATION_HOOKS "step_pkg_size*" are extended to create
>> a file with the list of all installed files per step and package. The
>> file name is "<package-build-dir>/.br_<host|staging|target>_filelist".
>> This file could be used by other hooks to scan the installed files,
>> e.g. to check the bin arch, the host rpath or for rpath sanitation.
>>
>> For a quick hack, the "packages-file-list.txt" is created as before.
> 
>   And the intention would be to move the generation of packages-file-list.txt to
> the finalize step, right?
> 
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   package/pkg-generic.mk | 46 ++++++++++++++++++++++++++++++++--------------
>>   1 file changed, 32 insertions(+), 14 deletions(-)
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index f474704..825ab0c 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -57,33 +57,51 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>>   
>>   # Hooks to collect statistics about installed files
>>   
>> -# This hook will be called before the target installation of a
>> -# package. We store in a file named .br_filelist_before the list of
>> -# files currently installed in the target. Note that the MD5 is also
>> -# stored, in order to identify if the files are overwritten.
>> +# This hook will be called before the installation of a host,
>> +# staging or target package.
> 
>   Just "package" - all packages are one of the three.
> 
>> +# We store in a file named .br_filelist_before the list of files
>> +# currently installed in the root directory. Note that the MD5 is
>> +# also stored, in order to identify if the files are overwritten.
>> +# Arguments:
>> +# $1: path to the root directory of host, staging or target tree
> 
>   Could you include http://patchwork.ozlabs.org/patch/724235/ in your series? It
> cleans it up a little bit.
> 
>>   define step_pkg_size_start
>> -	(cd $(TARGET_DIR) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
>> +	(cd $(1) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
>>   		$($(PKG)_DIR)/.br_filelist_before
>>   endef
>>   
>> -# This hook will be called after the target installation of a
>> -# package. We store in a file named .br_filelist_after the list of
>> -# files (and their MD5) currently installed in the target. We then do
>> -# a diff with the .br_filelist_before to compute the list of files
>> -# installed by this package.
>> +# This hook will be called before the installation of a host,
>                                after                        package.
> 
>> +# staging or target package.
>> +# We store in a file named .br_filelist_after the list of files
>> +# (and their MD5) currently installed in the root directory. We
>> +# then do a diff with the .br_filelist_before to compute the list
>> +# of files installed by this package.
>> +# Arguments:
>> +# $1: path to the root directory of host, staging or target tree
>> +# $2: name of the installation tree ("host"|"staging"|"target")
>> +# $3: name of the package
>>   define step_pkg_size_end
>> -	(cd $(TARGET_DIR); find . -type f -print0 | xargs -0 md5sum) | sort > \
>> +	(cd $(1); find . -type f -print0 | xargs -0 md5sum) | sort > \
>>   		$($(PKG)_DIR)/.br_filelist_after
>> +	: > $($(PKG)_DIR)/.br_$(2)_filelist; \
>                                             ^^^Not needed
> 
>   But instead of doing this, I have a slight preference for doing the redirect
> outside of the while loop instead of appending to the file inside the loop. It's
> more efficient and also easier to read IMO.
> 
>>   	comm -13 $($(PKG)_DIR)/.br_filelist_before $($(PKG)_DIR)/.br_filelist_after | \
>>   		while read hash file ; do \
>> -			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
>> +			echo "$${file}" | sed -e 's/^\.\///' >> $($(PKG)_DIR)/.br_$(2)_filelist ; \
>                                                So remove
> this:^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +			if [ $(2) = "target" ]; then \
>> +				echo "$(3),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
>> +			fi ; \
>>   		done
>     And insert here:  > $($(PKG)_DIR)/.br_$(2)_filelist
> 
>>   endef
>>   
>>   define step_pkg_size
>> +	$(if $(filter install-host,$(2)),\
>> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(HOST_DIR))) \
>> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(HOST_DIR),"host",$(3))))
>> +	$(if $(filter install-staging,$(2)),\
>> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(STAGING_DIR))) \
>> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(STAGING_DIR),"staging",$(3))))
>>   	$(if $(filter install-target,$(2)),\
>> -		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(3))) \
>> -		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(3))))
>> +		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(TARGET_DIR))) \
>> +		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(TARGET_DIR),"target",$(3))))
>>   endef
> 
>   This is getting quite complicated... I'm thinking the following:
> 
> define step_pkg_size_install-host
> 	$(call step_pkg_size_$(1),$(HOST_DIR),$(2),$(3))
> endef
> 
> define step_pkg_size_install-staging
> 	$(call step_pkg_size_$(1),$(STAGING_DIR),$(2),$(3))
> endef
> 
> define step_pkg_size_install-target
> 	$(call step_pkg_size_$(1),$(TARGET_DIR),$(2),$(3))
> endef
> 
> define step_pkg_size
> 	$(call step_pkg_size_$(2),$(1),$(2),$(3))
> endef
> 
> (untested, as usual) but I'm not entirely sure that it's better. Obviously, this
> should be done in a separate patch.

This needs polish to be more efficient and elegant. Already the name is 
not a good choice. I just hijecked it to get the job done for the moment.

Wolfgang.

[Buildroot] [RFC PATCH v5 05/11] core: we need host-patchelf as the very first package for RPATH sanitation

[Wolfgang Grandegger]

For this purpose we set DEPENDENCIES_HOST_PREREQ to host-patchelf early
in the Makefile.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 88d98e0..f5ea6a8 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,8 @@ include package/Makefile.in
 # arch/arch.mk.* must be after package/Makefile.in because it may need to
 # complement variables defined therein, like BR_NO_CHECK_HASH_FOR.
 -include $(wildcard arch/arch.mk.*)
+# We need patchelf early for RPATH sanitation
+DEPENDENCIES_HOST_PREREQ += host-patchelf
 include support/dependencies/dependencies.mk
 
 include toolchain/*.mk
-- 
2.7.4

[Buildroot] [RFC PATCH v5 05/11] core: we need host-patchelf as the very first package for RPATH sanitation

[Arnout Vandecappelle]

 Subject line is a bit long, how about

core: add host-patchelf to DEPENDENCIES_HOST_PREREQ

(in general, the subject line says what is done, the rest of the commit message
explains why and sometimes how).

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> For this purpose we set DEPENDENCIES_HOST_PREREQ to host-patchelf early
> in the Makefile.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 88d98e0..f5ea6a8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -483,6 +483,8 @@ include package/Makefile.in
>  # arch/arch.mk.* must be after package/Makefile.in because it may need to
>  # complement variables defined therein, like BR_NO_CHECK_HASH_FOR.
>  -include $(wildcard arch/arch.mk.*)
> +# We need patchelf early for RPATH sanitation

 It's sanitization, not sanitation.

 Regards,
 Arnout

> +DEPENDENCIES_HOST_PREREQ += host-patchelf
>  include support/dependencies/dependencies.mk
>  
>  include toolchain/*.mk
> 

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

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Wolfgang Grandegger]

The hook calls the script "fix-rpath" at the end of the installation
step.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 package/pkg-generic.mk | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 825ab0c..8812193 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -139,6 +139,16 @@ define step_check_build_dir
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
 
+define step_sanitize_rpath
+	$(if $(filter install-host-end,$(2)-$(1)),\
+		support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
+	$(if $(filter install-staging-end,$(2)-$(1)),\
+		support/scripts/fix-rpath "staging" $($(PKG)_DIR)/.br_staging_filelist)
+	$(if $(filter install-target-end,$(2)-$(1)),\
+		support/scripts/fix-rpath "target" $($(PKG)_DIR)/.br_target_filelist)
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += step_sanitize_rpath
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user
-- 
2.7.4

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Arnout Vandecappelle]

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> The hook calls the script "fix-rpath" at the end of the installation
> step.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

 Looks good to me. I'm not adding my reviewed-by yet because it will still
change when the fix-rpath arguments change.

 Regards,
 Arnout

> ---
>  package/pkg-generic.mk | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 825ab0c..8812193 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -139,6 +139,16 @@ define step_check_build_dir
>  endef
>  GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>  
> +define step_sanitize_rpath
> +	$(if $(filter install-host-end,$(2)-$(1)),\
> +		support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
> +	$(if $(filter install-staging-end,$(2)-$(1)),\
> +		support/scripts/fix-rpath "staging" $($(PKG)_DIR)/.br_staging_filelist)
> +	$(if $(filter install-target-end,$(2)-$(1)),\
> +		support/scripts/fix-rpath "target" $($(PKG)_DIR)/.br_target_filelist)
> +endef
> +GLOBAL_INSTRUMENTATION_HOOKS += step_sanitize_rpath
> +
>  # User-supplied script
>  ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
>  define step_user
> 

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

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Arnout Vandecappelle]

On 03-07-17 23:47, Arnout Vandecappelle wrote:
> 
> 
> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>> The hook calls the script "fix-rpath" at the end of the installation
>> step.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> 
>  Looks good to me. I'm not adding my reviewed-by yet because it will still
> change when the fix-rpath arguments change.
> 
>  Regards,
>  Arnout
> 
>> ---
>>  package/pkg-generic.mk | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index 825ab0c..8812193 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>  endef
>>  GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>  
>> +define step_sanitize_rpath
>> +	$(if $(filter install-host-end,$(2)-$(1)),\
>> +		support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
>> +	$(if $(filter install-staging-end,$(2)-$(1)),\
>> +		support/scripts/fix-rpath "staging" $($(PKG)_DIR)/.br_staging_filelist)
>> +	$(if $(filter install-target-end,$(2)-$(1)),\
>> +		support/scripts/fix-rpath "target" $($(PKG)_DIR)/.br_target_filelist)

 After reviewing the qt.conf thing, I realized that this is in fact not enough.
Indeed, staging-install may install things into host dir as well...

 So I think that both for generating the lists and for doing fix-rpath, we
should not just look at the directory corresponding to the step, but to all three.

 Since that means that the same directory may be iterated over three times, I
think it's best to also make a total of 9 files:

.br_host_install_host_filelist   -> things installed to host during host-install
.br_host_install_target_filelist -> things installed to target during host-install
.br_target_install_host_filelist -> things installed to target during target-install

 Ideally .br_host_install_target_filelist etc. should be empty, so we can also
warn or error if that is not the case.

 What do you think?

 Regards,
 Arnout

>> +endef
>> +GLOBAL_INSTRUMENTATION_HOOKS += step_sanitize_rpath
>> +
>>  # User-supplied script
>>  ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
>>  define step_user
>>
> 

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

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Wolfgang Grandegger]

Am 04.07.2017 um 00:30 schrieb Arnout Vandecappelle:
> 
> 
> On 03-07-17 23:47, Arnout Vandecappelle wrote:
>>
>>
>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>> The hook calls the script "fix-rpath" at the end of the installation
>>> step.
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>
>>   Looks good to me. I'm not adding my reviewed-by yet because it will still
>> change when the fix-rpath arguments change.
>>
>>   Regards,
>>   Arnout
>>
>>> ---
>>>   package/pkg-generic.mk | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>> index 825ab0c..8812193 100644
>>> --- a/package/pkg-generic.mk
>>> +++ b/package/pkg-generic.mk
>>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>>   endef
>>>   GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>>   
>>> +define step_sanitize_rpath
>>> +	$(if $(filter install-host-end,$(2)-$(1)),\
>>> +		support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
>>> +	$(if $(filter install-staging-end,$(2)-$(1)),\
>>> +		support/scripts/fix-rpath "staging" $($(PKG)_DIR)/.br_staging_filelist)
>>> +	$(if $(filter install-target-end,$(2)-$(1)),\
>>> +		support/scripts/fix-rpath "target" $($(PKG)_DIR)/.br_target_filelist)
> 
>   After reviewing the qt.conf thing, I realized that this is in fact not enough.
> Indeed, staging-install may install things into host dir as well...
> 
>   So I think that both for generating the lists and for doing fix-rpath, we
> should not just look at the directory corresponding to the step, but to all three.
> 
>   Since that means that the same directory may be iterated over three times, I
> think it's best to also make a total of 9 files:
> 
> .br_host_install_host_filelist   -> things installed to host during host-install
> .br_host_install_target_filelist -> things installed to target during host-install
> .br_target_install_host_filelist -> things installed to target during target-install

Or we store the full path of installed files of all trees in one 
.br_install_filelist. fix-rpath could then act upon the path prefix 
(being equal to $HOST_DIR, etc.).

>   Ideally .br_host_install_target_filelist etc. should be empty, so we can also
> warn or error if that is not the case.
> 
>   What do you think?

Well, things are getting complicated then. Maybe doing the global 
sanitization is then the better choice. I need to think a bit more about it.

Wolfgang.

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Wolfgang Grandegger]

Hello Arnout,

Am 04.07.2017 um 10:22 schrieb Wolfgang Grandegger:
> 
> 
> Am 04.07.2017 um 00:30 schrieb Arnout Vandecappelle:
>>
>>
>> On 03-07-17 23:47, Arnout Vandecappelle wrote:
>>>
>>>
>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>> The hook calls the script "fix-rpath" at the end of the installation
>>>> step.
>>>>
>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>
>>>   Looks good to me. I'm not adding my reviewed-by yet because it will 
>>> still
>>> change when the fix-rpath arguments change.
>>>
>>>   Regards,
>>>   Arnout
>>>
>>>> ---
>>>>   package/pkg-generic.mk | 10 ++++++++++
>>>>   1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>>> index 825ab0c..8812193 100644
>>>> --- a/package/pkg-generic.mk
>>>> +++ b/package/pkg-generic.mk
>>>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>>>   endef
>>>>   GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>>> +define step_sanitize_rpath
>>>> +    $(if $(filter install-host-end,$(2)-$(1)),\
>>>> +        support/scripts/fix-rpath "host" 
>>>> $($(PKG)_DIR)/.br_host_filelist)
>>>> +    $(if $(filter install-staging-end,$(2)-$(1)),\
>>>> +        support/scripts/fix-rpath "staging" 
>>>> $($(PKG)_DIR)/.br_staging_filelist)
>>>> +    $(if $(filter install-target-end,$(2)-$(1)),\
>>>> +        support/scripts/fix-rpath "target" 
>>>> $($(PKG)_DIR)/.br_target_filelist)
>>
>>   After reviewing the qt.conf thing, I realized that this is in fact 
>> not enough.
>> Indeed, staging-install may install things into host dir as well...
>>
>>   So I think that both for generating the lists and for doing 
>> fix-rpath, we
>> should not just look at the directory corresponding to the step, but 
>> to all three.
>>
>>   Since that means that the same directory may be iterated over three 
>> times, I
>> think it's best to also make a total of 9 files:
>>
>> .br_host_install_host_filelist   -> things installed to host during 
>> host-install
>> .br_host_install_target_filelist -> things installed to target during 
>> host-install
>> .br_target_install_host_filelist -> things installed to target during 
>> target-install
> 
> Or we store the full path of installed files of all trees in one 
> .br_install_filelist. fix-rpath could then act upon the path prefix 
> (being equal to $HOST_DIR, etc.).

That would even simplify things (in pkg-generic.mk). Either I use the 
full path or add the name of the tree before the relative file name:

   host "file-name"
   staging "another-file-name"

Then fix-rpath would scan that file and sanitize the file according to 
the tree type. I think using the absolute path is more efficient as
I can scan in fix-rpath directly for the files with that name. If it's 
an ELF file, it acts upon the rootdir prefix (host, staging or target).

What do you think?

Wolfgang.

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Wolfgang Grandegger]

Hello,

Am 04.07.2017 um 10:57 schrieb Wolfgang Grandegger:
> Hello Arnout,
> 
> Am 04.07.2017 um 10:22 schrieb Wolfgang Grandegger:
>>
>>
>> Am 04.07.2017 um 00:30 schrieb Arnout Vandecappelle:
>>>
>>>
>>> On 03-07-17 23:47, Arnout Vandecappelle wrote:
>>>>
>>>>
>>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>>> The hook calls the script "fix-rpath" at the end of the installation
>>>>> step.
>>>>>
>>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>>
>>>>   Looks good to me. I'm not adding my reviewed-by yet because it 
>>>> will still
>>>> change when the fix-rpath arguments change.
>>>>
>>>>   Regards,
>>>>   Arnout
>>>>
>>>>> ---
>>>>>   package/pkg-generic.mk | 10 ++++++++++
>>>>>   1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>>>> index 825ab0c..8812193 100644
>>>>> --- a/package/pkg-generic.mk
>>>>> +++ b/package/pkg-generic.mk
>>>>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>>>>   endef
>>>>>   GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>>>> +define step_sanitize_rpath
>>>>> +    $(if $(filter install-host-end,$(2)-$(1)),\
>>>>> +        support/scripts/fix-rpath "host" 
>>>>> $($(PKG)_DIR)/.br_host_filelist)
>>>>> +    $(if $(filter install-staging-end,$(2)-$(1)),\
>>>>> +        support/scripts/fix-rpath "staging" 
>>>>> $($(PKG)_DIR)/.br_staging_filelist)
>>>>> +    $(if $(filter install-target-end,$(2)-$(1)),\
>>>>> +        support/scripts/fix-rpath "target" 
>>>>> $($(PKG)_DIR)/.br_target_filelist)
>>>
>>>   After reviewing the qt.conf thing, I realized that this is in fact 
>>> not enough.
>>> Indeed, staging-install may install things into host dir as well...
>>>
>>>   So I think that both for generating the lists and for doing 
>>> fix-rpath, we
>>> should not just look at the directory corresponding to the step, but 
>>> to all three.
>>>
>>>   Since that means that the same directory may be iterated over three 
>>> times, I
>>> think it's best to also make a total of 9 files:
>>>
>>> .br_host_install_host_filelist   -> things installed to host during 
>>> host-install
>>> .br_host_install_target_filelist -> things installed to target during 
>>> host-install
>>> .br_target_install_host_filelist -> things installed to target during 
>>> target-install
>>
>> Or we store the full path of installed files of all trees in one 
>> .br_install_filelist. fix-rpath could then act upon the path prefix 
>> (being equal to $HOST_DIR, etc.).
> 
> That would even simplify things (in pkg-generic.mk). Either I use the 
> full path or add the name of the tree before the relative file name:
> 
>    host "file-name"
>    staging "another-file-name"
> 
> Then fix-rpath would scan that file and sanitize the file according to 
> the tree type. I think using the absolute path is more efficient as
> I can scan in fix-rpath directly for the files with that name. If it's 
> an ELF file, it acts upon the rootdir prefix (host, staging or target).
> 
> What do you think?

Thinking more about it... Generating the list of all installed files 
over and over (for *each* packet *twice*) again is not very efficient.
Here some measurement on my rather fast i7 7700K PC with a rather big BSP:

$ cd <a-host-dir>
$ time find . -type f -print0 | xargs -0 md5sum | sort > ~/junk/filelist
real	1m43.422s

Doing it once at the end is likely more efficient. To avoid rpath 
sanitization with subsequent makes, we could maybe used some tags.

Wolfgang.


> 
> Wolfgang.

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Arnout Vandecappelle]

On 04-07-17 12:59, Wolfgang Grandegger wrote:
> Hello,
> 
> Am 04.07.2017 um 10:57 schrieb Wolfgang Grandegger:
>> Hello Arnout,
>>
>> Am 04.07.2017 um 10:22 schrieb Wolfgang Grandegger:
>>>
>>>
>>> Am 04.07.2017 um 00:30 schrieb Arnout Vandecappelle:
>>>>
>>>>
>>>> On 03-07-17 23:47, Arnout Vandecappelle wrote:
>>>>>
>>>>>
>>>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>>>> The hook calls the script "fix-rpath" at the end of the installation
>>>>>> step.
>>>>>>
>>>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>>>
>>>>>   Looks good to me. I'm not adding my reviewed-by yet because it will still
>>>>> change when the fix-rpath arguments change.
>>>>>
>>>>>   Regards,
>>>>>   Arnout
>>>>>
>>>>>> ---
>>>>>>   package/pkg-generic.mk | 10 ++++++++++
>>>>>>   1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>>>>> index 825ab0c..8812193 100644
>>>>>> --- a/package/pkg-generic.mk
>>>>>> +++ b/package/pkg-generic.mk
>>>>>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>>>>>   endef
>>>>>>   GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>>>>> +define step_sanitize_rpath
>>>>>> +    $(if $(filter install-host-end,$(2)-$(1)),\
>>>>>> +        support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
>>>>>> +    $(if $(filter install-staging-end,$(2)-$(1)),\
>>>>>> +        support/scripts/fix-rpath "staging"
>>>>>> $($(PKG)_DIR)/.br_staging_filelist)
>>>>>> +    $(if $(filter install-target-end,$(2)-$(1)),\
>>>>>> +        support/scripts/fix-rpath "target"
>>>>>> $($(PKG)_DIR)/.br_target_filelist)
>>>>
>>>>   After reviewing the qt.conf thing, I realized that this is in fact not
>>>> enough.
>>>> Indeed, staging-install may install things into host dir as well...
>>>>
>>>>   So I think that both for generating the lists and for doing fix-rpath, we
>>>> should not just look at the directory corresponding to the step, but to all
>>>> three.
>>>>
>>>>   Since that means that the same directory may be iterated over three times, I
>>>> think it's best to also make a total of 9 files:
>>>>
>>>> .br_host_install_host_filelist   -> things installed to host during
>>>> host-install
>>>> .br_host_install_target_filelist -> things installed to target during
>>>> host-install
>>>> .br_target_install_host_filelist -> things installed to target during
>>>> target-install
>>>
>>> Or we store the full path of installed files of all trees in one
>>> .br_install_filelist. fix-rpath could then act upon the path prefix (being
>>> equal to $HOST_DIR, etc.).
>>
>> That would even simplify things (in pkg-generic.mk). Either I use the full
>> path or add the name of the tree before the relative file name:
>>
>>    host "file-name"
>>    staging "another-file-name"
>>
>> Then fix-rpath would scan that file and sanitize the file according to the
>> tree type. I think using the absolute path is more efficient as
>> I can scan in fix-rpath directly for the files with that name. If it's an ELF
>> file, it acts upon the rootdir prefix (host, staging or target).
>>
>> What do you think?
> 
> Thinking more about it... Generating the list of all installed files over and
> over (for *each* packet *twice*) again is not very efficient.
> Here some measurement on my rather fast i7 7700K PC with a rather big BSP:
> 
> $ cd <a-host-dir>
> $ time find . -type f -print0 | xargs -0 md5sum | sort > ~/junk/filelist
> real    1m43.422s
> 
> Doing it once at the end is likely more efficient. To avoid rpath sanitization
> with subsequent makes, we could maybe used some tags.

 OK, so it looks like nobody agrees with my idea of doing it for every package
:-( Sorry that I pointed you in that direction.

 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

[Buildroot] [RFC PATCH v5 06/11] pkg-generic: add GLOBAL_INSTRUMENTATION_HOOKS for rpath sanitation

[Wolfgang Grandegger]

Am 04.07.2017 um 13:42 schrieb Arnout Vandecappelle:
> 
> 
> On 04-07-17 12:59, Wolfgang Grandegger wrote:
>> Hello,
>>
>> Am 04.07.2017 um 10:57 schrieb Wolfgang Grandegger:
>>> Hello Arnout,
>>>
>>> Am 04.07.2017 um 10:22 schrieb Wolfgang Grandegger:
>>>>
>>>>
>>>> Am 04.07.2017 um 00:30 schrieb Arnout Vandecappelle:
>>>>>
>>>>>
>>>>> On 03-07-17 23:47, Arnout Vandecappelle wrote:
>>>>>>
>>>>>>
>>>>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>>>>>>> The hook calls the script "fix-rpath" at the end of the installation
>>>>>>> step.
>>>>>>>
>>>>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>>>>
>>>>>>    Looks good to me. I'm not adding my reviewed-by yet because it will still
>>>>>> change when the fix-rpath arguments change.
>>>>>>
>>>>>>    Regards,
>>>>>>    Arnout
>>>>>>
>>>>>>> ---
>>>>>>>    package/pkg-generic.mk | 10 ++++++++++
>>>>>>>    1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>>>>>> index 825ab0c..8812193 100644
>>>>>>> --- a/package/pkg-generic.mk
>>>>>>> +++ b/package/pkg-generic.mk
>>>>>>> @@ -139,6 +139,16 @@ define step_check_build_dir
>>>>>>>    endef
>>>>>>>    GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>>>>>>> +define step_sanitize_rpath
>>>>>>> +    $(if $(filter install-host-end,$(2)-$(1)),\
>>>>>>> +        support/scripts/fix-rpath "host" $($(PKG)_DIR)/.br_host_filelist)
>>>>>>> +    $(if $(filter install-staging-end,$(2)-$(1)),\
>>>>>>> +        support/scripts/fix-rpath "staging"
>>>>>>> $($(PKG)_DIR)/.br_staging_filelist)
>>>>>>> +    $(if $(filter install-target-end,$(2)-$(1)),\
>>>>>>> +        support/scripts/fix-rpath "target"
>>>>>>> $($(PKG)_DIR)/.br_target_filelist)
>>>>>
>>>>>    After reviewing the qt.conf thing, I realized that this is in fact not
>>>>> enough.
>>>>> Indeed, staging-install may install things into host dir as well...
>>>>>
>>>>>    So I think that both for generating the lists and for doing fix-rpath, we
>>>>> should not just look at the directory corresponding to the step, but to all
>>>>> three.
>>>>>
>>>>>    Since that means that the same directory may be iterated over three times, I
>>>>> think it's best to also make a total of 9 files:
>>>>>
>>>>> .br_host_install_host_filelist   -> things installed to host during
>>>>> host-install
>>>>> .br_host_install_target_filelist -> things installed to target during
>>>>> host-install
>>>>> .br_target_install_host_filelist -> things installed to target during
>>>>> target-install
>>>>
>>>> Or we store the full path of installed files of all trees in one
>>>> .br_install_filelist. fix-rpath could then act upon the path prefix (being
>>>> equal to $HOST_DIR, etc.).
>>>
>>> That would even simplify things (in pkg-generic.mk). Either I use the full
>>> path or add the name of the tree before the relative file name:
>>>
>>>     host "file-name"
>>>     staging "another-file-name"
>>>
>>> Then fix-rpath would scan that file and sanitize the file according to the
>>> tree type. I think using the absolute path is more efficient as
>>> I can scan in fix-rpath directly for the files with that name. If it's an ELF
>>> file, it acts upon the rootdir prefix (host, staging or target).
>>>
>>> What do you think?
>>
>> Thinking more about it... Generating the list of all installed files over and
>> over (for *each* packet *twice*) again is not very efficient.
>> Here some measurement on my rather fast i7 7700K PC with a rather big BSP:
>>
>> $ cd <a-host-dir>
>> $ time find . -type f -print0 | xargs -0 md5sum | sort > ~/junk/filelist
>> real    1m43.422s
>>
>> Doing it once at the end is likely more efficient. To avoid rpath sanitization
>> with subsequent makes, we could maybe used some tags.
> 
>   OK, so it looks like nobody agrees with my idea of doing it for every package
> :-( Sorry that I pointed you in that direction.

No problem. That appoarch looked promising at the beginning.

Wolfgang.

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Wolfgang Grandegger]

It will install the script "relocate-sdk.sh" in the HOST_DIR
allowing to adjust the path to the SDK directory in all text
files after it has been moved to a new location.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 support/misc/relocate-sdk.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100755 support/misc/relocate-sdk.sh

diff --git a/support/misc/relocate-sdk.sh b/support/misc/relocate-sdk.sh
new file mode 100755
index 0000000..1b0be33
--- /dev/null
+++ b/support/misc/relocate-sdk.sh
@@ -0,0 +1,47 @@
+#!/bin/bash
+#
+if [ "$#" -ne 0 ]; then
+    echo "Run this script to relocate the buildroot SDK at that location"
+    exit 1
+fi
+
+LOCFILE="./usr/share/buildroot/sdk-location"
+FILEPATH="$(readlink -f "$0")"
+NEWPATH="$(dirname "${FILEPATH}")"
+
+cd "${NEWPATH}"
+if [ ! -r "${LOCFILE}" ]; then
+    echo "Previous location of the buildroot SDK not found!"
+    exit 1
+fi
+OLDPATH="$(cat "${LOCFILE}")"
+
+if [ "${NEWPATH}" = "${OLDPATH}" ]; then
+    echo "This buildroot SDK has already been relocated!"
+    exit 0
+fi
+
+# Check if the path substitution does work properly, e.g.
+# a tree "/a/b/c" copied into "/a/b/c/" would not be allowed.
+newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"
+if [ "${NEWPATH}" != "${newpath}" ]; then
+    echo "Something went wrong with substituting the path!"
+    echo "Please choose another location for your SDK!"
+    exit 1
+fi
+
+echo "Relocating the buildroot SDK from ${OLDPATH} to ${NEWPATH} ..."
+
+# Make sure file uses the right language
+export LC_ALL=C
+# Replace the old path with the new one in all text files
+while read -r FILE ; do
+    if file -b --mime-type "${FILE}" | grep -q '^text/' && [ "${FILE}" != "${LOCFILE}" ]
+    then
+	sed -i "s\\${OLDPATH}\\${NEWPATH}\\g" "${FILE}"
+    fi;
+done < <(grep -lr "${OLDPATH}" .)
+
+# At the very end, we update the location file to not break the
+# SDK if this script gets interruted.
+sed -i "s\\${OLDPATH}\\${NEWPATH}\\g" ${LOCFILE}
-- 
2.7.4

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Arnout Vandecappelle]

 Type in the subject: reloacte -> relocate

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> It will install the script "relocate-sdk.sh" in the HOST_DIR
> allowing to adjust the path to the SDK directory in all text
> files after it has been moved to a new location.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  support/misc/relocate-sdk.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100755 support/misc/relocate-sdk.sh
> 
> diff --git a/support/misc/relocate-sdk.sh b/support/misc/relocate-sdk.sh
> new file mode 100755
> index 0000000..1b0be33
> --- /dev/null
> +++ b/support/misc/relocate-sdk.sh
> @@ -0,0 +1,47 @@
> +#!/bin/bash

 Minor nit: you're not using any bashism in this script so you could just as
well use /bin/sh as shebang.

> +#
> +if [ "$#" -ne 0 ]; then
> +    echo "Run this script to relocate the buildroot SDK at that location"
> +    exit 1
> +fi
> +
> +LOCFILE="./usr/share/buildroot/sdk-location"

 Minor nit: the ./ in front is redundant.

> +FILEPATH="$(readlink -f "$0")"
> +NEWPATH="$(dirname "${FILEPATH}")"
> +
> +cd "${NEWPATH}"
> +if [ ! -r "${LOCFILE}" ]; then
> +    echo "Previous location of the buildroot SDK not found!"
> +    exit 1
> +fi
> +OLDPATH="$(cat "${LOCFILE}")"
> +
> +if [ "${NEWPATH}" = "${OLDPATH}" ]; then
> +    echo "This buildroot SDK has already been relocated!"
> +    exit 0
> +fi
> +
> +# Check if the path substitution does work properly, e.g.
> +# a tree "/a/b/c" copied into "/a/b/c/" would not be allowed.
                                  ^^^^^^^ You mean /a/b/c/a/b/c ?
> +newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"

 \\ is a bit a weird sed separator... We use % mostly, sometimes @ or ,. But I'd
advise %.

> +if [ "${NEWPATH}" != "${newpath}" ]; then
> +    echo "Something went wrong with substituting the path!"
> +    echo "Please choose another location for your SDK!"
> +    exit 1
> +fi
> +
> +echo "Relocating the buildroot SDK from ${OLDPATH} to ${NEWPATH} ..."
> +
> +# Make sure file uses the right language
> +export LC_ALL=C
> +# Replace the old path with the new one in all text files
> +while read -r FILE ; do
> +    if file -b --mime-type "${FILE}" | grep -q '^text/' && [ "${FILE}" != "${LOCFILE}" ]
> +    then
> +	sed -i "s\\${OLDPATH}\\${NEWPATH}\\g" "${FILE}"

 Same remark about \ -> % here.

 Please use consistent indentation: either 4 spaces or a tab.

> +    fi;

 ; is redundant.

 Perhaps we should print a warning for non-text files that have the SDK path
encoded in them? Like

    else
        printf 'Warning: %s not relocated\n' "${FILE}"
    fi

> +done < <(grep -lr "${OLDPATH}" .)
> +
> +# At the very end, we update the location file to not break the
> +# SDK if this script gets interruted.
                                    ^p

> +sed -i "s\\${OLDPATH}\\${NEWPATH}\\g" ${LOCFILE}

 \\ -> %


 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

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Wolfgang Grandegger]

Am 04.07.2017 um 00:01 schrieb Arnout Vandecappelle:
>   Type in the subject: reloacte -> relocate
> 
> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>> It will install the script "relocate-sdk.sh" in the HOST_DIR
>> allowing to adjust the path to the SDK directory in all text
>> files after it has been moved to a new location.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   support/misc/relocate-sdk.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>   create mode 100755 support/misc/relocate-sdk.sh
>>
>> diff --git a/support/misc/relocate-sdk.sh b/support/misc/relocate-sdk.sh
>> new file mode 100755
>> index 0000000..1b0be33
>> --- /dev/null
>> +++ b/support/misc/relocate-sdk.sh
>> @@ -0,0 +1,47 @@
>> +#!/bin/bash
> 
>   Minor nit: you're not using any bashism in this script so you could just as
> well use /bin/sh as shebang.
> 
>> +#
>> +if [ "$#" -ne 0 ]; then
>> +    echo "Run this script to relocate the buildroot SDK at that location"
>> +    exit 1
>> +fi
>> +
>> +LOCFILE="./usr/share/buildroot/sdk-location"
> 
>   Minor nit: the ./ in front is redundant.
> 
>> +FILEPATH="$(readlink -f "$0")"
>> +NEWPATH="$(dirname "${FILEPATH}")"
>> +
>> +cd "${NEWPATH}"
>> +if [ ! -r "${LOCFILE}" ]; then
>> +    echo "Previous location of the buildroot SDK not found!"
>> +    exit 1
>> +fi
>> +OLDPATH="$(cat "${LOCFILE}")"
>> +
>> +if [ "${NEWPATH}" = "${OLDPATH}" ]; then
>> +    echo "This buildroot SDK has already been relocated!"
>> +    exit 0
>> +fi
>> +
>> +# Check if the path substitution does work properly, e.g.
>> +# a tree "/a/b/c" copied into "/a/b/c/" would not be allowed.
>                                    ^^^^^^^ You mean /a/b/c/a/b/c ?
>> +newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"
> 
>   \\ is a bit a weird sed separator... We use % mostly, sometimes @ or ,. But I'd
> advise %.

"%" and "@" are legal character of a file name. That's why I have chosen 
"\".

Wolfgang

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Samuel Martin]

Wolfgang,

On Tue, Jul 4, 2017 at 9:01 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>
> Am 04.07.2017 um 00:01 schrieb Arnout Vandecappelle:
>>
>>   Type in the subject: reloacte -> relocate
>>
>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
[...]
>>> +newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"
>>
>>
>>   \\ is a bit a weird sed separator... We use % mostly, sometimes @ or ,.
>> But I'd
>> advise %.
>
>
> "%" and "@" are legal character of a file name. That's why I have chosen
> "\".

I think youcan choose one of the characters from [1] (my preference
would be either ':' or '|').

[1] https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words


Regards,

-- 
Samuel

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Arnout Vandecappelle]

On 04-07-17 09:14, Samuel Martin wrote:
> Wolfgang,
> 
> On Tue, Jul 4, 2017 at 9:01 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>>
>> Am 04.07.2017 um 00:01 schrieb Arnout Vandecappelle:
>>>
>>>   Type in the subject: reloacte -> relocate
>>>
>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
> [...]
>>>> +newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"
>>>
>>>
>>>   \\ is a bit a weird sed separator... We use % mostly, sometimes @ or ,.
>>> But I'd
>>> advise %.
>>
>>
>> "%" and "@" are legal character of a file name. That's why I have chosen
>> "\".
> 
> I think youcan choose one of the characters from [1] (my preference
> would be either ':' or '|').
> 
> [1] https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

 Not a great reference. The only characters that are not allowed in Unix path
names are / and \0. Neither of them is usable for the sed replacement.

 However, Buildroot will break down if the output dir contains strange
characters. Admittedly the external toolchain dir is probably more robust. But
we already have plenty of sed replacements using % and some with @ or , or :.

 So I've discussed it here at the Summer Camp and basically everyone has a
different opinion. Since this patch is basically ready to go in (after fixing up
the type in the subject), whoever commits it will replace the \\ with their
preference.

 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

[Buildroot] [RFC PATCH v5 07/11] support/scripts: add reloacte-sdk.sh script for SDK relocation

[Wolfgang Grandegger]

Am 04.07.2017 um 12:06 schrieb Arnout Vandecappelle:
> 
> 
> On 04-07-17 09:14, Samuel Martin wrote:
>> Wolfgang,
>>
>> On Tue, Jul 4, 2017 at 9:01 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>>>
>>> Am 04.07.2017 um 00:01 schrieb Arnout Vandecappelle:
>>>>
>>>>    Type in the subject: reloacte -> relocate
>>>>
>>>> On 30-06-17 10:37, Wolfgang Grandegger wrote:
>> [...]
>>>>> +newpath="$(sed -e "s\\${OLDPATH}\\${NEWPATH}\\g" "${LOCFILE}")"
>>>>
>>>>
>>>>    \\ is a bit a weird sed separator... We use % mostly, sometimes @ or ,.
>>>> But I'd
>>>> advise %.
>>>
>>>
>>> "%" and "@" are legal character of a file name. That's why I have chosen
>>> "\".
>>
>> I think youcan choose one of the characters from [1] (my preference
>> would be either ':' or '|').
>>
>> [1] https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words
> 
>   Not a great reference. The only characters that are not allowed in Unix path
> names are / and \0. Neither of them is usable for the sed replacement.

Yes, see my other mail on that topic to Samuel. Also "my" "\" shows the 
same issue. But I thought it's exotic enough to be almost safe.

>   However, Buildroot will break down if the output dir contains strange
> characters. Admittedly the external toolchain dir is probably more robust. But
> we already have plenty of sed replacements using % and some with @ or , or :.
> 
>   So I've discussed it here at the Summer Camp and basically everyone has a
> different opinion. Since this patch is basically ready to go in (after fixing up
> the type in the subject), whoever commits it will replace the \\ with their
> preference.

My next preference would here be "|" as it, like "\" needs special 
handling (using an escape character), in contrast to:

$ mkdir dir%
$ mkdir dir@
$ mkdir dir:

Just another different opinion, of course.

Wolfgang.

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Wolfgang Grandegger]

The script "relocate-sdk.sh" is installed into the top directory of
the SDK (HOST_DIR) and the SDK location path is stored in the file
"HOST_DIR/usr/share/buildroot/sdk-location"-

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index f5ea6a8..a5dfdfd 100644
--- a/Makefile
+++ b/Makefile
@@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 
 .PHONY: world
 world: target-post-image
+	@$(call MESSAGE,"Rendering the SDK relocatable")
+	install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
+	echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
 
 # Populating the staging with the base directories is handled by the skeleton package
 $(STAGING_DIR):
-- 
2.7.4

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Samuel Martin]

Hi Wolfgang,

On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> The script "relocate-sdk.sh" is installed into the top directory of
> the SDK (HOST_DIR) and the SDK location path is stored in the file
> "HOST_DIR/usr/share/buildroot/sdk-location"-
>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index f5ea6a8..a5dfdfd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>
>  .PHONY: world
>  world: target-post-image
> +       @$(call MESSAGE,"Rendering the SDK relocatable")
> +       install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \

I don't think "&& \" is really needed here

> +       echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location

I'm not sure these few commands belong to the world target, maybe a
host-finalize target could be added...

>
>  # Populating the staging with the base directories is handled by the skeleton package
>  $(STAGING_DIR):
> --
> 2.7.4
>

Regards,

-- 
Samuel

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Arnout Vandecappelle]

On 01-07-17 16:11, Samuel Martin wrote:
> Hi Wolfgang,
> 
> On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> The script "relocate-sdk.sh" is installed into the top directory of
>> the SDK (HOST_DIR) and the SDK location path is stored in the file
>> "HOST_DIR/usr/share/buildroot/sdk-location"-
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>  Makefile | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index f5ea6a8..a5dfdfd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>>
>>  .PHONY: world
>>  world: target-post-image
>> +       @$(call MESSAGE,"Rendering the SDK relocatable")
>> +       install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
> 
> I don't think "&& \" is really needed here
> 
>> +       echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
> 
> I'm not sure these few commands belong to the world target, maybe a
> host-finalize target could be added...

 +1 to that. So

world: target-post-image host-finalize

.PHONY: host-finalize
host-finalize:
	...

 Regards,
 Arnout

> 
>>
>>  # Populating the staging with the base directories is handled by the skeleton package
>>  $(STAGING_DIR):
>> --
>> 2.7.4
>>
> 
> Regards,
> 

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

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Wolfgang Grandegger]

Am 01.07.2017 um 16:25 schrieb Arnout Vandecappelle:
> 
> 
> On 01-07-17 16:11, Samuel Martin wrote:
>> Hi Wolfgang,
>>
>> On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>>> The script "relocate-sdk.sh" is installed into the top directory of
>>> the SDK (HOST_DIR) and the SDK location path is stored in the file
>>> "HOST_DIR/usr/share/buildroot/sdk-location"-
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> ---
>>>   Makefile | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index f5ea6a8..a5dfdfd 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>>>
>>>   .PHONY: world
>>>   world: target-post-image
>>> +       @$(call MESSAGE,"Rendering the SDK relocatable")
>>> +       install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
>>
>> I don't think "&& \" is really needed here
>>
>>> +       echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
>>
>> I'm not sure these few commands belong to the world target, maybe a
>> host-finalize target could be added...
> 
>   +1 to that. So
> 
> world: target-post-image host-finalize
> 
> .PHONY: host-finalize
> host-finalize:

OK.

Wolfgang.

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Wolfgang Grandegger]

Am 01.07.2017 um 16:11 schrieb Samuel Martin:
> Hi Wolfgang,
> 
> On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> The script "relocate-sdk.sh" is installed into the top directory of
>> the SDK (HOST_DIR) and the SDK location path is stored in the file
>> "HOST_DIR/usr/share/buildroot/sdk-location"-
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   Makefile | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index f5ea6a8..a5dfdfd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>>
>>   .PHONY: world
>>   world: target-post-image
>> +       @$(call MESSAGE,"Rendering the SDK relocatable")
>> +       install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
> 
> I don't think "&& \" is really needed here

It's important. The "relocate-sdk.sh" will use it do do the relocation 
(please have a look to the script).

> 
>> +       echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
> 
> I'm not sure these few commands belong to the world target, maybe a
> host-finalize target could be added...
> 
>>
>>   # Populating the staging with the base directories is handled by the skeleton package
>>   $(STAGING_DIR):

Other opinions?

Wolfgang.

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Arnout Vandecappelle]

On 01-07-17 21:51, Wolfgang Grandegger wrote:
> 
> 
> Am 01.07.2017 um 16:11 schrieb Samuel Martin:
>> Hi Wolfgang,
>>
>> On Fri, Jun 30, 2017 at 10:37 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
>>> The script "relocate-sdk.sh" is installed into the top directory of
>>> the SDK (HOST_DIR) and the SDK location path is stored in the file
>>> "HOST_DIR/usr/share/buildroot/sdk-location"-
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> ---
>>>   Makefile | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index f5ea6a8..a5dfdfd 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>>>
>>>   .PHONY: world
>>>   world: target-post-image
>>> +       @$(call MESSAGE,"Rendering the SDK relocatable")
>>> +       install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
>>
>> I don't think "&& \" is really needed here
> 
> It's important. The "relocate-sdk.sh" will use it do do the relocation (please
> have a look to the script).

 The line below is indeed needed, but the && \ is not. Make will abort as soon
as one command fails. And anyway, the install shouldn't fail.

 By the way, while I'm commenting on this line:

	$(INSTALL) -D -m 0755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)

> 
>>
>>> +       echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
>>
>> I'm not sure these few commands belong to the world target, maybe a
>> host-finalize target could be added...
>>
>>>
>>>   # Populating the staging with the base directories is handled by the
>>> skeleton package
>>>   $(STAGING_DIR):
> 
> Other opinions?

 Not sure what you're asking here.

 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

[Buildroot] [RFC PATCH v5 08/11] core: install relocation script and location at the end of the build

[Arnout Vandecappelle]

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> The script "relocate-sdk.sh" is installed into the top directory of
> the SDK (HOST_DIR) and the SDK location path is stored in the file
> "HOST_DIR/usr/share/buildroot/sdk-location"-
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index f5ea6a8..a5dfdfd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -553,6 +553,9 @@ prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>  
>  .PHONY: world
>  world: target-post-image
> +	@$(call MESSAGE,"Rendering the SDK relocatable")

 In addition to all the comments that went before: this doesn't actually render
the SDK relocatable. So perhaps simple "Finalizing SDK" is better.

 Regards,
 Arnout

> +	install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) && \
> +	echo $(HOST_DIR) > $(HOST_DIR)/usr/share/buildroot/sdk-location
>  
>  # Populating the staging with the base directories is handled by the skeleton package
>  $(STAGING_DIR):
> 

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

[Buildroot] [RFC PATCH v5 09/11] external-toolchain: check if a buildroot SDK has already been relocated

[Wolfgang Grandegger]

The location of the buildroot SDK is stored in the file "sdk-location"
in "usr/share/buildroot". If it's content does not match the current
SDK location, ask the user to run the script "relocate-sdk.sh" in the
top directory once.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 toolchain/helpers.mk                                   | 15 +++++++++++++++
 toolchain/toolchain-external/pkg-toolchain-external.mk |  1 +
 2 files changed, 16 insertions(+)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 90834f4..43e89fc 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -479,3 +479,18 @@ define simplify_symlink
 	ln -sf "$${DOTS}$${REL_DEST}" "$${FULL_SRC}" ; \
 )
 endef
+
+#
+# Check if it's a buildroot toolchain and if it's already relocatable by
+# reading and testing the toolchain location file
+#
+# $1: toolchain installation directory
+#
+check_buildroot_sdk_relocated = \
+	if [ -r $(1)/share/buildroot/sdk-location ]; then \
+		sdkroot=`dirname "$(1)"`; \
+		if [ "`cat $(1)/share/buildroot/sdk-location`" != "$${sdkroot}" ]; then \
+			echo "Please relocate the buildroot SDK by executing \"$${sdkroot}/relocate-sdk.sh\" once!" ; \
+			exit 1 ; \
+		fi \
+	fi
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index 8269345..761221b 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -545,6 +545,7 @@ endif
 # matches the configuration provided in Buildroot: ABI, C++ support,
 # kernel headers version, type of C library and all C library features.
 define $(2)_CONFIGURE_CMDS
+	$$(Q)$$(call check_buildroot_sdk_relocated,$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR))
 	$$(Q)$$(call check_cross_compiler_exists,$$(TOOLCHAIN_EXTERNAL_CC))
 	$$(Q)$$(call check_unusable_toolchain,$$(TOOLCHAIN_EXTERNAL_CC))
 	$$(Q)SYSROOT_DIR="$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))" ; \
-- 
2.7.4

[Buildroot] [RFC PATCH v5 09/11] external-toolchain: check if a buildroot SDK has already been relocated

[Arnout Vandecappelle]

 This is really a great solution.

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> The location of the buildroot SDK is stored in the file "sdk-location"
> in "usr/share/buildroot". If it's content does not match the current
> SDK location, ask the user to run the script "relocate-sdk.sh" in the
> top directory once.

 Perhaps explain in the commit log why we can't just relocate it automatically:

The external toolchain may be a pre-installed one in a directory that is not
writeable by us. Therefore, we can't run the script directly.

> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  toolchain/helpers.mk                                   | 15 +++++++++++++++
>  toolchain/toolchain-external/pkg-toolchain-external.mk |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 90834f4..43e89fc 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -479,3 +479,18 @@ define simplify_symlink
>  	ln -sf "$${DOTS}$${REL_DEST}" "$${FULL_SRC}" ; \
>  )
>  endef
> +
> +#
> +# Check if it's a buildroot toolchain and if it's already relocatable by
> +# reading and testing the toolchain location file
> +#
> +# $1: toolchain installation directory
> +#
> +check_buildroot_sdk_relocated = \

 Since this is a multiline command, it's nicer to define it as a define:

define check_buildroot_sdk_relocated

> +	if [ -r $(1)/share/buildroot/sdk-location ]; then \
> +		sdkroot=`dirname "$(1)"`; \

 As you may have noticed, I'm working on removing the usr/ part of the HOST_DIR,
so this won't be needed any more (and in fact will be wrong). I hope that that
series will be committed tomorrow...

> +		if [ "`cat $(1)/share/buildroot/sdk-location`" != "$${sdkroot}" ]; then \
> +			echo "Please relocate the buildroot SDK by executing \"$${sdkroot}/relocate-sdk.sh\" once!" ; \
> +			exit 1 ; \
> +		fi \
> +	fi
endef

> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index 8269345..761221b 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -545,6 +545,7 @@ endif
>  # matches the configuration provided in Buildroot: ABI, C++ support,
>  # kernel headers version, type of C library and all C library features.
>  define $(2)_CONFIGURE_CMDS
> +	$$(Q)$$(call check_buildroot_sdk_relocated,$$(TOOLCHAIN_EXTERNAL_INSTALL_DIR))
>  	$$(Q)$$(call check_cross_compiler_exists,$$(TOOLCHAIN_EXTERNAL_CC))
>  	$$(Q)$$(call check_unusable_toolchain,$$(TOOLCHAIN_EXTERNAL_CC))
>  	$$(Q)SYSROOT_DIR="$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))" ; \
> 

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

[Buildroot] [RFC PATCH v5 10/11] support/scripts: check-host-rpath now handles $ORIGIN as well

[Wolfgang Grandegger]

"$ORIGIN/../../usr/lib" is also a valid RPATH for binaries in
"$hostdir/usr/bin". After RPATH sanitation, all RPATH
directories start with "$ORIGIN".

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 support/scripts/check-host-rpath | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 6ce547c..020c123 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -58,7 +58,7 @@ check_elf_has_rpath() {
         for dir in ${rpath//:/ }; do
             # Remove duplicate and trailing '/' for proper match
             dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
-            [ "${dir}" = "${hostdir}/usr/lib" ] && return 0
+            [ "${dir}" = "${hostdir}/usr/lib" -o "${dir}" = "\$ORIGIN/../../usr/lib" ] && return 0
         done
     done < <( readelf -d "${file}"                                              \
               |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
-- 
2.7.4

[Buildroot] [RFC PATCH v5 10/11] support/scripts: check-host-rpath now handles $ORIGIN as well

[Arnout Vandecappelle]

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> "$ORIGIN/../../usr/lib" is also a valid RPATH for binaries in
> "$hostdir/usr/bin". After RPATH sanitation, all RPATH
> directories start with "$ORIGIN".
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

 This patch should come way earlier in the series, because things break if you
don't have this and the sanitization is done.

 In fact, it can be applied right away:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  support/scripts/check-host-rpath | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 6ce547c..020c123 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -58,7 +58,7 @@ check_elf_has_rpath() {
>          for dir in ${rpath//:/ }; do
>              # Remove duplicate and trailing '/' for proper match
>              dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
> -            [ "${dir}" = "${hostdir}/usr/lib" ] && return 0
> +            [ "${dir}" = "${hostdir}/usr/lib" -o "${dir}" = "\$ORIGIN/../../usr/lib" ] && return 0
>          done
>      done < <( readelf -d "${file}"                                              \
>                |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
> 

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

[Buildroot] [RFC PATCH v5 10/11] support/scripts: check-host-rpath now handles $ORIGIN as well

[Thomas Petazzoni]

Hello,

On Fri, 30 Jun 2017 10:37:07 +0200, Wolfgang Grandegger wrote:
> "$ORIGIN/../../usr/lib" is also a valid RPATH for binaries in
> "$hostdir/usr/bin". After RPATH sanitation, all RPATH
> directories start with "$ORIGIN".
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  support/scripts/check-host-rpath | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to master, thanks.

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

[Buildroot] [RFC PATCH v5 11/11] package/qt5base: provide "qt.conf" to make "qmake" relocatable

[Wolfgang Grandegger]

The file "qt.conf" can be used to override the hard-coded paths that are
compiled into the Qt library. We need it to make "qmake" relocatable.

CC: Julien Corjon <corjon.j@ecagroup.com>
CC: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 package/qt5/qt5base/qt.conf.in | 6 ++++++
 package/qt5/qt5base/qt5base.mk | 8 ++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 package/qt5/qt5base/qt.conf.in

diff --git a/package/qt5/qt5base/qt.conf.in b/package/qt5/qt5base/qt.conf.in
new file mode 100644
index 0000000..48e4b92
--- /dev/null
+++ b/package/qt5/qt5base/qt.conf.in
@@ -0,0 +1,6 @@
+[Paths]
+Prefix=@@HOST_DIR@@/usr
+Sysroot=@@STAGING_DIR@@
+Headers=/usr/include/qt5
+Plugins=/usr/lib/qt/plugins
+Examples=/usr/lib/qt/examples
diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
index 5fe8bb8..71dbd5d 100644
--- a/package/qt5/qt5base/qt5base.mk
+++ b/package/qt5/qt5base/qt5base.mk
@@ -258,9 +258,17 @@ define QT5BASE_BUILD_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
 endef
 
+# The file "qt.conf" can be used to override the hard-coded paths that are
+# compiled into the Qt library. We need it to make "qmake" relocatable.
+define QT5BASE_INSTALL_QT_CONF
+	sed -e "s\\@@HOST_DIR@@\\$(HOST_DIR)\\" -e "s\\@@STAGING_DIR@@\\$(STAGING_DIR)\\" \
+	$(QT5BASE_PKGDIR)/qt.conf.in > $(HOST_DIR)/usr/bin/qt.conf
+endef
+
 define QT5BASE_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install
 	$(QT5_LA_PRL_FILES_FIXUP)
+	$(QT5BASE_INSTALL_QT_CONF)
 endef
 
 define QT5BASE_INSTALL_TARGET_LIBS
-- 
2.7.4

[Buildroot] [RFC PATCH v5 11/11] package/qt5base: provide "qt.conf" to make "qmake" relocatable

[Arnout Vandecappelle]

On 30-06-17 10:37, Wolfgang Grandegger wrote:
> The file "qt.conf" can be used to override the hard-coded paths that are
> compiled into the Qt library. We need it to make "qmake" relocatable.

 Looks good to me, except...

> 
> CC: Julien Corjon <corjon.j@ecagroup.com>
> CC: Peter Seiderer <ps.report@gmx.net>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  package/qt5/qt5base/qt.conf.in | 6 ++++++
>  package/qt5/qt5base/qt5base.mk | 8 ++++++++
>  2 files changed, 14 insertions(+)
>  create mode 100644 package/qt5/qt5base/qt.conf.in
> 
> diff --git a/package/qt5/qt5base/qt.conf.in b/package/qt5/qt5base/qt.conf.in
> new file mode 100644
> index 0000000..48e4b92
> --- /dev/null
> +++ b/package/qt5/qt5base/qt.conf.in
> @@ -0,0 +1,6 @@
> +[Paths]
> +Prefix=@@HOST_DIR@@/usr
> +Sysroot=@@STAGING_DIR@@
> +Headers=/usr/include/qt5
> +Plugins=/usr/lib/qt/plugins
> +Examples=/usr/lib/qt/examples
> diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
> index 5fe8bb8..71dbd5d 100644
> --- a/package/qt5/qt5base/qt5base.mk
> +++ b/package/qt5/qt5base/qt5base.mk
> @@ -258,9 +258,17 @@ define QT5BASE_BUILD_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
>  endef
>  
> +# The file "qt.conf" can be used to override the hard-coded paths that are
> +# compiled into the Qt library. We need it to make "qmake" relocatable.
> +define QT5BASE_INSTALL_QT_CONF
> +	sed -e "s\\@@HOST_DIR@@\\$(HOST_DIR)\\" -e "s\\@@STAGING_DIR@@\\$(STAGING_DIR)\\" \

 \\ -> %

> +	$(QT5BASE_PKGDIR)/qt.conf.in > $(HOST_DIR)/usr/bin/qt.conf

 Give it one more indent to make it clear it's a continuation line.

> +endef
> +
>  define QT5BASE_INSTALL_STAGING_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install
>  	$(QT5_LA_PRL_FILES_FIXUP)
> +	$(QT5BASE_INSTALL_QT_CONF)

 It's pretty hacky to do stuff in host dir while installing to staging, but
that's how it is...


 Regards,
 Arnout

>  endef
>  
>  define QT5BASE_INSTALL_TARGET_LIBS
> 

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

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Arnout Vandecappelle]

 Hi Wolfgang,

 Again thank you very much for sustaining the work on this series!

On 30-06-17 10:36, Wolfgang Grandegger wrote:
> Hello,
> 
> this is v5 of my patch series to make the buildroot SDK (HOST_DIR)
> relocatable. It sanitizes the RPATH of all ELF files in the "target",
> "staging" and "host" tree using "patchelf --make-rpath-relative". I
> have started the mainlining process to implement "--make-rpath-relative"
> using GitHub pull request [1]... till now, no answer!
> 
> In contrast to v4, this series now does RPATH sanitation per package
> after package installation into the host, staging or target tree
> using GLOBAL_INSTRUMENTATION_HOOKS, similar to "check-bin-arch" or
> "check-host-rpath". This is a first hack to demonstrate feasibility.
> The scripts in "pkg-generic.mk" needs some more polish to be more
> efficient and elegant. See also "Changes since v4" below.

 Samuel and I reviewed the entire series now. It's getting closer but I expect a
v7 will still be needed even after you send v6. I've marked these as Changes
Requested in patchwork.


> Furthermore this patch creates the script "relocate-sdk.sh" in the top
> directory of the "host" tree allowing to relocate the SDK after it has
> been moved to a new location. It replaces the old path with the new
> one in all text files identified by "file --mime-type". The location
> is stored in "usr/share/buildroot/sdk-location".

 These final patches are pretty much ready, just minor things. Peter will
probably fix them up tomorrow and commit directly. So if you start working on
those, please give us a ping first (preferably on IRC - Peter is Jacmet).


> Unfortunately, "qmake" uses hard-coded pathes compiled into the QT5
> libraries. To overcome this problem, "qt5pase" now creates "qt.conf".

 There will be many more packages that need this kind of treatment, I expect.
Things will hopefully improve over time.

 Regards,
 Arnout


[snip]

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

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Wolfgang Grandegger]

Hello Arnout,

Am 04.07.2017 um 00:50 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
>   Again thank you very much for sustaining the work on this series!
> 
> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>> Hello,
>>
>> this is v5 of my patch series to make the buildroot SDK (HOST_DIR)
>> relocatable. It sanitizes the RPATH of all ELF files in the "target",
>> "staging" and "host" tree using "patchelf --make-rpath-relative". I
>> have started the mainlining process to implement "--make-rpath-relative"
>> using GitHub pull request [1]... till now, no answer!
>>
>> In contrast to v4, this series now does RPATH sanitation per package
>> after package installation into the host, staging or target tree
>> using GLOBAL_INSTRUMENTATION_HOOKS, similar to "check-bin-arch" or
>> "check-host-rpath". This is a first hack to demonstrate feasibility.
>> The scripts in "pkg-generic.mk" needs some more polish to be more
>> efficient and elegant. See also "Changes since v4" below.
> 
>   Samuel and I reviewed the entire series now. It's getting closer but I expect a
> v7 will still be needed even after you send v6. I've marked these as Changes
> Requested in patchwork.

Great. Just wait for my next series as it already contains a lot of 
changes (mainly to pkg-generic.mk and fix-rpath).

>> Furthermore this patch creates the script "relocate-sdk.sh" in the top
>> directory of the "host" tree allowing to relocate the SDK after it has
>> been moved to a new location. It replaces the old path with the new
>> one in all text files identified by "file --mime-type". The location
>> is stored in "usr/share/buildroot/sdk-location".
> 
>   These final patches are pretty much ready, just minor things. Peter will
> probably fix them up tomorrow and commit directly. So if you start working on
> those, please give us a ping first (preferably on IRC - Peter is Jacmet).
> 
> 
>> Unfortunately, "qmake" uses hard-coded pathes compiled into the QT5
>> libraries. To overcome this problem, "qt5pase" now creates "qt.conf".
> 
>   There will be many more packages that need this kind of treatment, I expect.
> Things will hopefully improve over time.

Yes, that's also my feeling.

Wolfgang.

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Arnout Vandecappelle]

 Hi Wolfgang,

On 04-07-17 09:17, Wolfgang Grandegger wrote:
> Hello Arnout,
> 
> Am 04.07.2017 um 00:50 schrieb Arnout Vandecappelle:
>>   Hi Wolfgang,
>>
>>   Again thank you very much for sustaining the work on this series!
>>
>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>> Hello,
>>>
>>> this is v5 of my patch series to make the buildroot SDK (HOST_DIR)
>>> relocatable. It sanitizes the RPATH of all ELF files in the "target",
>>> "staging" and "host" tree using "patchelf --make-rpath-relative". I
>>> have started the mainlining process to implement "--make-rpath-relative"
>>> using GitHub pull request [1]... till now, no answer!
>>>
>>> In contrast to v4, this series now does RPATH sanitation per package
>>> after package installation into the host, staging or target tree
>>> using GLOBAL_INSTRUMENTATION_HOOKS, similar to "check-bin-arch" or
>>> "check-host-rpath". This is a first hack to demonstrate feasibility.
>>> The scripts in "pkg-generic.mk" needs some more polish to be more
>>> efficient and elegant. See also "Changes since v4" below.
>>
>>   Samuel and I reviewed the entire series now. It's getting closer but I expect a
>> v7 will still be needed even after you send v6. I've marked these as Changes
>> Requested in patchwork.
> 
> Great. Just wait for my next series as it already contains a lot of changes
> (mainly to pkg-generic.mk and fix-rpath).

 I had a big discussion with the others here at the BR Summer Camp, and they
don't agree that the current approach is the best one :-(

 The argument as always is simplicity. Doing things in an instrumentation hook
is making it more complicated, and more difficult to understand what is going
on. It is cleaner and more in-line with how we currently do things to do the
sanitization at the end of the build.

 The idea is that there would be an explicit 'make sdk' target that builds and
sanitizes the host (and of course installs the relocation script). There is no
sanitization at all if you don't call this 'make sdk'. Indeed, we currently
don't have it and things work fine as long as you stay within your build tree.

 So basically it means reverting to v4 of the fix-rpath script. It would be
called three times:
- from target-finalize to sanitize the target;
- from sdk to sanitize the host;
- from sdk to sanitize the staging.

 Oh, also we decided that the sanitization for staging should be the same as for
target, i.e. make it relative to the staging root. Indeed, the
executables/libraries in there should never be executed, unless they are copied
to the target tree. When copied, their location relative to the staging root
should remain the same, so doing the same sanitization as for target should work.


 I understand that it's annoying to be sent in the wrong direction like this,
and I hope we are not discouraging you. If you already have some patches to
generate the file lists, feel free to post them anyway since they may be useful
for other purposes - but then preferably separate from this series.


 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

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Wolfgang Grandegger]

Hello Arnout,

Am 04.07.2017 um 13:22 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
> On 04-07-17 09:17, Wolfgang Grandegger wrote:
>> Hello Arnout,
>>
>> Am 04.07.2017 um 00:50 schrieb Arnout Vandecappelle:
>>>    Hi Wolfgang,
>>>
>>>    Again thank you very much for sustaining the work on this series!
>>>
>>> On 30-06-17 10:36, Wolfgang Grandegger wrote:
>>>> Hello,
>>>>
>>>> this is v5 of my patch series to make the buildroot SDK (HOST_DIR)
>>>> relocatable. It sanitizes the RPATH of all ELF files in the "target",
>>>> "staging" and "host" tree using "patchelf --make-rpath-relative". I
>>>> have started the mainlining process to implement "--make-rpath-relative"
>>>> using GitHub pull request [1]... till now, no answer!
>>>>
>>>> In contrast to v4, this series now does RPATH sanitation per package
>>>> after package installation into the host, staging or target tree
>>>> using GLOBAL_INSTRUMENTATION_HOOKS, similar to "check-bin-arch" or
>>>> "check-host-rpath". This is a first hack to demonstrate feasibility.
>>>> The scripts in "pkg-generic.mk" needs some more polish to be more
>>>> efficient and elegant. See also "Changes since v4" below.
>>>
>>>    Samuel and I reviewed the entire series now. It's getting closer but I expect a
>>> v7 will still be needed even after you send v6. I've marked these as Changes
>>> Requested in patchwork.
>>
>> Great. Just wait for my next series as it already contains a lot of changes
>> (mainly to pkg-generic.mk and fix-rpath).
> 
>   I had a big discussion with the others here at the BR Summer Camp, and they
> don't agree that the current approach is the best one :-(
> 
>   The argument as always is simplicity. Doing things in an instrumentation hook
> is making it more complicated, and more difficult to understand what is going
> on. It is cleaner and more in-line with how we currently do things to do the
> sanitization at the end of the build.
> 
>   The idea is that there would be an explicit 'make sdk' target that builds and
> sanitizes the host (and of course installs the relocation script). There is no
> sanitization at all if you don't call this 'make sdk'. Indeed, we currently
> don't have it and things work fine as long as you stay within your build tree.

"make sdk", sounds good.

>   So basically it means reverting to v4 of the fix-rpath script. It would be
> called three times:
> - from target-finalize to sanitize the target;
> - from sdk to sanitize the host;
> - from sdk to sanitize the staging.
> 
>   Oh, also we decided that the sanitization for staging should be the same as for
> target, i.e. make it relative to the staging root. Indeed, the
> executables/libraries in there should never be executed, unless they are copied
> to the target tree. When copied, their location relative to the staging root
> should remain the same, so doing the same sanitization as for target should work.

OK, you mean not using $ORIGIN.

>   I understand that it's annoying to be sent in the wrong direction like this,
> and I hope we are not discouraging you. If you already have some patches to
> generate the file lists, feel free to post them anyway since they may be useful
> for other purposes - but then preferably separate from this series.

Well, after trying a few more things along the per package and step 
approach, I came to the same conclusion. It got more and more complex 
and slow (see my other mail).

Wolfgang.

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Thomas Petazzoni]

Hello,

On Fri, 30 Jun 2017 10:36:57 +0200, Wolfgang Grandegger wrote:

>   support/scripts: add reloacte-sdk.sh script for SDK relocation

Applied to master, after doing the fixes suggested by Arnout.

>   package/qt5base: provide "qt.conf" to make "qmake" relocatable

Applied to master, after doing the fixes suggested by Arnout.

All your other patches have been marked as "Changes Requested", as they
got some more fundamental comments.

If you have the chance to respin the series today/tomorrow, it'd be
great, as we are all together here and therefore able to quickly
discuss/merge the series.

Thanks a lot for this work, much appreciated!

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

[Buildroot] [RFC PATCH v5 00/11] Make the SDK relocatable

[Wolfgang Grandegger]

Hello Thomas,

Am 04.07.2017 um 18:07 schrieb Thomas Petazzoni:
> Hello,
> 
> On Fri, 30 Jun 2017 10:36:57 +0200, Wolfgang Grandegger wrote:
> 
>>    support/scripts: add reloacte-sdk.sh script for SDK relocation
> 
> Applied to master, after doing the fixes suggested by Arnout.
> 
>>    package/qt5base: provide "qt.conf" to make "qmake" relocatable
> 
> Applied to master, after doing the fixes suggested by Arnout.
> 
> All your other patches have been marked as "Changes Requested", as they
> got some more fundamental comments.
> 
> If you have the chance to respin the series today/tomorrow, it'd be
> great, as we are all together here and therefore able to quickly
> discuss/merge the series.

I have just sent out the v6 series... still including the patches above. 
Hope I have not missed too much pending issues.

> Thanks a lot for this work, much appreciated!

You are welcome! Happy hacking!

Wolfgang.