[Buildroot] [PATCH v7 0/9] Make the SDK relocatable

[Buildroot] [PATCH v7 0/9] Make the SDK relocatable

[Wolfgang Grandegger]

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

-- 
2.7.4

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Wolfgang Grandegger]

The patch allows to use patchelf to sanitize the rpath of the buildroot
libraries and binaries using the option "--make-rpath-relative <rootdir>".
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 <wg@grandegger.com>
---
 ...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

diff --git a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
new file mode 100644
index 0000000..eda32e8
--- /dev/null
+++ b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
@@ -0,0 +1,52 @@
+From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
+From: Eelco Dolstra <eelco.dolstra@logicblox.com>
+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 <wg@grandegger.com>
+
+---
+ 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 = "";
+ 
diff --git a/package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch b/package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch
new file mode 100644
index 0000000..0b00e6d
--- /dev/null
+++ b/package/patchelf/0002-Extract-a-function-for-splitting-a-colon-separated-s.patch
@@ -0,0 +1,63 @@
+From 2e3fdc2030c75c19df6fc2924083cfad53856562 Mon Sep 17 00:00:00 2001
+From: Tuomas Tynkkynen <tuomas@tuxera.com>
+Date: Fri, 3 Jun 2016 23:03:51 +0300
+Subject: [PATCH] Extract a function for splitting a colon-separated
+ string
+
+We're going to need this logic in another place, so make a function of
+this.
+
+[Upstream-commit: https://github.com/NixOS/patchelf/commit/2e3fdc2030c75c19df6fc2924083cfad53856562]
+Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
+
+
+---
+ src/patchelf.cc | 28 +++++++++++++++++++---------
+ 1 file changed, 19 insertions(+), 9 deletions(-)
+
+Index: patchelf-0.9/src/patchelf.cc
+===================================================================
+--- patchelf-0.9.orig/src/patchelf.cc	2017-07-03 14:04:36.988281130 +0200
++++ patchelf-0.9/src/patchelf.cc	2017-07-03 14:04:36.988281130 +0200
+@@ -57,6 +57,22 @@
+ #define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym
+ 
+ 
++static vector<string> splitColonDelimitedString(const char * s){
++    vector<string> parts;
++    const char * pos = s;
++    while (*pos) {
++        const char * end = strchr(pos, ':');
++        if (!end) end = strchr(pos, 0);
++
++        parts.push_back(string(pos, end - pos));
++        if (*end == ':') ++end;
++        pos = end;
++    }
++
++    return parts;
++}
++
++
+ static unsigned int getPageSize(){
+     return pageSize;
+ }
+@@ -1093,15 +1109,9 @@
+ 
+         newRPath = "";
+ 
+-        char * pos = rpath;
+-        while (*pos) {
+-            char * end = strchr(pos, ':');
+-            if (!end) end = strchr(pos, 0);
+-
+-            /* Get the name of the directory. */
+-            string dirName(pos, end - pos);
+-            if (*end == ':') ++end;
+-            pos = end;
++        vector<string> rpathDirs = splitColonDelimitedString(rpath);
++        for (vector<string>::iterator it = rpathDirs.begin(); it != rpathDirs.end(); ++it) {
++            const string & dirName = *it;
+ 
+             /* Non-absolute entries are allowed (e.g., the special
+                "$ORIGIN" hack). */
diff --git a/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
new file mode 100644
index 0000000..ed7bb12
--- /dev/null
+++ b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
@@ -0,0 +1,423 @@
+From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
+From: Wolfgang Grandegger <wg@grandegger.com>
+Date: Mon, 20 Feb 2017 16:29:24 +0100
+Subject: [PATCH] Add option to make the rpath relative under a specified root
+ directory
+
+Running "patchelf" with the option "--make-rpath-relative ROOTDIR" will
+modify or delete the RPATHDIRs according the following rules
+similar to Martin's patches [1] making the Buildroot toolchaing/SDK
+relocatable.
+
+RPATHDIR starts with "$ORIGIN":
+    The original build-system already took care of setting a relative
+    RPATH, resolve it and test if it's valid (does exist)
+
+RPATHDIR starts with ROOTDIR:
+    The original build-system added some absolute RPATH (absolute on
+    the build machine). Test if it's valid (does exist).
+
+ROOTDIR/RPATHDIR exists:
+    The original build-system already took care of setting an absolute
+    RPATH (absolute in the final rootfs), resolve it and test if it's
+    valid (does exist).
+
+RPATHDIR points somewhere else:
+    (can be anywhere: build trees, staging tree, host location,
+    non-existing location, etc.). Just discard such a path.
+
+The option "--no-standard-libs" will discard RPATHDIRs ROOTDIR/lib and
+ROOTDIR/usr/lib. Like "--shrink-rpath", RPATHDIRs are also discarded
+if the directories do not contain a library referenced by the
+DT_NEEDED fields.
+If the option "--relative-to-file" is given, the rpath will start
+with "$ORIGIN" making it relative to the ELF file, otherwise an
+absolute path relative to ROOTDIR will be used.
+
+A pull request for a similar patch [2] for mainline inclusion is
+pending.
+
+[1] http://lists.busybox.net/pipermail/buildroot/2016-April/159422.html
+[2] https://github.com/NixOS/patchelf/pull/118
+
+Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
+---
+ src/patchelf.cc | 187 ++++++++++++++++++++++++++++++++++++++++++++++++--------
+ 1 file changed, 161 insertions(+), 26 deletions(-)
+
+Index: patchelf-0.9/src/patchelf.cc
+===================================================================
+--- patchelf-0.9.orig/src/patchelf.cc	2017-07-03 14:05:07.196281487 +0200
++++ patchelf-0.9/src/patchelf.cc	2017-07-03 16:13:38.590374530 +0200
+@@ -46,13 +46,16 @@
+ 
+ static bool forceRPath = false;
+ 
++static bool noStandardLibDirs = false;
++
++static bool relativeToFile = false;
++
+ static string fileName;
+ static int pageSize = PAGESIZE;
+ 
+ off_t fileSize, maxSize;
+ unsigned char * contents = 0;
+ 
+-
+ #define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym
+ #define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym
+ 
+@@ -77,6 +80,49 @@
+     return pageSize;
+ }
+ 
++static bool absolutePathExists(const string & path, string & canonicalPath)
++{
++    char *cpath = realpath(path.c_str(), NULL);
++    if (cpath) {
++        canonicalPath = cpath;
++        free(cpath);
++        return true;
++    } else {
++        return false;
++    }
++}
++
++static string makePathRelative(const string & path,
++    const string & refPath)
++{
++    string relPath = "$ORIGIN";
++    string p = path, refP = refPath;
++    size_t pos;
++
++    /* Strip the common part of path and refPath */
++    while (true) {
++        pos = p.find_first_of('/', 1);
++	if (refP.find_first_of('/', 1) != pos)
++	    break;
++	if (p.substr(0, pos) != refP.substr(0, pos))
++            break;
++	if (pos == string::npos)
++	    break;
++	p = p.substr(pos);
++	refP = refP.substr(pos);
++    }
++    /* Check if both pathes are equal */
++    if (p != refP) {
++	pos = 0;
++	while (pos != string::npos) {
++	    pos =refP.find_first_of('/', pos + 1);
++	    relPath.append("/..");
++	}
++	relPath.append(p);
++    }
++
++    return relPath;
++}
+ 
+ template<ElfFileParams>
+ class ElfFile
+@@ -183,14 +229,18 @@
+ 
+     void setInterpreter(const string & newInterpreter);
+ 
+-    typedef enum { rpPrint, rpShrink, rpSet, rpRemove } RPathOp;
++    typedef enum { rpPrint, rpShrink, rpMakeRelative, rpSet, rpRemove} RPathOp;
+ 
+-    void modifyRPath(RPathOp op, string newRPath);
++    bool libFoundInRPath(const string & dirName,
++                         const vector<string> neededLibs,
++			 vector<bool> & neededLibFound);
++
++    void modifyRPath(RPathOp op, string rootDir, string newRPath);
+ 
+     void addNeeded(set<string> libs);
+ 
+     void removeNeeded(set<string> libs);
+-    
++
+     void replaceNeeded(map<string, string>& libs);
+ 
+     void printNeededLibs();
+@@ -1041,7 +1091,27 @@
+ 
+ 
+ template<ElfFileParams>
+-void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op, string newRPath)
++bool ElfFile<ElfFileParamNames>::libFoundInRPath(const string & dirName,
++    const vector<string> neededLibs, vector<bool> & neededLibFound)
++{
++    /* For each library that we haven't found yet, see if it
++       exists in this directory. */
++    bool libFound = false;
++    for (unsigned int j = 0; j < neededLibs.size(); ++j)
++        if (!neededLibFound[j]) {
++            string libName = dirName + "/" + neededLibs[j];
++            struct stat st;
++            if (stat(libName.c_str(), &st) == 0) {
++                neededLibFound[j] = true;
++                libFound = true;
++            }
++        }
++    return libFound;
++}
++
++
++template<ElfFileParams>
++void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op, string rootDir, string newRPath)
+ {
+     Elf_Shdr & shdrDynamic = findSection(".dynamic");
+ 
+@@ -1096,6 +1166,11 @@
+         return;
+     }
+ 
++    if (op == rpMakeRelative && !rpath) {
++        debug("no RPATH to make relative\n");
++        return;
++    }
++
+     if (op == rpShrink && !rpath) {
+         debug("no RPATH to shrink\n");
+         return;
+@@ -1120,26 +1195,86 @@
+                 continue;
+             }
+ 
+-            /* For each library that we haven't found yet, see if it
+-               exists in this directory. */
+-            bool libFound = false;
+-            for (unsigned int j = 0; j < neededLibs.size(); ++j)
+-                if (!neededLibFound[j]) {
+-                    string libName = dirName + "/" + neededLibs[j];
+-                    struct stat st;
+-                    if (stat(libName.c_str(), &st) == 0) {
+-                        neededLibFound[j] = true;
+-                        libFound = true;
+-                    }
+-                }
+-
+-            if (!libFound)
++            if (!libFoundInRPath(dirName, neededLibs, neededLibFound))
+                 debug("removing directory `%s' from RPATH\n", dirName.c_str());
+             else
+                 concatToRPath(newRPath, dirName);
+         }
+     }
+ 
++    /* Make the the RPATH relative to the specified path */
++    if (op == rpMakeRelative) {
++        vector<bool> neededLibFound(neededLibs.size(), false);
++        string fileDir = fileName.substr(0, fileName.find_last_of("/"));
++
++        newRPath = "";
++
++        vector<string> rpathDirs = splitColonDelimitedString(rpath);
++        for (vector<string>::iterator it = rpathDirs.begin(); it != rpathDirs.end(); ++it) {
++            const string & dirName = *it;
++
++            string canonicalPath;
++
++            /* Figure out if we should keep or discard the path. There are several
++               cases to be handled:
++               "dirName" starts with "$ORIGIN":
++                   The original build-system already took care of setting a relative
++                   RPATH. Resolve it and test if it's valid (does exist).
++               "dirName" start with "rootDir":
++                   The original build-system added some absolute RPATH (absolute on
++                   the build machine). Test if it's valid (does exist).
++               "rootDir"/"dirName" exists:
++                    The original build-system already took care of setting an absolute
++                    RPATH (absolute in the final rootfs). Resolve it and test if it's
++                    valid (does exist).
++               "dirName" points somewhere else:
++                    (can be anywhere: build trees, staging tree, host location,
++                    non-existing location, etc.). Just discard such a path. */
++            if (!dirName.compare(0, 7, "$ORIGIN")) {
++                string path = fileDir + dirName.substr(7);
++                if (!absolutePathExists(path, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because '%s' doesn't exist\n",
++                          dirName.c_str(), path.c_str());
++                    continue;
++                }
++            } else if (!dirName.compare(0, rootDir.length(), rootDir)) {
++                if (!absolutePathExists(dirName, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because it doesn't exist\n", dirName.c_str());
++                    continue;
++                }
++            } else {
++                string path = rootDir + dirName;
++                if (!absolutePathExists(path, canonicalPath)) {
++                    debug("removing directory '%s' from RPATH because it's not in rootdir\n",
++                          dirName.c_str());
++                    continue;
++                }
++            }
++
++            if (noStandardLibDirs) {
++                if (!canonicalPath.compare(rootDir + "/lib") ||
++                    !canonicalPath.compare(rootDir + "/usr/lib")) {
++                    debug("removing directory '%s' from RPATH because it's a standard library directory\n",
++                         dirName.c_str());
++                    continue;
++                }
++            }
++
++            if (!libFoundInRPath(canonicalPath, neededLibs, neededLibFound)) {
++                debug("removing directory '%s' from RPATH because it does not contain needed libs\n",
++                      dirName.c_str());
++                continue;
++            }
++
++            /* Finally make "canonicalPath" relative to "filedir" in "rootDir" */
++            if (relativeToFile)
++                concatToRPath(newRPath, makePathRelative(canonicalPath, fileDir));
++            else
++                concatToRPath(newRPath, canonicalPath.substr(rootDir.length()));
++            debug("keeping relative path of %s\n", canonicalPath.c_str());
++        }
++    }
++
+     if (op == rpRemove) {
+         if (!rpath) {
+             debug("no RPATH to delete\n");
+@@ -1261,35 +1396,35 @@
+ void ElfFile<ElfFileParamNames>::replaceNeeded(map<string, string>& libs)
+ {
+     if (libs.empty()) return;
+-    
++
+     Elf_Shdr & shdrDynamic = findSection(".dynamic");
+     Elf_Shdr & shdrDynStr = findSection(".dynstr");
+     char * strTab = (char *) contents + rdi(shdrDynStr.sh_offset);
+ 
+     Elf_Dyn * dyn = (Elf_Dyn *) (contents + rdi(shdrDynamic.sh_offset));
+-    
++
+     unsigned int dynStrAddedBytes = 0;
+-    
++
+     for ( ; rdi(dyn->d_tag) != DT_NULL; dyn++) {
+         if (rdi(dyn->d_tag) == DT_NEEDED) {
+             char * name = strTab + rdi(dyn->d_un.d_val);
+             if (libs.find(name) != libs.end()) {
+                 const string & replacement = libs[name];
+-                
++
+                 debug("replacing DT_NEEDED entry `%s' with `%s'\n", name, replacement.c_str());
+-                
++
+                 // technically, the string referred by d_val could be used otherwise, too (although unlikely)
+                 // we'll therefore add a new string
+                 debug("resizing .dynstr ...");
+-                
++
+                 string & newDynStr = replaceSection(".dynstr",
+                     rdi(shdrDynStr.sh_size) + replacement.size() + 1 + dynStrAddedBytes);
+                 setSubstr(newDynStr, rdi(shdrDynStr.sh_size) + dynStrAddedBytes, replacement + '\0');
+-                
++
+                 dyn->d_un.d_val = shdrDynStr.sh_size + dynStrAddedBytes;
+-                
++
+                 dynStrAddedBytes += replacement.size() + 1;
+-                
++
+                 changed = true;
+             } else {
+                 debug("keeping DT_NEEDED entry `%s'\n", name);
+@@ -1311,7 +1446,7 @@
+     for (set<string>::iterator it = libs.begin(); it != libs.end(); it++) {
+         length += it->size() + 1;
+     }
+-    
++
+     string & newDynStr = replaceSection(".dynstr",
+         rdi(shdrDynStr.sh_size) + length + 1);
+     set<Elf64_Xword> libStrings;
+@@ -1321,7 +1456,7 @@
+         libStrings.insert(rdi(shdrDynStr.sh_size) + pos);
+         pos += it->size() + 1;
+     }
+-    
++
+     /* add all new needed entries to the dynamic section */
+     string & newDynamic = replaceSection(".dynamic",
+         rdi(shdrDynamic.sh_size) + sizeof(Elf_Dyn) * libs.size());
+@@ -1342,7 +1477,7 @@
+         wri(newDyn.d_un.d_val, *it);
+         setSubstr(newDynamic, i * sizeof(Elf_Dyn), string((char *) &newDyn, sizeof(Elf_Dyn)));
+     }
+-    
++
+     changed = true;
+ }
+ 
+@@ -1413,7 +1548,9 @@
+ static bool removeRPath = false;
+ static bool setRPath = false;
+ static bool printRPath = false;
++static bool makeRPathRelative = false;
+ static string newRPath;
++static string rootDir;
+ static set<string> neededLibsToRemove;
+ static map<string, string> neededLibsToReplace;
+ static set<string> neededLibsToAdd;
+@@ -1438,14 +1575,16 @@
+         elfFile.setInterpreter(newInterpreter);
+ 
+     if (printRPath)
+-        elfFile.modifyRPath(elfFile.rpPrint, "");
++        elfFile.modifyRPath(elfFile.rpPrint, "", "");
+ 
+     if (shrinkRPath)
+-        elfFile.modifyRPath(elfFile.rpShrink, "");
++        elfFile.modifyRPath(elfFile.rpShrink, "", "");
+     else if (removeRPath)
+-        elfFile.modifyRPath(elfFile.rpRemove, "");
++        elfFile.modifyRPath(elfFile.rpRemove, "", "");
+     else if (setRPath)
+-        elfFile.modifyRPath(elfFile.rpSet, newRPath);
++        elfFile.modifyRPath(elfFile.rpSet, "", newRPath);
++    else if (makeRPathRelative)
++        elfFile.modifyRPath(elfFile.rpMakeRelative, rootDir, "");
+ 
+     if (printNeeded) elfFile.printNeededLibs();
+ 
+@@ -1508,6 +1647,9 @@
+   [--set-rpath RPATH]\n\
+   [--remove-rpath]\n\
+   [--shrink-rpath]\n\
++  [--make-rpath-relative ROOTDIR]\n\
++  [--no-standard-lib-dirs]\n\
++  [--relative-to-file]\n\
+   [--print-rpath]\n\
+   [--force-rpath]\n\
+   [--add-needed LIBRARY]\n\
+@@ -1541,7 +1683,7 @@
+             if (++i == argc) error("missing argument");
+             pageSize = atoi(argv[i]);
+             if (pageSize <= 0) error("invalid argument to --page-size");
+-	}
++        }
+         else if (arg == "--print-interpreter") {
+             printInterpreter = true;
+         }
+@@ -1564,6 +1706,17 @@
+             setRPath = true;
+             newRPath = argv[i];
+         }
++        else if (arg == "--make-rpath-relative") {
++            if (++i == argc) error("missing argument to --make-rpath-relative");
++            makeRPathRelative = true;
++            rootDir = argv[i];
++        }
++        else if (arg == "--no-standard-lib-dirs") {
++            noStandardLibDirs = true;
++        }
++        else if (arg == "--relative-to-file") {
++            relativeToFile = true;
++        }
+         else if (arg == "--print-rpath") {
+             printRPath = true;
+         }
-- 
2.7.4

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

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

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

diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
new file mode 100755
index 0000000..acf3f73
--- /dev/null
+++ b/support/scripts/fix-rpath
@@ -0,0 +1,122 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+# Copyright (C) 2017 Wolfgang Grandegger <wg@grandegger.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+usage() {
+  cat <<EOF >&2
+Usage:	${0} TREE_KIND
+
+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 ${@}
-- 
2.7.4

[Buildroot] [PATCH v7 3/9] core: sanitize RPATH in staging tree at the end of target finalization

[Wolfgang Grandegger]

We need that pachelf utility for RPATH sanitization.

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

diff --git a/Makefile b/Makefile
index a22e87f..cdc3a64 100644
--- a/Makefile
+++ b/Makefile
@@ -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)), \
-- 
2.7.4

