Make the SDK relocatable
v7 cover Make the SDK relocatable
Hello,
this is v7 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". We now use patchelf v0.9 to still support old Debian and RHEL systems.
v5 did RPATH sanitization per package after package installation into the host, staging or target tree using GLOBAL_INSTRUMENTATION_HOOKS. This approach got more and more complex and inefficient. Therefore it was abandoned in favor of global sanititation at the end of the host, staging and target build (see changes since v5).
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 "share/buildroot/sdk-location". This patch is already upstream.
Unfortunately, "qmake" uses hard-coded pathes compiled into the QT5 libraries. To overcome this problem, "qt5pase" now creates "qt.conf".
Hope I have not too much pending issues.
Wolfgang.
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 v6:
- patchelf patches: update patch header with upstream commit, etc.
- ${HOST_DIR}/usr is gone which required various fixes
- Proper qt.conf.in added
Changes since v5:
- switch back to v4
- patchelf patches are now based on v0.9
- patchelf now calculates minimal relative path to the ELF file (and not to the rootdir as before).
- patchelf: neededLibFound is now passed to libFoundInRPath
- Makefile: the "host-finalize" target has been added for SDK relocation
- fix-rpath now uses variables to define the list of dirs to be pruned
- fix-rpath: the staging tree is sanitized like the target tree
- various minor fixes (typos)
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.
Samuel Martin (1):
support/scripts: add fix-rpath script to sanitize the rpath
Wolfgang Grandegger (8):
package/patchelf: add patch for rpath sanitization under a root
directory
core: sanitize RPATH in staging tree at the end of target finalization
core: sanitize RPATH in target tree at the end of target finalization
core: sanitize RPATH in host tree at the very end of the build
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: relocate-sdk.sh now creates sdk-location in
share/buildroot
package/qt5base: provide "qt.conf" to make "qmake" relocatable
Makefile | 17 +- ...move-apparently-incorrect-usage-of-static.patch | 52 +++ ...unction-for-splitting-a-colon-separated-s.patch | 63 +++ ...to-make-the-rpath-relative-under-a-specif.patch | 423 +++++++++++++++++++++ package/qt5/qt5base/qt.conf.in | 19 + package/qt5/qt5base/qt5base.mk | 8 + support/misc/relocate-sdk.sh | 2 +- support/scripts/fix-rpath | 122 ++++++ toolchain/helpers.mk | 18 + .../toolchain-external/pkg-toolchain-external.mk | 1 + 10 files changed, 723 insertions(+), 2 deletions(-) create mode 100644 package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch create mode 100644 package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch create mode 100644 package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch create mode 100644 package/qt5/qt5base/qt.conf.in create mode 100755 support/scripts/fix-rpath
Signature · 1 line
2.7.4
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
The patch allows to use patchelf to sanitize the rpath of the buildroot libraries and binaries using the option "--make-rpath-relative <rootdir>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
--- ...move-apparently-incorrect-usage-of-static.patch | 52 +++ ...unction-for-splitting-a-colon-separated-s.patch | 63 +++ ...to-make-the-rpath-relative-under-a-specif.patch | 423 +++++++++++++++++++++ 3 files changed, 538 insertions(+) create mode 100644 package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch create mode 100644 package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch create mode 100644 package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +52 -0
@@ -0,0 +1,52 @@ +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra <[email protected]> +Date: Mon, 19 Sep 2016 17:31:37 +0200 +Subject: [PATCH] Remove apparently incorrect usage of "static" + +[Upstream-commit: https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e] +Signed-off-by: Wolfgang Grandegger <[email protected]> + +--- + src/patchelf.cc | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +Index: patchelf-0.9.old/src/patchelf.cc +=================================================================== +--- patchelf-0.9.old.orig/src/patchelf.cc 2017-07-03 09:06:12.292069400 +0200 ++++ patchelf-0.9.old/src/patchelf.cc 2017-07-03 09:20:57.000000000 +0200 +@@ -941,7 +941,6 @@ + assert(strTabAddr == rdi(shdrDynStr.sh_addr)); + + /* Walk through the dynamic section, look for the DT_SONAME entry. */ +- static vector<string> neededLibs; + dyn = (Elf_Dyn *) (contents + rdi(shdrDynamic.sh_offset)); + Elf_Dyn * dynSoname = 0; + char * soname = 0; +@@ -949,8 +948,7 @@ + if (rdi(dyn->d_tag) == DT_SONAME) { + dynSoname = dyn; + soname = strTab + rdi(dyn->d_un.d_val); +- } else if (rdi(dyn->d_tag) == DT_INIT) +- neededLibs.push_back(string(strTab + rdi(dyn->d_un.d_val))); ++ } + } + + if (op == printSoname) { +@@ -1058,7 +1056,7 @@ + unless you use its `--enable-new-dtag' option, in which case it + generates a DT_RPATH and DT_RUNPATH pointing at the same + string. */ +- static vector<string> neededLibs; ++ vector<string> neededLibs; + dyn = (Elf_Dyn *) (contents + rdi(shdrDynamic.sh_offset)); + Elf_Dyn * dynRPath = 0, * dynRunPath = 0; + char * rpath = 0; +@@ -1091,7 +1089,7 @@ + /* For each directory in the RPATH, check if it contains any + needed library. */ + if (op == rpShrink) { +- static vector<bool> neededLibFound(neededLibs.size(), false); ++ vector<bool> neededLibFound(neededLibs.size(), false); + + newRPath = ""; +
package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch new file +63 -0
63 lines not on this page · raw message
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch new file +423 -0
423 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
Hi Wolfgang,
Quoted 6 lines · 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>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
Signed-off-by Wolfgang Grandegger <[email protected]>
I still have a bunch of comments, but they're solidly in the nitpicking
category. We definitely want this series (or at least part of it) in 2017.08-rc1, so if you don't respin in time, it will be applied. In that case, however, feel free to fix my nitpicks in follow-up patches. That said:
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 4
Quoted 10 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +4
-0@@ -0,0 +1,52 @@ +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra <[email protected]> +Date: Mon, 19 Sep 2016 17:31:37 +0200 +Subject: [PATCH] Remove apparently incorrect usage of "static"
I was going to say: we don't really need this patch. However, we do need it
because it removes the DT_INIT symbols from needed_libs (DT_INIT points to library initialisation function, not to needed libraries...). So perhaps that bit should be added to the patch message.
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 13
Quoted 9 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +9
-0+ +[Upstream-commit: https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e] +Signed-off-by: Wolfgang Grandegger <[email protected]> + +--- + src/patchelf.cc | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +Index: patchelf-0.9.old/src/patchelf.cc
Looks like you didn't really generate this with git-format-patch, although the
header looks like it... It's not very important, but we really like to be able to regenerate exactly the same patches with the following procedure:
cd patchelf
git checkout -b buildroot 0.9
git am ../buildroot/package/patchelf/*.patch
git format-patch -N --no-renames -o ../buildroot/package/patchelf 0.9..
[snip]
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch line 5
Quoted 11 lines
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch new file +5
-0@@ -0,0 +1,423 @@ +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001 +From: Wolfgang Grandegger <[email protected]> +Date: Mon, 20 Feb 2017 16:29:24 +0100 +Subject: [PATCH] Add option to make the rpath relative under a specified root + directory
I'm not yet 100% satisfied with this patch, because it combines too many things
in a single bunch. However, improving it really requires a lot of additional refactoring in patchelf. And patchelf test coverage is exactly great so refactoring is tricky. And we anyway can't use the latest patchelf version. So I guess this Buildroot-specific patch is good enough.
[snip]
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch line 289
Quoted 6 lines
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch new file +6
-0+@@ -1261,35 +1396,35 @@ + void ElfFile<ElfFileParamNames>::replaceNeeded(map<string, string>& libs) + { + if (libs.empty()) return; +- ++
The patch really shouldn't contain these whitespace fixes... (Lots more below).
Regards,
Arnout
[snip]
Signature · 6 lines
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
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
Hello Arnout,
Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
Quoted 8 lines
Hi Wolfgang,
Quoted 5 lines · 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>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
Quoted 6 lines
Quoted 1 line
Signed-off-by Wolfgang Grandegger <[email protected]>I still have a bunch of comments, but they're solidly in the nitpicking
category. We definitely want this series (or at least part of it) in 2017.08-rc1, so if you don't respin in time, it will be applied. In that case, however, feel free to fix my nitpicks in follow-up patches. That said:
OK. I will concentrate on the important (and trivial) issues.
Quoted 1 line
Should I also add your "Acked-by" to the patchelf patches itself?
Quoted 7 lines
Quoted 7 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +1
-0@@ -0,0 +1,52 @@ +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
Quoted 8 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 4
Quoted 3 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +3
-0+From: Eelco Dolstra <[email protected]> +Date: Mon, 19 Sep 2016 17:31:37 +0200 +Subject: [PATCH] Remove apparently incorrect usage of "static"I was going to say: we don't really need this patch. However, we do need it
because it removes theDT_INITsymbols fromneeded_libs(DT_INITpoints to library initialisation function, not to needed libraries...). So perhaps that bit should be added to the patch message.
Added to the [] comment. I think the original messages should bot be touched.
Quoted 14 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 13
Quoted 9 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +9
-0+ +[Upstream-commit: https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e] +Signed-off-by: Wolfgang Grandegger <[email protected]> + +--- + src/patchelf.cc | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +Index: patchelf-0.9.old/src/patchelf.ccLooks like you didn't really generate this with git-format-patch, although the
header looks like it... It's not very important, but we really like to be able to regenerate exactly the same patches with the following procedure:
Quoted 4 lines
cd patchelf
git checkout -b buildroot 0.9
git am ../buildroot/package/patchelf/*.patch
git format-patch -N --no-renames -o ../buildroot/package/patchelf 0.9..
This is generate with quilt against patchelf-0.9.tag.bz2. Will fix.
Quoted 20 lines
[snip]
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch line 5
Quoted 11 lines
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch new file +5
-0@@ -0,0 +1,423 @@ +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001 +From: Wolfgang Grandegger <[email protected]> +Date: Mon, 20 Feb 2017 16:29:24 +0100 +Subject: [PATCH] Add option to make the rpath relative under a specified root + directoryI'm not yet 100% satisfied with this patch, because it combines too many things
in a single bunch. However, improving it really requires a lot of additional refactoring in patchelf. And patchelf test coverage is exactly great so refactoring is tricky. And we anyway can't use the latest patchelf version. So I guess this Buildroot-specific patch is good enough.[snip]
Quoted 8 lines
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch line 289
Quoted 6 lines
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch new file +6
-0+@@ -1261,35 +1396,35 @@ + void ElfFile<ElfFileParamNames>::replaceNeeded(map<string, string>& libs) + { + if (libs.empty()) return; +- ++The patch really shouldn't contain these whitespace fixes... (Lots more below).
Ah, obviously I did a remove-trailing-white-space. I have reverted them.
More soon...
Wolfgang.
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
Quoted 18 lines · Wolfgang Grandegger
Hello Arnout,
Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
Quoted 8 lines
Hi Wolfgang,
Quoted 5 lines · 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>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
Quoted 6 lines
Quoted 1 line
Signed-off-by Wolfgang Grandegger <[email protected]>I still have a bunch of comments, but they're solidly in the nitpicking
category. We definitely want this series (or at least part of it) in 2017.08-rc1, so if you don't respin in time, it will be applied. In that case, however, feel free to fix my nitpicks in follow-up patches. That said:
Quoted 5 lines
OK. I will concentrate on the important (and trivial) issues.
Quoted 1 line
Should I also add your "Acked-by" to the patchelf patches itself?
No, not to the patchelf patches. That doesn't make sense, since it's
meaningless for upstream, and for Buildroot the ack is already in the Buildroot patch.
The Sob needs to be there because its meaning is different for the Buildroot
patch and for the package patch. For the package patch, it means "I vouch that it's OK to distribute this change under the license of the package", and the license of the package is different from the license of Buildroot.
Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
diff --git
a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patchb/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patchnew file mode 100644
index 0000000..eda32e8package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +1
-0@@ -0,0 +1,52 @@ +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001Quoted 8 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 4
Quoted 3 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +3
-0+From: Eelco Dolstra <[email protected]> +Date: Mon, 19 Sep 2016 17:31:37 +0200 +Subject: [PATCH] Remove apparently incorrect usage of "static"I was going to say: we don't really need this patch. However, we do need it
because it removes theDT_INITsymbols fromneeded_libs(DT_INITpoints to library initialisation function, not to needed libraries...). So perhaps that bit should be added to the patch message.
Quoted 1 line
Added to the [] comment. I think the original messages should bot be touched.
ACK that.
Quoted 17 lines
Quoted 17 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 13
Quoted 12 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +2
-0+ +[Upstream-commit:https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +7
-0+Signed-off-by: Wolfgang Grandegger <[email protected]> + +--- + src/patchelf.cc | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +Index: patchelf-0.9.old/src/patchelf.ccLooks like you didn't really generate this with git-format-patch, although the
header looks like it... It's not very important, but we really like to be able to regenerate exactly the same patches with the following procedure:
Quoted 6 lines
Quoted 4 lines
cd patchelf
git checkout -b buildroot 0.9
git am ../buildroot/package/patchelf/*.patch
git format-patch -N --no-renames -o ../buildroot/package/patchelf 0.9..This is generate with quilt against patchelf-0.9.tag.bz2. Will fix.
Ah, that makes sense as well. Again, it's not very important. Do you find using
quilt to manage package patches easier than using git?
Regards,
Arnout
[snip]
Signature · 6 lines
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
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
Am 20.07.2017 um 09:55 schrieb Arnout Vandecappelle:
Quoted 27 lines · Wolfgang Grandegger
Quoted 18 lines
Hello Arnout,
Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
Quoted 8 lines
Hi Wolfgang,
Quoted 5 lines · 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>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
Quoted 6 lines
Quoted 1 line
Signed-off-by Wolfgang Grandegger <[email protected]>I still have a bunch of comments, but they're solidly in the nitpicking
category. We definitely want this series (or at least part of it) in 2017.08-rc1, so if you don't respin in time, it will be applied. In that case, however, feel free to fix my nitpicks in follow-up patches. That said:Quoted 5 lines
OK. I will concentrate on the important (and trivial) issues.
Quoted 1 line
Should I also add your "Acked-by" to the patchelf patches itself?
Quoted 8 lines
No, not to the patchelf patches. That doesn't make sense, since it's
meaningless for upstream, and for Buildroot the ack is already in the Buildroot patch.The Sob needs to be there because its meaning is different for the Buildroot
patch and for the package patch. For the package patch, it means "I vouch that it's OK to distribute this change under the license of the package", and the license of the package is different from the license of Buildroot.
OK, makes sense.
Quoted 45 lines
Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
diff --git
a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patchb/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patchnew file mode 100644
index 0000000..eda32e8package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +1
-0@@ -0,0 +1,52 @@ +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001Quoted 8 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 4
Quoted 3 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +3
-0+From: Eelco Dolstra <[email protected]> +Date: Mon, 19 Sep 2016 17:31:37 +0200 +Subject: [PATCH] Remove apparently incorrect usage of "static"I was going to say: we don't really need this patch. However, we do need it
because it removes theDT_INITsymbols fromneeded_libs(DT_INITpoints to library initialisation function, not to needed libraries...). So perhaps that bit should be added to the patch message.Quoted 1 line
Added to the [] comment. I think the original messages should bot be touched.
ACK that.
Quoted 22 lines
Quoted 21 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch line 13
Quoted 12 lines
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +2
-0+ +[Upstream-commit:https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch new file +7
-0+Signed-off-by: Wolfgang Grandegger <[email protected]> + +--- + src/patchelf.cc | 8 +++----- + 1 file changed, 3 insertions(+), 5 deletions(-) + +Index: patchelf-0.9.old/src/patchelf.ccLooks like you didn't really generate this with git-format-patch, although the
header looks like it... It's not very important, but we really like to be able to regenerate exactly the same patches with the following procedure:cd patchelf
git checkout -b buildroot 0.9
git am ../buildroot/package/patchelf/*.patch
git format-patch -N --no-renames -o ../buildroot/package/patchelf 0.9..
Quoted 4 lines
Quoted 1 line
This is generate with quilt against patchelf-0.9.tag.bz2. Will fix.
Ah, that makes sense as well. Again, it's not very important. Do you find using
quilt to manage package patches easier than using git?
Well, with both, "git rebase -i" and "quilt" you have to be careful not mixing up things. If I don't have git, I use quilt. I have now switched to git to maintain the patchelf patches.
Wolfgang.
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
In reply to Wolfgang GrandeggerQuoted 4 lines · 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>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
One more thing (but that can be a completely separate follow-up patch).
Currently, in most cases, we clear the rpath completely. But patchelf also has
functionality to remove the RUNPATH entry in the dynamic section. Although this doesn't really make the file smaller (the space for the original string is kept, the old string is just overwritten with X's), removing the RUNPATH entry could speed up dynamic loading slightly. It's also just weird to see these empty RUNPATH strings when you do a readelf.
The code to do that is already there, in the rpRemove section. It's pretty
simple to factor it out.
Regards,
Arnout
Signature · 6 lines
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
PATCH v7 1/9 package/patchelf: add patch for rpath sanitization under a root directory
Am 20.07.2017 um 01:17 schrieb Arnout Vandecappelle:
Quoted 17 lines · Wolfgang Grandegger
Quoted 4 lines
The patch allows to use patchelf to sanitize the rpath of the buildroot libraries and binaries using the option "--make-rpath-relative <rootdir>". Recent versions of patchelf will not built on old Debian and RHEL systems due to C++11 constructs. Therefore we stick with v0.9 for the time being.
One more thing (but that can be a completely separate follow-up patch).
Currently, in most cases, we clear the rpath completely. But patchelf also has
functionality to remove the RUNPATH entry in the dynamic section. Although this doesn't really make the file smaller (the space for the original string is kept, the old string is just overwritten with X's), removing the RUNPATH entry could speed up dynamic loading slightly. It's also just weird to see these empty RUNPATH strings when you do a readelf.
Quoted 2 lines
The code to do that is already there, in the rpRemove section. It's pretty
simple to factor it out.
OK, will take care later.
Wolfgang.
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
In reply to Wolfgang GrandeggerFrom: Samuel Martin <[email protected]>
This commit introduces the script "fix-rpath" able to scan a tree, detect 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>".
--- support/scripts/fix-rpath | 122 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100755 support/scripts/fix-rpath
support/scripts/fix-rpath new file +122 -0
@@ -0,0 +1,122 @@ +#!/usr/bin/env bash + +# Copyright (C) 2016 Samuel Martin <[email protected]> +# Copyright (C) 2017 Wolfgang Grandegger <[email protected]> +# +# 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 + +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 + +Environment: + + PATCHELF patchelf program to use + (default: HOST_DIR/bin/patchelf) +EOF +} + +: ${PATCHELF:=${HOST_DIR}/bin/patchelf} + +# ELF files should not be in these sub-directories +HOST_EXCLUDEPATHS="/share/terminfo" +STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" + +main() { + local rootdir + local tree="${1}" + local find_args=( ) + local sanitize_extra_args=( ) + + case "${tree}" in + host) + rootdir="${HOST_DIR}" + + # do not process the sysroot (only contains target binaries) + find_args+=( "-path" "${STAGING_DIR}" "-prune" "-o" ) + + # do not process the external toolchain installation directory to + # avoid breaking it. + test "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" != "" && \ + find_args+=( "-path" "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" "-prune" "-o" ) + + for excludepath in ${HOST_EXCLUDEPATHS}; do + find_args+=( "-path" "${HOST_DIR}""${excludepath}" "-prune" "-o" ); + done + + # do not process the patchelf binary but a copy to work-around "file in use" + find_args+=( "-path" "${PATCHELF}" "-prune" "-o" ) + cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" + + sanitize_extra_args+=( "--relative-to-file" ) + ;; + + staging) + rootdir="${STAGING_DIR}" + + # ELF files should not be in these sub-directories + for excludepath in ${STAGING_EXCLUDEPATHS}; do + find_args+=( "-path" "${STAGING_DIR}""${excludepath}" "-prune" "-o" ) + done + + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" ) + ;; + + target) + rootdir="${TARGET_DIR}" + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" ) + ;; + + *) + usage + exit 1 + ;; + esac + + find_args+=( "-type" "f" "-print" ) + + while read file ; do + # check if it's an ELF file + if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" + fi + done < <(find "${rootdir}" ${find_args[@]}) + + # Restore patched patchelf utility + test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}" + + # ignore errors + return 0 +} + +main ${@}
Signature · 1 line
2.7.4
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Quoted 9 lines · Wolfgang Grandegger
From: Samuel Martin <[email protected]>
This commit introduces the script "fix-rpath" able to scan a tree, detect 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 <[email protected]>Signed-off-by Wolfgang Grandegger <[email protected]>
Some small comments below, but again, can be fixed in follow-up patches, so
[snip]
support/scripts/fix-rpath line 26
Quoted 3 lines
support/scripts/fix-rpath new file +3
-0+Description: + + This script scans a tree and sanitize ELF files' RPATH found in there.
^s
support/scripts/fix-rpath line 30
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+ + 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".
^t
support/scripts/fix-rpath line 40
Quoted 10 lines
support/scripts/fix-rpath new file +10
-0+ +Arguments: + + TREE_KIND Kind of tree to be processed. + Allowed values: host, target, staging + +Environment: + + PATCHELF patchelf program to use + (default: HOST_DIR/bin/patchelf)
And also HOST_DIR, TARGET_DIR, STAGING_DIR, andTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.
[snip]
support/scripts/fix-rpath line 66
Quoted 11 lines
support/scripts/fix-rpath new file +11
-0+ case "${tree}" in + host) + rootdir="${HOST_DIR}" + + # do not process the sysroot (only contains target binaries) + find_args+=( "-path" "${STAGING_DIR}" "-prune" "-o" ) + + # do not process the external toolchain installation directory to + # avoid breaking it. + test "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" != "" && \ + find_args+=( "-path" "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" "-prune" "-o" )
Since this variable isn't exported, it doesn't actually end up being used. But
I think the solution is to pass it explicitly in patch 5.
support/scripts/fix-rpath line 70
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+ + for excludepath in ${HOST_EXCLUDEPATHS}; do + find_args+=( "-path" "${HOST_DIR}""${excludepath}" "-prune" "-o" ); + done
Perhaps this could be refactored into defining here:
excludepaths="${HOST_EXCLUDEPATHS}"and moving the loop to the generic part. It would anyway be nicer to have TARGET_EXCLUDEPATHS, for symmetry.
Quoted 12 lines
support/scripts/fix-rpath new file +12
-0+ + # do not process the patchelf binary but a copy to work-around "file in use" + find_args+=( "-path" "${PATCHELF}" "-prune" "-o" ) + cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" + + sanitize_extra_args+=( "--relative-to-file" ) + ;; + + staging) + rootdir="${STAGING_DIR}" + + # ELF files should not be in these sub-directories
support/scripts/fix-rpath line 92
Quoted 10 lines
support/scripts/fix-rpath new file +10
-0+ for excludepath in ${STAGING_EXCLUDEPATHS}; do + find_args+=( "-path" "${STAGING_DIR}""${excludepath}" "-prune" "-o" ) + done + + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" ) + ;; + + target) + rootdir="${TARGET_DIR}" + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )
I thought we decided that target and staging should NOT be relative-to-file,
but should be absolute paths without rootdir? Or was this a problem for staging, because the linker would add the RUNPATH to the library search path and this would point to the system libs instead of target libs? According to the man page this should only be the case for a native linker, so not for a cross linker.
I don't really like having $ORIGIN-based rpaths in target, but it's no big deal.
Regardless, the reasoning behind the choice we make should be documented here
in comments. So for host, add something to the effect that we always want $ORIGIN-based rpaths to make it relocatable. For staging, say that we want it to be the same as target. And for target, explain the choice you made.
support/scripts/fix-rpath line 104
Quoted 2 lines
support/scripts/fix-rpath new file +2
-0+ while read file ; do + # check if it's an ELF file
Indentation is inconsistent: sometimes 4 spaces, sometimes 2 spaces (inside the
case, and in the condition below). And here, you're switching from spaces to tabs!
support/scripts/fix-rpath line 105
Quoted 1 line
support/scripts/fix-rpath new file +1
-0+ if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then
It might be good to check if the file actually has a non-empty rpath. When
processing a second time, most rpaths are empty so they can be skipped.
Maybe even add another patch to patchelf to exit early and quietly with just 0
or 1 depending on rpath presence.
Regards,
Arnout
[snip]
Signature · 6 lines
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
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
Quoted 28 lines · Wolfgang Grandegger
Quoted 9 lines
From: Samuel Martin <[email protected]>
This commit introduces the script "fix-rpath" able to scan a tree, detect 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 <[email protected]>Signed-off-by Wolfgang Grandegger <[email protected]>Some small comments below, but again, can be fixed in follow-up patches, so
[snip]
support/scripts/fix-rpath line 26
Quoted 3 lines
support/scripts/fix-rpath new file +3
-0+Description: + + This script scans a tree and sanitize ELF files' RPATH found in there.^ssupport/scripts/fix-rpath line 30
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+ + 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".^t
Quoted 13 lines
support/scripts/fix-rpath line 40
Quoted 10 lines
support/scripts/fix-rpath new file +10
-0+ +Arguments: + + TREE_KIND Kind of tree to be processed. + Allowed values: host, target, staging + +Environment: + + PATCHELF patchelf program to use + (default: HOST_DIR/bin/patchelf)And also
HOST_DIR,TARGET_DIR,STAGING_DIR, andTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.
PATCHELF points to the patchelf utility! Or what do you mean here?
Quoted 23 lines
[snip]
support/scripts/fix-rpath line 66
Quoted 11 lines
support/scripts/fix-rpath new file +11
-0+ case "${tree}" in + host) + rootdir="${HOST_DIR}" + + # do not process the sysroot (only contains target binaries) + find_args+=( "-path" "${STAGING_DIR}" "-prune" "-o" ) + + # do not process the external toolchain installation directory to + # avoid breaking it. + test "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" != "" && \ + find_args+=( "-path" "${TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR}" "-prune" "-o" )Since this variable isn't exported, it doesn't actually end up being used. But
I think the solution is to pass it explicitly in patch 5.support/scripts/fix-rpath line 70
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+ + for excludepath in ${HOST_EXCLUDEPATHS}; do + find_args+=( "-path" "${HOST_DIR}""${excludepath}" "-prune" "-o" ); + donePerhaps this could be refactored into defining here:
Quoted 3 lines
excludepaths="${HOST_EXCLUDEPATHS}"and moving the loop to the generic part. It would anyway be nicer to have
TARGET_EXCLUDEPATHS, for symmetry.
Will improve that later.
Quoted 21 lines
Quoted 21 lines
support/scripts/fix-rpath new file +20
-0+ + # do not process the patchelf binary but a copy to work-around "file in use" + find_args+=( "-path" "${PATCHELF}" "-prune" "-o" ) + cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" + + sanitize_extra_args+=( "--relative-to-file" ) + ;; + + staging) + rootdir="${STAGING_DIR}" + + # ELF files should not be in these sub-directories + for excludepath in ${STAGING_EXCLUDEPATHS}; do + find_args+=( "-path" "${STAGING_DIR}""${excludepath}" "-prune" "-o" ) + done + + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" ) + ;; + + target)
Quoted 8 lines
support/scripts/fix-rpath line 92
Quoted 2 lines
support/scripts/fix-rpath new file +2
-0+ rootdir="${TARGET_DIR}" + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )I thought we decided that target and staging should NOT be relative-to-file,
but should be absolute paths without rootdir? Or was this a problem for staging, because the linker would add the RUNPATH to the library search path and this would point to the system libs instead of target libs? According to the man page this should only be the case for a native linker, so not for a cross linker.
Grrr, that's wrong, cut an paste error :(. Fixed!
Quoted 8 lines
I don't really like having $ORIGIN-based rpaths in target, but it's no big deal.
Regardless, the reasoning behind the choice we make should be documented here
in comments. So for host, add something to the effect that we always want $ORIGIN-based rpaths to make it relocatable. For staging, say that we want it to be the same as target. And for target, explain the choice you made.
Quoted 4 lines
support/scripts/fix-rpath line 104
Quoted 2 lines
support/scripts/fix-rpath new file +2
-0+ while read file ; do + # check if it's an ELF fileIndentation is inconsistent: sometimes 4 spaces, sometimes 2 spaces (inside the
That's due to 8 spaces replaced by one tab.
Quoted 1 line
case, and in the condition below). And here, you're switching from spaces to tabs!
I use the default indention from emacs "Shell-script[bash]" which is usually not a bad choice. I'm going to replace all tabs with spaces.
Quoted 4 lines
support/scripts/fix-rpath line 105
Quoted 1 line
support/scripts/fix-rpath new file +1
-0+ if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; thenIt might be good to check if the file actually has a non-empty rpath. When
processing a second time, most rpaths are empty so they can be skipped.
I already tried that. It will make the check slower, meaning it costs more that calling patchelf a second time.
Quoted 2 lines
Maybe even add another patch to patchelf to exit early and quietly with just 0
or 1 depending on rpath presence.
Optimization of the execution time needs more careful analysis. I have a patchelf patch which just open and reads the patchelf header to check quickly if it's an ELF file. But that does not help a lot. readelf is still faster and I guess that's because it's written in C (and not C++).
Wolfgang.
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Quoted 4 lines · Wolfgang Grandegger
Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
Quoted 3 lines · Wolfgang Grandegger
[snip]
Quoted 9 lines
Quoted 7 lines
support/scripts/fix-rpath line 40
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+Environment: + + PATCHELF patchelf program to use + (default: HOST_DIR/bin/patchelf)And also
HOST_DIR,TARGET_DIR,STAGING_DIR, andTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.PATCHELF points to the patchelf utility! Or what do you mean here?
I mean, also these environment variables are used by the script. Moreover, the_DIR variables are not optional.
[snip]
Quoted 11 lines
Quoted 9 lines
support/scripts/fix-rpath line 92
Quoted 3 lines
support/scripts/fix-rpath new file +3
-0+ target) + rootdir="${TARGET_DIR}" + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )I thought we decided that target and staging should NOT be relative-to-file,
but should be absolute paths without rootdir? Or was this a problem for staging, because the linker would add the RUNPATH to the library search path and this would point to the system libs instead of target libs? According to the man page this should only be the case for a native linker, so not for a cross linker.Grrr, that's wrong, cut an paste error :(. Fixed!
But staging stays the same as target, right?
And did you add comments everywhere to explain why these options are chosen?
Quoted 10 lines
Quoted 10 lines
I don't really like having $ORIGIN-based rpaths in target, but it's no big
deal.Regardless, the reasoning behind the choice we make should be documented here
in comments. So for host, add something to the effect that we always want $ORIGIN-based rpaths to make it relocatable. For staging, say that we want it to be the same as target. And for target, explain the choice you made.
Quoted 6 lines
Quoted 4 lines
support/scripts/fix-rpath line 104
Quoted 2 lines
support/scripts/fix-rpath new file +2
-0+ while read file ; do + # check if it's an ELF fileIndentation is inconsistent: sometimes 4 spaces, sometimes 2 spaces (inside the
That's due to 8 spaces replaced by one tab.
Fix your editor not to do that.
Quoted 6 lines
Quoted 3 lines
case, and in the condition below). And here, you're switching from spaces to tabs!
I use the default indention from emacs "Shell-script[bash]" which is usually not a bad choice. I'm going to replace all tabs with spaces.
Since emacs, vim and kate all have compatible mode-settings lines, feel free to
add a mode-setting line below the shebang of the script. Do test it with vim please.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
+ if ${PATCHELF} --print-rpath "${file}" >
/dev/null2>&1; thenIt might be good to check if the file actually has a non-empty rpath. When
processing a second time, most rpaths are empty so they can be skipped.I already tried that. It will make the check slower, meaning it costs more that calling patchelf a second time.
Quoted 7 lines
Quoted 2 lines
Maybe even add another patch to patchelf to exit early and quietly with just 0
or 1 depending on rpath presence.Optimization of the execution time needs more careful analysis. I have a patchelf patch which just open and reads the patchelf header to check quickly if it's an ELF file. But that does not help a lot. readelf is still faster and I guess that's because it's written in C (and not C++).
I have a few speedup suggestions for patchelf (but all should be in follow-up
patches, not as part of patch 1/9).
- Add a --has-rpath option that is quiet and just exits 0 or 1.
- Instead of reading into a string, mmap the file. Since in most cases the file size isn't changed, that works *very* efficiently. Note that this needs a configure.ac check (mmap is not always available, e.g. not on nommu). With mmap, it's not needed to first read the header only, since it will just pull in a page when needed. It will be slower on NFS and some FUSE filesystems though.
- Remove empty RUNPATH entries, so a second run doesn't even start.
- Link patchelf statically, preferably with -flto as well. This will probably just speed things up with a few %, but it's also on the "empty" calls so could be significant.
Regards,
Arnout
Signature · 6 lines
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
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Am 20.07.2017 um 10:15 schrieb Arnout Vandecappelle:
Quoted 16 lines · Wolfgang Grandegger
Quoted 4 lines
Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
Quoted 3 lines · Wolfgang Grandegger
[snip]
Quoted 8 lines
Quoted 7 lines
support/scripts/fix-rpath line 40
Quoted 4 lines
support/scripts/fix-rpath new file +4
-0+Environment: + + PATCHELF patchelf program to use + (default: HOST_DIR/bin/patchelf)And also
HOST_DIR,TARGET_DIR,STAGING_DIR, andTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.
Quoted 4 lines
Quoted 1 line
PATCHELF points to the patchelf utility! Or what do you mean here?
I mean, also these environment variables are used by the script. Moreover, the
_DIRvariables are not optional.
Got it!
Quoted 11 lines
[snip]
Quoted 10 lines
Quoted 9 lines
support/scripts/fix-rpath line 92
Quoted 3 lines
support/scripts/fix-rpath new file +3
-0+ target) + rootdir="${TARGET_DIR}" + sanitize_extra_args+=( "--no-standard-lib-dirs" "--relative-to-file" )I thought we decided that target and staging should NOT be relative-to-file,
but should be absolute paths without rootdir? Or was this a problem for staging, because the linker would add the RUNPATH to the library search path and this would point to the system libs instead of target libs? According to the man page this should only be the case for a native linker, so not for a cross linker.
Quoted 5 lines
Quoted 1 line
Grrr, that's wrong, cut an paste error :(. Fixed!
But staging stays the same as target, right?
And did you add comments everywhere to explain why these options are chosen?
case "${tree}" in
host)
...
# we always want $ORIGIN-based rpaths to make it relocatable.
sanitize_extra_args+=( "--relative-to-file" )
;;
staging)
...
# should be like for the target tree below
sanitize_extra_args+=( "--no-standard-lib-dirs" )
;;
target)
...
# we don't want $ORIGIN-based rpaths but absolute paths without rootdir.
# we also want to remove rpaths pointing to /lib or /usr/lib.
sanitize_extra_args+=( "--no-standard-lib-dirs" )
;;
...
esacQuoted 42 lines
Quoted 11 lines
Quoted 11 lines
I don't really like having $ORIGIN-based rpaths in target, but it's no big
deal.Regardless, the reasoning behind the choice we make should be documented here
in comments. So for host, add something to the effect that we always want $ORIGIN-based rpaths to make it relocatable. For staging, say that we want it to be the same as target. And for target, explain the choice you made.Quoted 6 lines
Quoted 4 lines
support/scripts/fix-rpath line 104
Quoted 2 lines
support/scripts/fix-rpath new file +2
-0+ while read file ; do + # check if it's an ELF fileIndentation is inconsistent: sometimes 4 spaces, sometimes 2 spaces (inside the
That's due to 8 spaces replaced by one tab.
Fix your editor not to do that.
Quoted 6 lines
Quoted 3 lines
case, and in the condition below). And here, you're switching from spaces to tabs!
I use the default indention from emacs "Shell-script[bash]" which is usually not a bad choice. I'm going to replace all tabs with spaces.
Since emacs, vim and kate all have compatible mode-settings lines, feel free to
add a mode-setting line below the shebang of the script. Do test it with vim please.Quoted 12 lines
Quoted 5 lines
Quoted 2 lines
+ if ${PATCHELF} --print-rpath "${file}" >
/dev/null2>&1; thenIt might be good to check if the file actually has a non-empty rpath. When
processing a second time, most rpaths are empty so they can be skipped.I already tried that. It will make the check slower, meaning it costs more that calling patchelf a second time.
Quoted 2 lines
Maybe even add another patch to patchelf to exit early and quietly with just 0
or 1 depending on rpath presence.
Quoted 7 lines
Quoted 4 lines
Optimization of the execution time needs more careful analysis. I have a patchelf patch which just open and reads the patchelf header to check quickly if it's an ELF file. But that does not help a lot. readelf is still faster and I guess that's because it's written in C (and not C++).
I have a few speedup suggestions for patchelf (but all should be in follow-up
patches, not as part of patch 1/9).
As I said, the execution time comes form scanning for ELF files. The rest doesn't matter a lot. The attached patch speeds up ELF file checking. As said, readelf is still faster even with that patch.
Quoted 6 lines
- Add a --has-rpath option that is quiet and just exits 0 or 1.
- Instead of reading into a string, mmap the file. Since in most cases the file size isn't changed, that works *very* efficiently. Note that this needs a configure.ac check (mmap is not always available, e.g. not on nommu). With mmap, it's not needed to first read the header only, since it will just pull in a page when needed. It will be slower on NFS and some FUSE filesystems though.
OK, will try that when time permits.
Quoted 4 lines
- Remove empty RUNPATH entries, so a second run doesn't even start.
- Link patchelf statically, preferably with -flto as well. This will probably just speed things up with a few %, but it's also on the "empty" calls so could be significant.
Wolfgang.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Optimize-ELF-file-check.patch
Type: text/x-patch
Size: 1696 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170720/274a74f4/attachment.bin>
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Quoted 4 lines · Wolfgang Grandegger
case "${tree}" in
host)
...
# we always want $ORIGIN-based rpaths to make it relocatable.Perfect. Just please start the sentence with a capital (it ends with a .)
Quoted 6 lines
sanitize_extra_args+=( "--relative-to-file" )
;;
staging)
...
# should be like for the target tree belows/like/the same as/
Quoted 7 lines
sanitize_extra_args+=( "--no-standard-lib-dirs" )
;;
target)
...
# we don't want $ORIGIN-based rpaths but absolute paths without rootdir.
# we also want to remove rpaths pointing to /lib or /usr/lib. Capitalization, otherwise perfect.
[snip]
Quoted 2 lines
As I said, the execution time comes form scanning for ELF files. The rest doesn't matter a lot.
Instead of the patch you attached, it might actually make more sense to create
a tiny C file that does this, and execute that as part of the find command. Or you could just create support/scripts/elfmagic with those 4 bytes and use "cmp -n 4 {} ${0%/*}/elfmagic".
By the way, I believe filtering with -exec is much faster than doing it with
while/if. Unfortunately you can't use that with patchelf --print-rpath because the output of patchelf would be mixed up with the find output.
Regards,
Arnout
Quoted 2 lines
The attached patch speeds up ELF file checking. As said, readelf is still faster even with that patch.
[snip]
Signature · 6 lines
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
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Hello Arnout,
Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
Quoted 9 lines · Wolfgang Grandegger
Quoted 4 lines
case "${tree}" in host) ... # we always want $ORIGIN-based rpaths to make it relocatable.Perfect. Just please start the sentence with a capital (it ends with a .)
Well, I followed the style of the original script... just to be consistent... otherwise we will have an ugly mix.
Quoted 20 lines
Quoted 7 lines
sanitize_extra_args+=( "--relative-to-file" ) ;; staging) ... # should be like for the target tree belows/like/the same as/
Quoted 7 lines
sanitize_extra_args+=( "--no-standard-lib-dirs" ) ;; target) ... # we don't want $ORIGIN-based rpaths but absolute paths without rootdir. # we also want to remove rpaths pointing to /lib or /usr/lib.Capitalization, otherwise perfect.
Quoted 8 lines
[snip]
Quoted 2 lines
As I said, the execution time comes form scanning for ELF files. The rest doesn't matter a lot.
Instead of the patch you attached, it might actually make more sense to create
a tiny C file that does this, and execute that as part of the find command. Or you could just create support/scripts/elfmagic with those 4 bytes and use "cmp -n 4 {} ${0%/*}/elfmagic".
Yes, I had that idea as well and I have already hacked something. The problem is that it does not check if it does have an rpath. "cmp" is not faster, I already tried!
Quoted 3 lines
By the way, I believe filtering with -exec is much faster than doing it with
while/if. Unfortunately you can't use that with patchelf --print-rpath because the output of patchelf would be mixed up with the find output.
I already tried various other solutions.
Just preparing v8...
Wolfgang.
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Quoted 11 lines · Wolfgang Grandegger
Hello Arnout,
Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
Quoted 8 lines · Wolfgang Grandegger
Quoted 4 lines
case "${tree}" in host) ... # we always want $ORIGIN-based rpaths to make it relocatable.
Quoted 4 lines
Quoted 1 line
Perfect. Just please start the sentence with a capital (it ends with a .)
Well, I followed the style of the original script... just to be consistent... otherwise we will have an ugly mix.
There is no original script, only an original author :-)
So indeed, fix it script-wide please.
[snip]
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
As I said, the execution time comes form scanning for ELF files. The rest doesn't
matter a lot.Instead of the patch you attached, it might actually make more sense to create
a tiny C file that does this, and execute that as part of the find command. Or you could just create support/scripts/elfmagic with those 4 bytes and use "cmp -n 4 {} ${0%/*}/elfmagic".Yes, I had that idea as well and I have already hacked something. The problem is that it does not check if it does have an rpath.
Well, you could do that particular check in the find -exec and keep the
--print-rpath check in the while loop.
Quoted 2 lines
"cmp" is not faster, I already
tried!
OK good. Might be a good idea to keep track of the things you already tried in
the cover letter.
Regards,
Arnout
Quoted 11 lines
Quoted 4 lines
By the way, I believe filtering with -exec is much faster than doing it with
while/if. Unfortunately you can't use that with patchelf --print-rpath because the output of patchelf would be mixed up with the find output.I already tried various other solutions.
Just preparing v8...
Wolfgang.
Signature · 6 lines
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
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Am 20.07.2017 um 16:19 schrieb Arnout Vandecappelle:
Quoted 14 lines · Wolfgang Grandegger
Quoted 11 lines
Hello Arnout,
Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
Quoted 8 lines · Wolfgang Grandegger
Quoted 4 lines
case "${tree}" in host) ... # we always want $ORIGIN-based rpaths to make it relocatable.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Perfect. Just please start the sentence with a capital (it ends with a .)
Well, I followed the style of the original script... just to be consistent... otherwise we will have an ugly mix.
There is no original script, only an original author :-)
I have no problem to adapt the style of the original author(s) ;). It should just be consistent.
Quoted 19 lines
So indeed, fix it script-wide please.
[snip]
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
As I said, the execution time comes form scanning for ELF files. The rest doesn't
matter a lot.Instead of the patch you attached, it might actually make more sense to create
a tiny C file that does this, and execute that as part of the find command. Or you could just create support/scripts/elfmagic with those 4 bytes and use "cmp -n 4 {} ${0%/*}/elfmagic".Yes, I had that idea as well and I have already hacked something. The problem is that it does not check if it does have an rpath.
Well, you could do that particular check in the find -exec and keep the
--print-rpath check in the while loop.
Quoted 5 lines
Quoted 2 lines
"cmp" is not faster, I already
tried!OK good. Might be a good idea to keep track of the things you already tried in
the cover letter.
Well, it's already a few month ago that I tried finding a good and fast solution. I tried a lot of things, be can't remember the details. My time working on this topic is limited! When I have more free time I will take care.
Wolfgang.
PATCH v7 2/9 support/scripts: add fix-rpath script to sanitize the rpath
Quoted 3 lines · Wolfgang Grandegger
Am 20.07.2017 um 16:19 schrieb Arnout Vandecappelle:
[snip]
Quoted 5 lines
Quoted 2 lines
OK good. Might be a good idea to keep track of the things you already tried in
the cover letter.Well, it's already a few month ago that I tried finding a good and fast solution. I tried a lot of things, be can't remember the details.
That's exactly why I ask to keep track of it in the cover letter. :-)
Since you already posted the series, no need to do it now. But when you post
patches that speed things up, please try to write down what you tested (at least the things that you are sure of).
Regards,
Arnout
Quoted 4 lines
My time
working on this topic is limited! When I have more free time I will take care.Wolfgang.
Signature · 6 lines
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
PATCH v7 3/9 core: sanitize RPATH in staging tree at the end of target finalization
In reply to Wolfgang GrandeggerWe need that pachelf utility for RPATH sanitization.
--- Makefile | 6 ++++++ 1 file changed, 6 insertions(+)
Makefile +6 -0
@@ -456,6 +456,9 @@ else export BR_NO_CCACHE endif +# We need patchelf for RPATH sanitization +PACKAGES += host-patchelf + # Scripts in support/ or post-build scripts may need to reference # these locations, so export them so it is easier to use export BR2_CONFIG
@@ -720,6 +723,9 @@ endif $(call MESSAGE,"Executing post-build script $(s)"); \ $(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep)) + @$(call MESSAGE,"Sanitizing RPATH in staging tree") + $(TOPDIR)/support/scripts/fix-rpath staging + .PHONY: target-post-image target-post-image: $(TARGETS_ROOTFS) target-finalize @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
Signature · 1 line
2.7.4
PATCH v7 3/9 core: sanitize RPATH in staging tree at the end of target finalization
Quoted 12 lines · Wolfgang Grandegger
We need that pachelf utility for RPATH sanitization.
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 6 ++++++ 1 file changed, 6 insertions(+)Makefile +0
-0
Makefile else · line 460
Quoted 5 lines
Makefile +2
-0@@ -456,6 +456,9 @@ else export BR_NO_CCACHE endif +# We need patchelf for RPATH sanitization +PACKAGES += host-patchelf
This really should be a separate patch. Also, I would do it differently.
Instead of adding it explicitly to PACKAGES, add a Config.in.host for patchelf, with a hidden symbol BR2_PACKAGE_HOST_PATCHELF, and default it to y. We want to create Config.in.host files with hidden symbols for all host packages, so doing it that way is already a step in the right direction.
Makefile endif · line 727
Quoted 10 lines
Makefile +3
-0+ # Scripts in support/ or post-build scripts may need to reference # these locations, so export them so it is easier to use export BR2_CONFIG@@ -720,6 +723,9 @@ endif $(call MESSAGE,"Executing post-build script $(s)"); \ $(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep)) + @$(call MESSAGE,"Sanitizing RPATH in staging tree") + $(TOPDIR)/support/scripts/fix-rpath staging
I don't think we should do this in target-finalize. But I'll discuss this in
patch 5.
Regards,
Arnout
Quoted 5 lines
Makefile +1
-0+ .PHONY: target-post-image target-post-image: $(TARGETS_ROOTFS) target-finalize @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
Signature · 6 lines
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
PATCH v7 4/9 core: sanitize RPATH in target tree at the end of target finalization
In reply to Wolfgang Grandegger--- Makefile | 2 ++ 1 file changed, 2 insertions(+)
Makefile +2 -0
@@ -725,6 +725,8 @@ endif @$(call MESSAGE,"Sanitizing RPATH in staging tree") $(TOPDIR)/support/scripts/fix-rpath staging + @$(call MESSAGE,"Sanitizing RPATH in target tree") + $(TOPDIR)/support/scripts/fix-rpath target .PHONY: target-post-image target-post-image: $(TARGETS_ROOTFS) target-finalize
Signature · 1 line
2.7.4
PATCH v7 4/9 core: sanitize RPATH in target tree at the end of target finalization
Quoted 10 lines · Wolfgang Grandegger
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 2 ++ 1 file changed, 2 insertions(+)Makefile +0
-0
Makefile endif · line 729
Quoted 5 lines
Makefile +2
-0@@ -725,6 +725,8 @@ endif @$(call MESSAGE,"Sanitizing RPATH in staging tree") $(TOPDIR)/support/scripts/fix-rpath staging + @$(call MESSAGE,"Sanitizing RPATH in target tree") + $(TOPDIR)/support/scripts/fix-rpath target
I think we should do this *before* copying the overlay and calling the
post-build script. Indeed, in my mind, the user is completely responsible for what happens there, and it should never be touched by us.
Regards,
Arnout
Quoted 4 lines
Makefile +0
-0.PHONY: target-post-image target-post-image: $(TARGETS_ROOTFS) target-finalize
Signature · 6 lines
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
PATCH v7 4/9 core: sanitize RPATH in target tree at the end of target finalization
Am 20.07.2017 um 01:34 schrieb Arnout Vandecappelle:
Quoted 14 lines · Wolfgang Grandegger
Quoted 11 lines
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 2 ++ 1 file changed, 2 insertions(+)Makefile +0
-0@@ -725,6 +725,8 @@ endif
Quoted 8 lines
Makefile endif · line 729
Quoted 4 lines
Makefile +2
-0@$(call MESSAGE,"Sanitizing RPATH in staging tree") $(TOPDIR)/support/scripts/fix-rpath staging + @$(call MESSAGE,"Sanitizing RPATH in target tree") + $(TOPDIR)/support/scripts/fix-rpath targetI think we should do this *before* copying the overlay and calling the
post-build script. Indeed, in my mind, the user is completely responsible for what happens there, and it should never be touched by us.
Well, the user will usually not care about the rpath. Therefore he might profit from the sanitization as well. Actually, the sanitization does usually not harm. The only thing I can think of is copying libraries at runtime to a dir set in an rpath. How is stripping handled?
Wolfgang.
PATCH v7 4/9 core: sanitize RPATH in target tree at the end of target finalization
Quoted 25 lines · Wolfgang Grandegger
Am 20.07.2017 um 01:34 schrieb Arnout Vandecappelle:
Quoted 8 lines · Wolfgang Grandegger
Quoted 5 lines
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 2 ++ 1 file changed, 2 insertions(+)Quoted 13 lines
Makefile endif · line 728
Quoted 9 lines
Makefile +2
-0@@ -725,6 +725,8 @@ endif @$(call MESSAGE,"Sanitizing RPATH in staging tree") $(TOPDIR)/support/scripts/fix-rpath staging + @$(call MESSAGE,"Sanitizing RPATH in target tree") + $(TOPDIR)/support/scripts/fix-rpath targetI think we should do this *before* copying the overlay and calling the
post-build script. Indeed, in my mind, the user is completely responsible for what happens there, and it should never be touched by us.
Quoted 4 lines
Well, the user will usually not care about the rpath. Therefore he might profit from the sanitization as well. Actually, the sanitization does usually not harm. The only thing I can think of is copying libraries at runtime to a dir set in an rpath. How is stripping handled?
Look at the code :-) Stripping is done before overlay and post-build. Same with
removing locales and documentation etc. And also all package hooks, like removing *.py files. Really, overlay and post-build are currently the very last things that are done before making the rootfs.
Regards,
Arnout
Signature · 6 lines
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
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
In reply to Wolfgang GrandeggerWe introduce the "finalize-host" target for that purpose.
--- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Makefile +6 -1
@@ -553,7 +553,12 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) prepare: $(BUILD_DIR)/buildroot-config/auto.conf .PHONY: world -world: target-post-image +world: target-post-image host-finalize + +.PHONY: host-finalize +host-finalize: + @$(call MESSAGE,"Rendering the SDK relocatable") + $(TOPDIR)/support/scripts/fix-rpath host # When creating HOST_DIR, also symlink usr -> . $(HOST_DIR):
Signature · 1 line
2.7.4
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Quoted 12 lines · Wolfgang Grandegger
We introduce the "finalize-host" target for that purpose.
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)Makefile +0
-0
Makefile $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) · line 561
Quoted 10 lines
Makefile +6
-1@@ -553,7 +553,12 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) prepare: $(BUILD_DIR)/buildroot-config/auto.conf .PHONY: world -world: target-post-image +world: target-post-image host-finalize + +.PHONY: host-finalize +host-finalize: + @$(call MESSAGE,"Rendering the SDK relocatable") + $(TOPDIR)/support/scripts/fix-rpath host
I'm not sure why, but this is taking a very long time for me - twice as much as
target and staging together. Possibly this is because the external toolchain (which tends to be large) is also still processed. Passing TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR in the environment of this call would help.
This sanitization takes a non-trivial amount of time. For example, on a config
with basically just systemd enabled, rebuilding the rootfs tarballs without the sanitization takes just 5 seconds. target+staging sanitization adds 10 seconds to that. host sanitization adds another 20 seconds. So globally we go from 5 to 34 seconds as the baseline time to rebuild anything.
Also, we discussed this before (also with Thomas): I'm not so sure we want to
do this as part of the normal build. It would make sense to me to have a new "sdk" target which is NOT a dependency of world, that should be called explicitly to get a relocatable toolchain. That already solves a big part of the time issue I mentioned above. But it also means we can later add other things to that sdk target, like creating a tarball of it. And anyway the host sanitization is only useful if you're actually going to move the sdk somewhere else.
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.
Regards,
Arnout
Quoted 4 lines
Makefile +0
-0# When creating HOST_DIR, also symlink usr -> . $(HOST_DIR):
Signature · 6 lines
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
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
Quoted 31 lines · Wolfgang Grandegger
Quoted 12 lines
We introduce the "finalize-host" target for that purpose.
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)Makefile +0
-0Makefile $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) · line 561
Quoted 10 lines
Makefile +6
-1@@ -553,7 +553,12 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) prepare: $(BUILD_DIR)/buildroot-config/auto.conf .PHONY: world -world: target-post-image +world: target-post-image host-finalize + +.PHONY: host-finalize +host-finalize: + @$(call MESSAGE,"Rendering the SDK relocatable") + $(TOPDIR)/support/scripts/fix-rpath hostI'm not sure why, but this is taking a very long time for me - twice as much as
target and staging together. Possibly this is because the external toolchain (which tends to be large) is also still processed. PassingTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIRin the environment of this call would help.
Quoted 5 lines
This sanitization takes a non-trivial amount of time. For example, on a config
with basically just systemd enabled, rebuilding the rootfs tarballs without the sanitization takes just 5 seconds. target+staging sanitization adds 10 seconds to that. host sanitization adds another 20 seconds. So globally we go from 5 to 34 seconds as the baseline time to rebuild anything.
Yes, that's the downside of doing it at the end of the build on all 3 trees. Most of the time is consumed for scanning for ELF files. The execution time should be therefore proportional to the number of scanned files. Maybe a database/cache could speed up things. Could you please send me your "defconfig" to check the times here?
Quoted 8 lines
Also, we discussed this before (also with Thomas): I'm not so sure we want to
do this as part of the normal build. It would make sense to me to have a new "sdk" target which is NOT a dependency of world, that should be called explicitly to get a relocatable toolchain. That already solves a big part of the time issue I mentioned above. But it also means we can later add other things to that sdk target, like creating a tarball of it. And anyway the host sanitization is only useful if you're actually going to move the sdk somewhere else.
I already understood that "make sdk" is the preferred solution. I just thought somebody else would implement that ;).
Quoted 4 lines
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.
I might have missed a few things here.
Quoted 3 lines
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.
D'accord!
Wolfgang.
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Quoted 40 lines · Wolfgang Grandegger
Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
Quoted 31 lines · Wolfgang Grandegger
Quoted 12 lines
We introduce the "finalize-host" target for that purpose.
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)Makefile +0
-0Makefile $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) · line 560
Quoted 9 lines
Makefile +6
-1@@ -553,7 +553,12 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) prepare: $(BUILD_DIR)/buildroot-config/auto.conf .PHONY: world -world: target-post-image +world: target-post-image host-finalize + +.PHONY: host-finalize +host-finalize: + @$(call MESSAGE,"Rendering the SDK relocatable") + $(TOPDIR)/support/scripts/fix-rpath hostI'm not sure why, but this is taking a very long time for me - twice as much as
target and staging together. Possibly this is because the external toolchain (which tends to be large) is also still processed. PassingTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIRin the environment of this call would help.Quoted 5 lines
This sanitization takes a non-trivial amount of time. For example, on a config
with basically just systemd enabled, rebuilding the rootfs tarballs without the sanitization takes just 5 seconds. target+staging sanitization adds 10 seconds to that. host sanitization adds another 20 seconds. So globally we go from 5 to 34 seconds as the baseline time to rebuild anything.
Quoted 4 lines
Yes, that's the downside of doing it at the end of the build on all 3 trees. Most of the time is consumed for scanning for ELF files. The execution time should be therefore proportional to the number of scanned files. Maybe a database/cache could speed up things.
That was the idea behind doing things after each package build :-)
For a cache, you still need to md5 all files to detect if they have change
since the last time. Checking for an ELF header should be faster...
Quoted 1 line
Could you please send me your "defconfig" to check the times here?
It's just an external toolchain + kernel + BR2_INIT_SYSTEMD=y
Quoted 10 lines
Quoted 7 lines
Also, we discussed this before (also with Thomas): I'm not so sure we want to
do this as part of the normal build. It would make sense to me to have a new "sdk" target which is NOT a dependency of world, that should be called explicitly to get a relocatable toolchain. That already solves a big part of the time issue I mentioned above. But it also means we can later add other things to that sdk target, like creating a tarball of it. And anyway the host sanitization is only useful if you're actually going to move the sdk somewhere else.I already understood that "make sdk" is the preferred solution. I just thought somebody else would implement that ;).
Well, just rename host-finalize to sdk, and remove it from the dependencies of
"world". Oh, and add it to the 'make help' output.
Documentation should be added as well, but that should be done anyway (to
explain that the relocate-sdk script should be called).
Quoted 6 lines
Quoted 4 lines
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.I might have missed a few things here.
Dependencies are really needed, otherwise a top-level parallel build would run
host-finalize in the beginning of the build instead of at the end.
Oh, and forget about filtering host-% from PACKAGES. If staging sanitization is
also done here, then it should depend on all packages.
Regards,
Arnout
Quoted 8 lines
Quoted 4 lines
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.D'accord!
Wolfgang.
Signature · 6 lines
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
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:
Quoted 66 lines · Wolfgang Grandegger
Quoted 40 lines
Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
Quoted 31 lines · Wolfgang Grandegger
Quoted 12 lines
We introduce the "finalize-host" target for that purpose.
Signed-off-by Wolfgang Grandegger <[email protected]>--- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)Makefile +0
-0Makefile $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) · line 560
Quoted 9 lines
Makefile +6
-1@@ -553,7 +553,12 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) prepare: $(BUILD_DIR)/buildroot-config/auto.conf .PHONY: world -world: target-post-image +world: target-post-image host-finalize + +.PHONY: host-finalize +host-finalize: + @$(call MESSAGE,"Rendering the SDK relocatable") + $(TOPDIR)/support/scripts/fix-rpath hostI'm not sure why, but this is taking a very long time for me - twice as much as
target and staging together. Possibly this is because the external toolchain (which tends to be large) is also still processed. PassingTOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIRin the environment of this call would help.Quoted 5 lines
This sanitization takes a non-trivial amount of time. For example, on a config
with basically just systemd enabled, rebuilding the rootfs tarballs without the sanitization takes just 5 seconds. target+staging sanitization adds 10 seconds to that. host sanitization adds another 20 seconds. So globally we go from 5 to 34 seconds as the baseline time to rebuild anything.Quoted 4 lines
Yes, that's the downside of doing it at the end of the build on all 3 trees. Most of the time is consumed for scanning for ELF files. The execution time should be therefore proportional to the number of scanned files. Maybe a database/cache could speed up things.
That was the idea behind doing things after each package build :-)
For a cache, you still need to md5 all files to detect if they have change
since the last time. Checking for an ELF header should be faster...Quoted 1 line
Could you please send me your "defconfig" to check the times here?
It's just an external toolchain + kernel + BR2_INIT_SYSTEMD=y
Quoted 8 lines
Quoted 7 lines
Also, we discussed this before (also with Thomas): I'm not so sure we want to
do this as part of the normal build. It would make sense to me to have a new "sdk" target which is NOT a dependency of world, that should be called explicitly to get a relocatable toolchain. That already solves a big part of the time issue I mentioned above. But it also means we can later add other things to that sdk target, like creating a tarball of it. And anyway the host sanitization is only useful if you're actually going to move the sdk somewhere else.
Quoted 8 lines
Quoted 2 lines
I already understood that "make sdk" is the preferred solution. I just thought somebody else would implement that ;).
Well, just rename host-finalize to sdk, and remove it from the dependencies of
"world". Oh, and add it to the 'make help' output.Documentation should be added as well, but that should be done anyway (to
explain that the relocate-sdk script should be called).
OK, just to double-check. "make sdk" will sanitize the staging and host tree and add relocate-sdk.sh. But the target tree will always be sanitized.
Wolfgang.
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Quoted 3 lines · Wolfgang Grandegger
Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:
[snip]
Quoted 8 lines
Quoted 5 lines
Well, just rename host-finalize to sdk, and remove it from the dependencies of
"world". Oh, and add it to the 'make help' output.Documentation should be added as well, but that should be done anyway (to
explain that the relocate-sdk script should be called).OK, just to double-check. "make sdk" will sanitize the staging and host tree and add
relocate-sdk.sh. But the target tree will always be sanitized.
Exactly!
Regards,
Arnout
Signature · 6 lines
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
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Hello,
Quoted 6 lines · Arnout Vandecappelle
Quoted 1 line
Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:
[snip]
Quoted 3 lines
Quoted 3 lines
Well, just rename host-finalize to sdk, and remove it from the dependencies of
"world". Oh, and add it to the 'make help' output.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Documentation should be added as well, but that should be done anyway (to
explain that the relocate-sdk script should be called).OK, just to double-check. "make sdk" will sanitize the staging and host tree and add
relocate-sdk.sh. But the target tree will always be sanitized.Exactly!
No, I don't agree here. Why would the staging sanitization be related to doing the SDK ?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
In reply to Arnout VandecappelleHello,
Quoted 5 lines · Arnout Vandecappelle
This sanitization takes a non-trivial amount of time. For example, on a config
with basically just systemd enabled, rebuilding the rootfs tarballs without the sanitization takes just 5 seconds. target+staging sanitization adds 10 seconds to that. host sanitization adds another 20 seconds. So globally we go from 5 to 34 seconds as the baseline time to rebuild anything.
This is indeed not good.
Quoted 7 lines
Also, we discussed this before (also with Thomas): I'm not so sure we want to
do this as part of the normal build. It would make sense to me to have a new "sdk" target which is NOT a dependency of world, that should be called explicitly to get a relocatable toolchain. That already solves a big part of the time issue I mentioned above. But it also means we can later add other things to that sdk target, like creating a tarball of it. And anyway the host sanitization is only useful if you're actually going to move the sdk somewhere else.
Yes for "make sdk".
Quoted 4 lines
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.
For the SDK, you also need to build all the target packages. The SDK contains the sysroot (i.e STAGING_DIR), where the target packages install libraries and headers.
Basically, sdk needs to depend on "world".
Quoted 3 lines
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.
The sanitization in staging and target has nothing to do with making the SDK relocatable. Why do you think it's related ?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Quoted 3 lines · Thomas Petazzoni
Hello,
On Thu, 20 Jul 2017 01:51:53 +0200, Arnout Vandecappelle wrote:
[snip]
Quoted 8 lines
Quoted 4 lines
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.For the SDK, you also need to build all the target packages. The SDK contains the sysroot (i.e
STAGING_DIR), where the target packages install libraries and headers.
This is what I wrote in an earlier reply to Wolfgang.
Quoted 1 line
Basically, sdk needs to depend on "world".
Not really, it doesn't need to depend on target-finalize or the rootfs targets.
Quoted 6 lines
Quoted 3 lines
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.The sanitization in staging and target has nothing to do with making the SDK relocatable. Why do you think it's related ?
target indeed has nothing to do with making the SDK relocatable. It is only
needed to deal with stupid packages that don't correctly use --prefix/DESTDIR and that end up putting the full absolute build-time directory in rpath.
staging is different. Sanitizing staging is not really needed, in the sense
that any rpath in there is simply not going to be used. We want to sanitize staging for the following reasons:
- To avoid leaking references to the original output directory. This way, we can validate that the SDK is relocatable by running a simple "grep -r ${BASE_DIR} ${HOST_DIR}". Obviously RPATH sanitization is not sufficient (e.g. also the references to source files have to be stripped), but it's a step in the right direction. This reason is obviously only relevant for the SDK.
- To make sure that when an executable is copied to target that it actually executes correctly. Since within Buildroot we never copy stuff from staging to target, this is clearly only relevant for the SDK.
Wolfgang, could you make sure that this explanation ends up in the commit log
of patch 3/9?
Regards,
Arnout
Quoted 5 lines
Best regards,
Thomas
Signature · 6 lines
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
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Hello,
Quoted 10 lines · Arnout Vandecappelle
Quoted 8 lines
Quoted 4 lines
Also, host-finalize (or sdk) should depend on toolchain, and probably also on
$(filter host-%,$(PACKAGES)). The latter is currently incomplete, but when we get around to selecting BR2_PACKAGE_HOST_* for all host packages, we will make sure that 'make sdk' really generates the complete sdk.For the SDK, you also need to build all the target packages. The SDK contains the sysroot (i.e
STAGING_DIR), where the target packages install libraries and headers.This is what I wrote in an earlier reply to Wolfgang.
Yes, I was replying as I was reading the thread, and saw your other reply afterwards.
Quoted 3 lines
Quoted 1 line
Basically, sdk needs to depend on "world".
Not really, it doesn't need to depend on target-finalize or the rootfs targets.
Correct, but I am not sure it is really worth being smart here.
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
And finally, I think that the staging sanitization fits better here than in
target-finalize. It has nothing to do with target, and the only reason that we do such sanitization is to make the sdk relocatable.The sanitization in staging and target has nothing to do with making the SDK relocatable. Why do you think it's related ?
target indeed has nothing to do with making the SDK relocatable. It is only
needed to deal with stupid packages that don't correctly use --prefix/DESTDIR and that end up putting the full absolute build-time directory in rpath.
Agreed.
Quoted 10 lines
staging is different. Sanitizing staging is not really needed, in the sense
that any rpath in there is simply not going to be used. We want to sanitize staging for the following reasons:- To avoid leaking references to the original output directory. This way, we can validate that the SDK is relocatable by running a simple "grep -r ${BASE_DIR} ${HOST_DIR}". Obviously RPATH sanitization is not sufficient (e.g. also the references to source files have to be stripped), but it's a step in the right direction. This reason is obviously only relevant for the SDK.
Quoted 3 lines
- To make sure that when an executable is copied to target that it actually executes correctly. Since within Buildroot we never copy stuff from staging to target, this is clearly only relevant for the SDK.
OK, makes sense. I indeed believe having those details in the commit log or in the code would be nice.
So do we agree that:
- target sanitization will be done in target-finalize
- staging sanitization will be done in "make sdk"
- host sanitization will be done in "make sdk"
Correct ?
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH v7 5/9 core: sanitize RPATH in host tree at the very end of the build
Quoted 3 lines · Thomas Petazzoni
Hello,
On Thu, 20 Jul 2017 15:58:49 +0200, Arnout Vandecappelle wrote:
[snip]
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Basically, sdk needs to depend on "world".
Not really, it doesn't need to depend on target-finalize or the rootfs targets.
Correct, but I am not sure it is really worth being smart here.
Indeed it isn't. And having
sdk: world
will discourage anyone trying to add
world: sdk
:-)
[snip]
Quoted 9 lines
So do we agree that:
- target sanitization will be done in target-finalize
- staging sanitization will be done in "make sdk"
- host sanitization will be done in "make sdk"
Correct ?
Yes! And also other sanitization (like stripping references to the build
directories) will be done in "make sdk".
Regards,
Arnout
Signature · 6 lines
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
PATCH v7 6/9 core: install relocation script and location at the end of the build
In reply to Wolfgang GrandeggerThe 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/share/buildroot/sdk-location"-
--- Makefile | 2 ++ 1 file changed, 2 insertions(+)
Makefile +2 -0
@@ -559,6 +559,8 @@ world: target-post-image host-finalize host-finalize: @$(call MESSAGE,"Rendering the SDK relocatable") $(TOPDIR)/support/scripts/fix-rpath host + install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) + echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location # When creating HOST_DIR, also symlink usr -> . $(HOST_DIR):
Signature · 1 line
2.7.4
PATCH v7 6/9 core: install relocation script and location at the end of the build
Quoted 5 lines · 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/share/buildroot/sdk-location"-Signed-off-by Wolfgang Grandegger <[email protected]>
However, while testing this I noticed a problem in relocate-sdk.sh: it uses a
bashism on line 43: < <(...) but the shebang is /bin/sh. I think the best solution is to convert it into a normal pipe, since there is no real reason there to use the < <(...) construct.
Regards,
Arnout
Quoted 18 lines
--- Makefile | 2 ++ 1 file changed, 2 insertions(+)Makefile +2
-0@@ -559,6 +559,8 @@ world: target-post-image host-finalize host-finalize: @$(call MESSAGE,"Rendering the SDK relocatable") $(TOPDIR)/support/scripts/fix-rpath host + install $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR) + echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location # When creating HOST_DIR, also symlink usr -> . $(HOST_DIR):
Signature · 6 lines
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
PATCH v7 7/9 external-toolchain: check if a buildroot SDK has already been relocated
In reply to Wolfgang GrandeggerThe location of the buildroot SDK is stored in the file "sdk-location" in "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. 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.
--- toolchain/helpers.mk | 18 ++++++++++++++++++ toolchain/toolchain-external/pkg-toolchain-external.mk | 1 + 2 files changed, 19 insertions(+)
toolchain/helpers.mk +18 -0
@@ -476,3 +476,21 @@ 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 +# +define 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 \ +) +endef
toolchain/toolchain-external/pkg-toolchain-external.mk +1 -0
@@ -519,6 +519,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))" ; \
Signature · 1 line
2.7.4
PATCH v7 7/9 external-toolchain: check if a buildroot SDK has already been relocated
Quoted 20 lines · Wolfgang Grandegger
The location of the buildroot SDK is stored in the file "sdk-location" in "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. 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 <[email protected]>--- toolchain/helpers.mk | 18 ++++++++++++++++++ toolchain/toolchain-external/pkg-toolchain-external.mk | 1 + 2 files changed, 19 insertions(+)toolchain/helpers.mk +0
-0@@ -476,3 +476,21 @@ define simplify_symlink ln -sf "$${DOTS}$${REL_DEST}" "$${FULL_SRC}" ; \ )
toolchain/helpers.mk define simplify_symlink · line 487
Quoted 10 lines
toolchain/helpers.mk +9
-0endef + +# +# 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 +# +define check_buildroot_sdk_relocated +( \
The brackets here are redundant. Yes, I know, most other functions in this file
add them, but really they shouldn't.
toolchain/helpers.mk define simplify_symlink · line 489
Quoted 2 lines
toolchain/helpers.mk +2
-0+ if [ -r $(1)/share/buildroot/sdk-location ]; then \ + sdkroot=`dirname "$(1)"`; \
Is this correct? Shouldn't it be just $(1)? Yes it should :-)
Otherwise looks good to me.
Regards,
Arnout
Quoted 20 lines
toolchain/helpers.mk +7
-0+ 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 \ +) +endeftoolchain/toolchain-external/pkg-toolchain-external.mk +1
-0@@ -519,6 +519,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))" ; \
Signature · 6 lines
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
PATCH v7 8/9 support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot
In reply to Wolfgang GrandeggerThis is because $(HOST_DIR)/usr is gone.
--- support/misc/relocate-sdk.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
support/misc/relocate-sdk.sh +1 -1
@@ -5,7 +5,7 @@ if [ "$#" -ne 0 ]; then exit 1 fi -LOCFILE="usr/share/buildroot/sdk-location" +LOCFILE="share/buildroot/sdk-location" FILEPATH="$(readlink -f "$0")" NEWPATH="$(dirname "${FILEPATH}")"
Signature · 1 line
2.7.4
PATCH v7 8/9 support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot
Quoted 3 lines · Wolfgang Grandegger
This is because $(HOST_DIR)/usr is gone.
Signed-off-by Wolfgang Grandegger <[email protected]>
Can be applied right away.
Regards,
Arnout
Quoted 18 lines
--- support/misc/relocate-sdk.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)support/misc/relocate-sdk.sh +1
-1@@ -5,7 +5,7 @@ if [ "$#" -ne 0 ]; then exit 1 fi -LOCFILE="usr/share/buildroot/sdk-location" +LOCFILE="share/buildroot/sdk-location" FILEPATH="$(readlink -f "$0")" NEWPATH="$(dirname "${FILEPATH}")"
Signature · 6 lines
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
Thomas Petazzoni
AppliedPATCH v7 8/9 support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot
In reply to Wolfgang GrandeggerHello,
Quoted 6 lines · Wolfgang Grandegger
This is because $(HOST_DIR)/usr is gone.
Signed-off-by Wolfgang Grandegger <[email protected]>--- support/misc/relocate-sdk.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to master, thanks.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH v7 9/9 package/qt5base: provide "qt.conf" to make "qmake" relocatable
In reply to Wolfgang GrandeggerThe file "qt.conf" can be used to override the hard-coded paths that are compiled into the Qt library. This is required to make "qmake" relocatable. Actually, we need to specify all variables to overwrite the compiled in values.
--- package/qt5/qt5base/qt.conf.in | 19 +++++++++++++++++++ package/qt5/qt5base/qt5base.mk | 8 ++++++++ 2 files changed, 27 insertions(+) create mode 100644 package/qt5/qt5base/qt.conf.in
package/qt5/qt5base/qt.conf.in new file +19 -0
@@ -0,0 +1,19 @@ +[Paths] +Prefix=@@HOST_DIR@@/usr +Sysroot=@@STAGING_DIR@@ +Headers=/usr/include/qt5 +Libraries=/usr/lib +LibraryExecutables=/usr/libexec +Binaries=/usr/bin +Plugins=/usr/lib/qt/plugins +Examples=/usr/lib/qt/examples +Qml2Imports=/usr/qml +Imports=/usr/imports +Translations=/usr/translations +Examples=/usr/lib/qt/examples +Demos=/usr/lib/qt/examples +Tests=/usr/tests +Settings=/usr +Documentation=/usr/doc +ArchData=/usr +Data=/usr
package/qt5/qt5base/qt5base.mk +8 -0
@@ -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
Signature · 1 line
2.7.4
v7 cover Make the SDK relocatable
In reply to Wolfgang GrandeggerHello,
Quoted 15 lines · Wolfgang Grandegger
Samuel Martin (1):
support/scripts: add fix-rpath script to sanitize the rpathWolfgang Grandegger (8):
package/patchelf: add patch for rpath sanitization under a root
directory
core: sanitize RPATH in staging tree at the end of target finalization
core: sanitize RPATH in target tree at the end of target finalization
core: sanitize RPATH in host tree at the very end of the build
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:relocate-sdk.shnow creates sdk-location in
share/buildroot
package/qt5base: provide "qt.conf" to make "qmake" relocatable
Could you please Cc: me explicitly for future versions of this patch series? I'm really interested in following the discussion going on with Arnout, and I missed the discussion between Arnout and you from earlier today, because I wasn't Cc'ed.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com