[Buildroot] [PATCH v7 4/9] core: sanitize RPATH in target tree at the end of target finalization

[Wolfgang Grandegger]

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

diff --git a/Makefile b/Makefile
index cdc3a64..1a826fc 100644
--- a/Makefile
+++ b/Makefile
@@ -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
-- 
2.7.4

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Wolfgang Grandegger]

We introduce the "finalize-host" target for that purpose.

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

diff --git a/Makefile b/Makefile
index 1a826fc..2a265e0 100644
--- a/Makefile
+++ b/Makefile
@@ -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):
-- 
2.7.4

[Buildroot] [PATCH v7 6/9] core: install relocation script and location at the end of the build

[Wolfgang Grandegger]

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

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

diff --git a/Makefile b/Makefile
index 2a265e0..32f85e8 100644
--- a/Makefile
+++ b/Makefile
@@ -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):
-- 
2.7.4

[Buildroot] [PATCH v7 7/9] external-toolchain: check if a buildroot SDK has already been relocated

[Wolfgang Grandegger]

The location of the buildroot SDK is stored in the file "sdk-location"
in "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 <wg@grandegger.com>
---
 toolchain/helpers.mk                                   | 18 ++++++++++++++++++
 toolchain/toolchain-external/pkg-toolchain-external.mk |  1 +
 2 files changed, 19 insertions(+)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 6136aef..5c6539e 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -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
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index 33449d3..afccbb6 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -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))" ; \
-- 
2.7.4

[Buildroot] [PATCH v7 8/9] support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot

[Wolfgang Grandegger]

This is because $(HOST_DIR)/usr is gone.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 support/misc/relocate-sdk.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/misc/relocate-sdk.sh b/support/misc/relocate-sdk.sh
index 2a9ebc9..729353a 100755
--- a/support/misc/relocate-sdk.sh
+++ b/support/misc/relocate-sdk.sh
@@ -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}")"
 
-- 
2.7.4

[Buildroot] [PATCH v7 9/9] package/qt5base: provide "qt.conf" to make "qmake" relocatable

[Wolfgang Grandegger]

The file "qt.conf" can be used to override the hard-coded paths that
are compiled into the Qt library. This is required to make "qmake"
relocatable. Actually, we need to specify all variables to overwrite
the compiled in values.

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

diff --git a/package/qt5/qt5base/qt.conf.in b/package/qt5/qt5base/qt.conf.in
new file mode 100644
index 0000000..67ae04d
--- /dev/null
+++ b/package/qt5/qt5base/qt.conf.in
@@ -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
diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
index c5d2aa1..71592b2 100644
--- a/package/qt5/qt5base/qt5base.mk
+++ b/package/qt5/qt5base/qt5base.mk
@@ -258,9 +258,17 @@ define QT5BASE_BUILD_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
 endef
 
+# The file "qt.conf" can be used to override the hard-coded paths that are
+# compiled into the Qt library. We need it to make "qmake" relocatable.
+define QT5BASE_INSTALL_QT_CONF
+	sed -e "s|@@HOST_DIR@@|$(HOST_DIR)|" -e "s|@@STAGING_DIR@@|$(STAGING_DIR)|" \
+		$(QT5BASE_PKGDIR)/qt.conf.in > $(HOST_DIR)/usr/bin/qt.conf
+endef
+
 define QT5BASE_INSTALL_STAGING_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install
 	$(QT5_LA_PRL_FILES_FIXUP)
+	$(QT5BASE_INSTALL_QT_CONF)
 endef
 
 define QT5BASE_INSTALL_TARGET_LIBS
-- 
2.7.4

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Arnout Vandecappelle]

 Hi Wolfgang,

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> The patch allows to use patchelf to sanitize the rpath of the buildroot
> libraries and binaries using the option "--make-rpath-relative <rootdir>".
> 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 <wg@grandegger.com>

 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:

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



> diff --git a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
> new file mode 100644
> index 0000000..eda32e8
> --- /dev/null
> +++ b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
> @@ -0,0 +1,52 @@
> +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
> +From: Eelco Dolstra <eelco.dolstra@logicblox.com>
> +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.

> +
> +[Upstream-commit: https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> +
> +---
> + 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]
> diff --git a/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
> new file mode 100644
> index 0000000..ed7bb12
> --- /dev/null
> +++ b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
> @@ -0,0 +1,423 @@
> +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
> +From: Wolfgang Grandegger <wg@grandegger.com>
> +Date: Mon, 20 Feb 2017 16:29:24 +0100
> +Subject: [PATCH] Add option to make the rpath relative under a specified root
> + directory

 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]
> +@@ -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]

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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> From: Samuel Martin <s.martin49@gmail.com>
> 
> 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 <s.martin49@gmail.com>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

 Some small comments below, but again, can be fixed in follow-up patches, so

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

[snip]
> +Description:
> +
> +    This script scans a tree and sanitize ELF files' RPATH found in there.
                                            ^s
> +
> +    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

> +
> +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, and
TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.

[snip]
> +    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.

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

> +
> +	    # 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" )

 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.


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

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

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> The patch allows to use patchelf to sanitize the rpath of the buildroot
> libraries and binaries using the option "--make-rpath-relative <rootdir>".
> 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

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

[Buildroot] [PATCH v7 3/9] core: sanitize RPATH in staging tree at the end of target finalization

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> We need that pachelf utility for RPATH sanitization.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index a22e87f..cdc3a64 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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.


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

> +
>  .PHONY: target-post-image
>  target-post-image: $(TARGETS_ROOTFS) target-finalize
>  	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
> 

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

[Buildroot] [PATCH v7 4/9] core: sanitize RPATH in target tree at the end of target finalization

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index cdc3a64..1a826fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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

>  
>  .PHONY: target-post-image
>  target-post-image: $(TARGETS_ROOTFS) target-finalize
> 

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> We introduce the "finalize-host" target for that purpose.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  Makefile | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 1a826fc..2a265e0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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

>  
>  # When creating HOST_DIR, also symlink usr -> .
>  $(HOST_DIR):
> 

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

[Buildroot] [PATCH v7 6/9] core: install relocation script and location at the end of the build

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> The script "relocate-sdk.sh" is installed into the top directory of
> the SDK (HOST_DIR) and the SDK location path is stored in the file
> "HOST_DIR/share/buildroot/sdk-location"-
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

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

 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

> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 2a265e0..32f85e8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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):
> 

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

[Buildroot] [PATCH v7 7/9] external-toolchain: check if a buildroot SDK has already been relocated

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> 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 <wg@grandegger.com>
> ---
>  toolchain/helpers.mk                                   | 18 ++++++++++++++++++
>  toolchain/toolchain-external/pkg-toolchain-external.mk |  1 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 6136aef..5c6539e 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -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
> +( \

 The brackets here are redundant. Yes, I know, most other functions in this file
add them, but really they shouldn't.

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

> +		if [ "`cat $(1)/share/buildroot/sdk-location`" != "$${sdkroot}" ]; then \
> +			echo "Please relocate the buildroot SDK by executing \"$${sdkroot}/relocate-sdk.sh\" once!" ; \
> +			exit 1 ; \
> +		fi \
> +	fi \
> +)
> +endef
> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index 33449d3..afccbb6 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -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))" ; \
> 

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

[Buildroot] [PATCH v7 8/9] support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot

[Arnout Vandecappelle]

On 05-07-17 18:53, Wolfgang Grandegger wrote:
> This is because $(HOST_DIR)/usr is gone.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

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

 Can be applied right away.

 Regards,
 Arnout

> ---
>  support/misc/relocate-sdk.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/support/misc/relocate-sdk.sh b/support/misc/relocate-sdk.sh
> index 2a9ebc9..729353a 100755
> --- a/support/misc/relocate-sdk.sh
> +++ b/support/misc/relocate-sdk.sh
> @@ -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}")"
>  
> 

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

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Wolfgang Grandegger]

Hello Arnout,

Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
>   Hi Wolfgang,
> 
> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>> 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 <wg@grandegger.com>
> 
>   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.

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

Should I also add your "Acked-by" to the patchelf patches itself?

>> diff --git a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>> new file mode 100644
>> index 0000000..eda32e8
>> --- /dev/null
>> +++ b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>> @@ -0,0 +1,52 @@
>> +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
>> +From: Eelco Dolstra <eelco.dolstra@logicblox.com>
>> +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.

Added to the [] comment. I think the original messages should bot be 
touched.

>> +
>> +[Upstream-commit: https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
>> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> +
>> +---
>> + 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..

This is generate with quilt against patchelf-0.9.tag.bz2. Will fix.

> [snip]
>> diff --git a/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
>> new file mode 100644
>> index 0000000..ed7bb12
>> --- /dev/null
>> +++ b/package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch
>> @@ -0,0 +1,423 @@
>> +From af8d4a24a0ef613bdb47f0b1c3d962d59c53a4be Mon Sep 17 00:00:00 2001
>> +From: Wolfgang Grandegger <wg@grandegger.com>
>> +Date: Mon, 20 Feb 2017 16:29:24 +0100
>> +Subject: [PATCH] Add option to make the rpath relative under a specified root
>> + directory
> 
>   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]
>> +@@ -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.

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
> 
> 
> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>> From: Samuel Martin <s.martin49@gmail.com>
>>
>> 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 <s.martin49@gmail.com>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> 
>   Some small comments below, but again, can be fixed in follow-up patches, so
> 
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> [snip]
>> +Description:
>> +
>> +    This script scans a tree and sanitize ELF files' RPATH found in there.
>                                              ^s
>> +
>> +    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
> 
>> +
>> +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, and
> TOOLCHAIN_EXTERNAL_DOWNLOAD_INSTALL_DIR.

PATCHELF points to the patchelf utility! Or what do you mean here?

> 
> [snip]
>> +    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.
> 
>> +
>> +	    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.

Will improve that later.

> 
>> +
>> +	    # 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" )
> 
>   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!

>   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.
> 
> 
>> +    while read file ; do
>> +	# check if it's an ELF file
> 
>   Indentation is inconsistent: sometimes 4 spaces, sometimes 2 spaces (inside the

That's due to 8 spaces replaced by one tab.

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

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

I already tried that. It will make the check slower, meaning it costs 
more that calling patchelf a second time.

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

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Wolfgang Grandegger]

Am 20.07.2017 um 01:17 schrieb Arnout Vandecappelle:
> 
> 
> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>> 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.

OK, will take care later.

Wolfgang.

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Arnout Vandecappelle]

On 20-07-17 08:55, Wolfgang Grandegger wrote:
> Hello Arnout,
> 
> Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
>>   Hi Wolfgang,
>>
>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>>> 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 <wg@grandegger.com>
>>
>>   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.
> 
>> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> 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.

> 
>>> diff --git
>>> a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>> b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>> new file mode 100644
>>> index 0000000..eda32e8
>>> --- /dev/null
>>> +++ b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>> @@ -0,0 +1,52 @@
>>> +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
>>> +From: Eelco Dolstra <eelco.dolstra@logicblox.com>
>>> +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.
> 
> Added to the [] comment. I think the original messages should bot be touched.

 ACK that.

> 
>>> +
>>> +[Upstream-commit:
>>> https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
>>>
>>> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> +
>>> +---
>>> + 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..
> 
> 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]
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v7 1/9] package/patchelf: add patch for rpath sanitization under a root directory

[Wolfgang Grandegger]

Am 20.07.2017 um 09:55 schrieb Arnout Vandecappelle:
> 
> 
> On 20-07-17 08:55, Wolfgang Grandegger wrote:
>> Hello Arnout,
>>
>> Am 19.07.2017 um 23:08 schrieb Arnout Vandecappelle:
>>>    Hi Wolfgang,
>>>
>>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>>>> The patch allows to use patchelf to sanitize the rpath of the buildroot
>>>> libraries and binaries using the option "--make-rpath-relative <rootdir>".
>>>> 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 <wg@grandegger.com>
>>>
>>>    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.
>>
>>> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>> 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.

OK, makes sense.

>>
>>>> diff --git
>>>> a/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>>> b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>>> new file mode 100644
>>>> index 0000000..eda32e8
>>>> --- /dev/null
>>>> +++ b/package/patchelf/0001-Remove-apparently-incorrect-usage-of-static.patch
>>>> @@ -0,0 +1,52 @@
>>>> +From a365bcb7d7025da51b33165ef7ebc7180199a05e Mon Sep 17 00:00:00 2001
>>>> +From: Eelco Dolstra <eelco.dolstra@logicblox.com>
>>>> +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.
>>
>> Added to the [] comment. I think the original messages should bot be touched.
> 
>   ACK that.
> 
>>
>>>> +
>>>> +[Upstream-commit:
>>>> https://github.com/NixOS/patchelf/commit/a365bcb7d7025da51b33165ef7ebc7180199a05e]
>>>>
>>>> +Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>> +
>>>> +---
>>>> + 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..
>>
>> 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.

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

On 20-07-17 09:31, Wolfgang Grandegger wrote:
> Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
>>
>>
>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
[snip]
>>> +Environment:
>>> +
>>> +    PATCHELF    patchelf program to use
>>> +        (default: HOST_DIR/bin/patchelf)
>>
>>   And also HOST_DIR, TARGET_DIR, STAGING_DIR, and
>> TOOLCHAIN_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]
>>> +    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?

> 
>>   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.
>>
>>
>>> +    while read file ; do
>>> +    # check if it's an ELF file
>>
>>   Indentation 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.

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

> 
>>> +    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.
> 
> I already tried that. It will make the check slower, meaning it costs more that
> calling patchelf a second time.
> 
>>   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

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

[Buildroot] [PATCH v7 4/9] core: sanitize RPATH in target tree at the end of target finalization

[Wolfgang Grandegger]

Am 20.07.2017 um 01:34 schrieb Arnout Vandecappelle:
> 
> 
> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   Makefile | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index cdc3a64..1a826fc 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -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.

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.

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Wolfgang Grandegger]

Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
> 
> 
> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>> We introduce the "finalize-host" target for that purpose.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>>   Makefile | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 1a826fc..2a265e0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -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.

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?

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

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

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Arnout Vandecappelle]

On 20-07-17 10:40, Wolfgang Grandegger wrote:
> 
> 
> Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
>>
>>
>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>>> We introduce the "finalize-host" target for that purpose.
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> ---
>>>   Makefile | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 1a826fc..2a265e0 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -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.
> 
> 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...

> Could you please send me your "defconfig" to check the times here?

 It's just an external toolchain + kernel + BR2_INIT_SYSTEMD=y


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


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

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

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

[Buildroot] [PATCH v7 4/9] core: sanitize RPATH in target tree at the end of target finalization

[Arnout Vandecappelle]

On 20-07-17 10:25, Wolfgang Grandegger wrote:
> 
> 
> Am 20.07.2017 um 01:34 schrieb Arnout Vandecappelle:
>>
>>
>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>> ---
>>>   Makefile | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index cdc3a64..1a826fc 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -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.
> 
> 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


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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Am 20.07.2017 um 10:15 schrieb Arnout Vandecappelle:
> 
> 
> On 20-07-17 09:31, Wolfgang Grandegger wrote:
>> Am 20.07.2017 um 01:02 schrieb Arnout Vandecappelle:
>>>
>>>
>>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
> [snip]
>>>> +Environment:
>>>> +
>>>> +    PATCHELF    patchelf program to use
>>>> +        (default: HOST_DIR/bin/patchelf)
>>>
>>>    And also HOST_DIR, TARGET_DIR, STAGING_DIR, and
>>> TOOLCHAIN_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.

Got it!

> [snip]
>>>> +    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?

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

        ...
    esac


> 
>>
>>>    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.
>>>
>>>
>>>> +    while read file ; do
>>>> +    # check if it's an ELF file
>>>
>>>    Indentation 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.
> 
>>
>>> 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.
> 
>>
>>>> +    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.
>>
>> I already tried that. It will make the check slower, meaning it costs more that
>> calling patchelf a second time.
>>
>>>    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).

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

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Wolfgang Grandegger]

Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:
> 
> 
> On 20-07-17 10:40, Wolfgang Grandegger wrote:
>>
>>
>> Am 20.07.2017 um 01:51 schrieb Arnout Vandecappelle:
>>>
>>>
>>> On 05-07-17 18:53, Wolfgang Grandegger wrote:
>>>> We introduce the "finalize-host" target for that purpose.
>>>>
>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>>>> ---
>>>>    Makefile | 7 ++++++-
>>>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 1a826fc..2a265e0 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -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.
>>
>> 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...
> 
>> Could you please send me your "defconfig" to check the times here?
> 
>   It's just an external toolchain + kernel + BR2_INIT_SYSTEMD=y
> 
> 
>>>    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).

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.

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Arnout Vandecappelle]

On 20-07-17 11:15, Wolfgang Grandegger wrote:
> 
> 
> Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:

[snip]
>>   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

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Thomas Petazzoni]

Hello,

On Thu, 20 Jul 2017 01:51:53 +0200, Arnout Vandecappelle wrote:

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

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

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

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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

On 20-07-17 11:03, Wolfgang Grandegger wrote:
>     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 .)

>             sanitize_extra_args+=( "--relative-to-file" )
>             ;;
> 
>         staging)
>             ...
>             # should be like for the target tree below

 s/like/the same as/

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

> The attached patch speeds up ELF file checking. As said, readelf
> is still faster even with that patch.
[snip]

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Thomas Petazzoni]

Hello,

On Thu, 20 Jul 2017 12:45:38 +0200, Arnout Vandecappelle wrote:

> > Am 20.07.2017 um 10:49 schrieb Arnout Vandecappelle:  
> 
> [snip]
> >>   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!

No, I don't agree here. Why would the staging sanitization be related
to doing the SDK ?

Best regards,

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

[Buildroot] [PATCH v7 8/9] support/scripts: relocate-sdk.sh now creates sdk-location in share/buildroot

[Thomas Petazzoni]

Hello,

On Wed,  5 Jul 2017 18:53:13 +0200, Wolfgang Grandegger wrote:
> This is because $(HOST_DIR)/usr is gone.
> 
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
>  support/misc/relocate-sdk.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to master, thanks.

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

[Buildroot] [PATCH v7 0/9] Make the SDK relocatable

[Thomas Petazzoni]

Hello,

On Wed,  5 Jul 2017 18:53:05 +0200, Wolfgang Grandegger wrote:

> 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

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Arnout Vandecappelle]

On 20-07-17 15:44, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 20 Jul 2017 01:51:53 +0200, Arnout Vandecappelle wrote:
[snip]
>>  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.


> Basically, sdk needs to depend on "world".

 Not really, it doesn't need to depend on target-finalize or the rootfs targets.

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

> 
> Best regards,
> 
> Thomas
> 

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Thomas Petazzoni]

Hello,

On Thu, 20 Jul 2017 15:58:49 +0200, Arnout Vandecappelle wrote:

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

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

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

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

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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Hello Arnout,

Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
> 
> 
> On 20-07-17 11:03, Wolfgang Grandegger wrote:
>>      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.

> 
>>              sanitize_extra_args+=( "--relative-to-file" )
>>              ;;
>>
>>          staging)
>>              ...
>>              # should be like for the target tree below
> 
>   s/like/the same as/
> 
>>              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]
>> 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!

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

[Buildroot] [PATCH v7 5/9] core: sanitize RPATH in host tree at the very end of the build

[Arnout Vandecappelle]

On 20-07-17 16:06, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 20 Jul 2017 15:58:49 +0200, Arnout Vandecappelle wrote:
[snip]
>>> 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]
> 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

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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

On 20-07-17 16:08, Wolfgang Grandegger wrote:
> Hello Arnout,
> 
> Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
>>
>>
>> On 20-07-17 11:03, Wolfgang Grandegger wrote:
>>>      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.

 There is no original script, only an original author :-)

 So indeed, fix it script-wide please.

[snip]
>>> 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.


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


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

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

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Wolfgang Grandegger]

Am 20.07.2017 um 16:19 schrieb Arnout Vandecappelle:
> 
> 
> On 20-07-17 16:08, Wolfgang Grandegger wrote:
>> Hello Arnout,
>>
>> Am 20.07.2017 um 15:45 schrieb Arnout Vandecappelle:
>>>
>>>
>>> On 20-07-17 11:03, Wolfgang Grandegger wrote:
>>>>       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.
> 
>   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.

>   So indeed, fix it script-wide please.
> 
> [snip]
>>>> 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.
> 
> 
>> "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.

[Buildroot] [PATCH v7 2/9] support/scripts: add fix-rpath script to sanitize the rpath

[Arnout Vandecappelle]

On 20-07-17 18:50, Wolfgang Grandegger wrote:
> 
> 
> Am 20.07.2017 um 16:19 schrieb Arnout Vandecappelle:
[snip]
>>   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

> My time
> working on this topic is limited! When I have more free time I will take care.
> 
> Wolfgang.

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