All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks
@ 2016-03-09 22:58 Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation Samuel Martin
                   ` (17 more replies)
  0 siblings, 18 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

Hi all,


Here is yet another round of the series aiming improving the relocatability
of the SDK built by Buildroot.

The new round improves the shell implementation started in the previous
submission.


In details:

Patches 1 to 3:
  Clean up and preparatory work for the relocatable SDK series and also helping
  to track build machine leaks into host, target and staging trees.

Patches 4 to 7:
  Make some pieces of the Buildroot SDK relocatable (limited to some symlinks
  and *-config scripts).

Patches 8 to 10:
  Add the fix-rpath script, then run it on the Buildroot host, target and
  staging trees.

Patch 11:
  Clean-up speex package WRT its RPATH hook.

Patch 12:
  Add a post-install hook to the toolchain package to render it relocatable.

Patch 13 to 15:
  Update and fix the check-host-rpath to re-use the shell helpers.

Patches 16 and 17:
  Add means to track build machine leaks into the host, target and staging
  trees. These patches will allow to identify what remains to be fixed WRT
  build machine leaks.

Patches 18:
  Update documentation about how to enable/adjust log level on script using
  the shell modules introduced in patch 8/18.


Worth noting:

* patches 4, 5, 6 and 7 fix scripts or wrappers WRT improving the SDK
  relocatability;

* patches 8 and 16 add new scripts (which are not executed without the
  patches 9, 10 and 12).

* patches 9, 10 and 12 add make targets called on all builds, improving the
  SDK relocatability; patch 17 also add make targets, but they are not in as
  dependency of any other, so must be invoke explicitly.

* all others patches are fairly trivial fixes or improvements.


Future works:

* Handling autotools/libtools relocation

* Analysing and fixing leaks and relocation remaining issues


Regards,
Samuel
--

Samuel Martin (18):
  package/linux-headers: cleanup installation
  core: use $(CURDIR) to set TOPDIR
  core: re-enter make if $(CURDIR) or $(O) are not absolute canonical
    path
  core: staging symlink uses a relative path when possible
  core: make staging *-config scripts relocatable
  core: make host *-config scripts relocatable
  package/pkgconf: make the pkg-config wrapper relocatable
  support/scripts: add fix-rpath script + a bunch of helpers
  core: add HOST_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS
  core: add {TARGET,STAGING}_SANITIZE_RPATH_HOOK to
    TARGET_FINALIZE_HOOKS
  package/speex: remove no longer needed hook
  toolchain: add post-install hooks making the toolchain relocatable
  support/scripts: update check-host-rpath to use the shell helpers
  support/scripts/check-host-rpath: silent find command
  support/scripts/check-host-rpath: also check HOST_DIR/{bin,sbin}
  support/scripts: add check-host-leaks script + all needed helpers
  core: add check-leaks-in-{target,host,staging} targets
  docs/manual: document how to debug shell script

 Makefile                               |  76 +++++++-
 docs/manual/debugging-buildroot.txt    |  15 ++
 package/linux-headers/linux-headers.mk |   1 +
 package/pkg-generic.mk                 |  14 +-
 package/pkg-utils.mk                   |   5 +
 package/pkgconf/pkg-config.in          |   4 +-
 package/pkgconf/pkgconf.mk             |   8 +-
 package/speex/speex.mk                 |   5 -
 support/scripts/check-host-leaks       |  63 +++++++
 support/scripts/check-host-rpath       |  71 +++----
 support/scripts/fix-rpath              | 116 ++++++++++++
 support/scripts/shell/log.sh           |  61 ++++++
 support/scripts/shell/patchelf.sh      | 178 ++++++++++++++++++
 support/scripts/shell/readelf.sh       | 328 +++++++++++++++++++++++++++++++++
 support/scripts/shell/sdk.sh           | 138 ++++++++++++++
 support/scripts/shell/source.sh        |  77 ++++++++
 support/scripts/shell/utils.sh         |  76 ++++++++
 toolchain/toolchain/toolchain.mk       |   5 +
 18 files changed, 1183 insertions(+), 58 deletions(-)
 mode change 100644 => 100755 package/pkgconf/pkg-config.in
 create mode 100755 support/scripts/check-host-leaks
 create mode 100755 support/scripts/fix-rpath
 create mode 100644 support/scripts/shell/log.sh
 create mode 100644 support/scripts/shell/patchelf.sh
 create mode 100644 support/scripts/shell/readelf.sh
 create mode 100644 support/scripts/shell/sdk.sh
 create mode 100644 support/scripts/shell/source.sh
 create mode 100644 support/scripts/shell/utils.sh

--
2.7.2

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

* [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 23:09   ` Yann E. MORIN
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This patch removes unnecessary files generated during installation, that
also are causing build machine paths leaking into the Buildroot's SDK.

Note that similar cleanup commands can be found in other distributions
(e.g. gentoo [1]); thought this is not a valid reason.

[1] https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-kernel/linux-headers/linux-headers-4.4.ebuild#n36

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- improve commit log

changes v5->v6:
- new patch
---
 package/linux-headers/linux-headers.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
index 0900778..7633257 100644
--- a/package/linux-headers/linux-headers.mk
+++ b/package/linux-headers/linux-headers.mk
@@ -123,6 +123,7 @@ define LINUX_HEADERS_INSTALL_STAGING_CMDS
 			HOSTCXX="$(HOSTCXX)" \
 			INSTALL_HDR_PATH=$(STAGING_DIR)/usr \
 			headers_install)
+	find $(STAGING_DIR)/usr \( -name .install -o -name ..install.cmd \) -delete
 endef
 
 ifeq ($(BR2_KERNEL_HEADERS_VERSION)$(BR2_KERNEL_HEADERS_AS_KERNEL),y)
-- 
2.7.2

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

* [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 23:10   ` Yann E. MORIN
                     ` (2 more replies)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path Samuel Martin
                   ` (15 subsequent siblings)
  17 siblings, 3 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This changes saves a shell call and uses a variable automatically set
by make [1].

[1] http://www.gnu.org/software/make/manual/make.html#Quick-Reference

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- new patch
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f2822a2..98c8dc7 100644
--- a/Makefile
+++ b/Makefile
@@ -69,7 +69,7 @@ endif
 .NOTPARALLEL:
 
 # absolute path
-TOPDIR := $(shell pwd)
+TOPDIR := $(CURDIR)
 CONFIG_CONFIG_IN = Config.in
 CONFIG = support/kconfig
 DATE := $(shell date +%Y%m%d)
-- 
2.7.2

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 23:15   ` Yann E. MORIN
       [not found]   ` <CAFt09wNvtXGcfZ3PFZOgGu+bDvp6mEr44uwxa_ryjeP6DDQKBQ@mail.gmail.com>
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible Samuel Martin
                   ` (14 subsequent siblings)
  17 siblings, 2 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
they can be resolved differently, depending on each package build-system
(whether it uses the given paths or get the absolute canonical ones).

Thus, to make easier tracking down host machine paths leaking into the
host, target or staging trees, the CURDIR and O variables are set to
their absolute canonical paths.

Note that this change takes care of the makefile wrapper installed in
$(O) to avoid unneeded make recursion.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- new patch
---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 98c8dc7..52a92f6 100644
--- a/Makefile
+++ b/Makefile
@@ -26,16 +26,30 @@
 
 # Trick for always running with a fixed umask
 UMASK = 0022
-ifneq ($(shell umask),$(UMASK))
+
+# Check if we need to re-enter make for one or several of the following reasons:
+# 1- Wrong (too restrictive) umask:
+#    This prevents Buildroot and packages from creating files and directories.
+# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
+#    This makes harder tracking and fixing host machine path leaks.
+# 3- Output location (i.e. $(O)) not being the absolute canonical path:
+#    This makes harder tracking and fixing host machine path leaks.
+#
+# Note:
+# - remove the trailing '/.' from $(O) as it can be added by the makefile
+#   wrapper installed in the $(O).
+ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
 .PHONY: _all $(MAKECMDGOALS)
 
 $(MAKECMDGOALS): _all
 	@:
 
 _all:
-	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
+	umask $(UMASK) && \
+		$(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
+			$(MAKECMDGOALS) O=$(realpath $(O))
 
-else # umask
+else # umask / $(CURDIR) / $(O)
 
 # This is our default rule, so must come first
 all:
@@ -1007,4 +1021,4 @@ include docs/manual/manual.mk
 
 .PHONY: $(noconfig_targets)
 
-endif #umask
+endif #umask / $(CURDIR) / $(O)
-- 
2.7.2

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

* [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (2 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 23:20   ` Yann E. MORIN
  2016-03-10 20:47   ` Thomas Petazzoni
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable Samuel Martin
                   ` (13 subsequent siblings)
  17 siblings, 2 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This change turns the staging/ symlink to a relative one when
$(STAGING_DIR) points to some location in the Buildroot's output
directory (i.e.: in $(BASE_DIR)/).

In any other case, the symlink target remains the absolute path
to the sysroot location.

This slight change is a step forward toward a relocatable SDK.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- improve commit log (ThomasP)

changes v5->v6:
- no changes

changes v4->v5:
- add support when BR2_HOST_DIR is set somewhere out of BASE_DIR

changes v3->v4:
- no change

changes v2->v3:
- no change
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 52a92f6..f1de4ed 100644
--- a/Makefile
+++ b/Makefile
@@ -518,7 +518,11 @@ $(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIS
 # Populating the staging with the base directories is handled by the skeleton package
 $(STAGING_DIR):
 	@mkdir -p $(STAGING_DIR)
+ifeq ($(BASE_DIR)/host/$(STAGING_SUBDIR),$(STAGING_DIR))
+	@ln -snf host/$(STAGING_SUBDIR) $(BASE_DIR)/staging
+else
 	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
+endif
 
 RSYNC_VCS_EXCLUSIONS = \
 	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
-- 
2.7.2

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

* [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (3 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 23:24   ` Yann E. MORIN
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 06/18] core: make host " Samuel Martin
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix computed
relatively to the script location.

This patch hook only fixes *-config scripts located in the staging area,
the target ones are already removed. A follow-up change will fix those
from the HOST_DIR location.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- new patch
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3904c09..ffa21ee 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -240,7 +240,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 		$(call MESSAGE,"Fixing package configuration files") ;\
 			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
 				-e "s,$(STAGING_DIR), at STAGING_DIR@,g" \
-				-e "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
+				-e "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \
 				-e "s,-I/usr/,-I at STAGING_DIR@/usr/,g" \
 				-e "s,-L/usr/,-L at STAGING_DIR@/usr/,g" \
 				-e "s, at STAGING_DIR@,$(STAGING_DIR),g" \
-- 
2.7.2

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

* [Buildroot] [PATCH v7 06/18] core: make host *-config scripts relocatable
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (4 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-10  0:12   ` Arnout Vandecappelle
  2016-03-10 20:50   ` Thomas Petazzoni
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable Samuel Martin
                   ` (11 subsequent siblings)
  17 siblings, 2 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix computed
relatively to the script location.

This patch only fixes *-config scripts located in the host area, those
from the target and staging area are already handled.

This hook is a kind of brute-force sed on all files matching '*-config'
in the $(HOST_DIR)/usr/bin directory. This allows correctly handle
script whose name may depends on the host system, not the target
configuration (e.g. ncurses can be configured for the target to be
ncurses6w, so ncurses6w-config ; whereas for the host it will be
ncurses5, so ncurses5-config).
Since there way less host packages than target ones, it is reasonable to
assume that processing all *-config scripts found when a host package
(whose target package set the *_CONFIG_SCRIPTS variable) is fairly short
and does not add its overhead not even noticeable.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- new patch
---
 package/pkg-generic.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index ffa21ee..845c8d3 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -207,6 +207,18 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
+	$(Q)if test -n "$($(RAWNAME)_CONFIG_SCRIPTS)" ; then \
+		$(call MESSAGE,"Fixing package configuration files") ;\
+			ls $(HOST_DIR)/usr/bin/*-config 2>/dev/null |\
+			xargs --no-run-if-empty \
+			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
+				-e "s,$(HOST_DIR), at HOST_DIR@,g" \
+				-e "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \
+				-e "s,-I/usr/,-I at HOST_DIR@/usr/,g" \
+				-e "s,-L/usr/,-L at HOST_DIR@/usr/,g" \
+				-e "s, at HOST_DIR@,$(HOST_DIR),g" \
+				-e "s, at BASE_DIR@,$(BASE_DIR),g" ;\
+	fi
 	@$(call step_end,install-host)
 	$(Q)touch $@
 
-- 
2.7.2

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

* [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (5 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 06/18] core: make host " Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-10 20:53   ` Thomas Petazzoni
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 08/18] support/scripts: add fix-rpath script + a bunch of helpers Samuel Martin
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This change refactors the pkg-config wrapper template and the way it is filled
in a way there is no longer hard-coded paths to the staging directory; so it
is now fully relocatable.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
change v6->v7:
- new patch
---
 package/pkgconf/pkg-config.in | 4 +++-
 package/pkgconf/pkgconf.mk    | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 package/pkgconf/pkg-config.in

diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
old mode 100644
new mode 100755
index 4dec487..aa4e484
--- a/package/pkgconf/pkg-config.in
+++ b/package/pkgconf/pkg-config.in
@@ -1,2 +1,4 @@
 #!/bin/sh
-PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:- at PKG_CONFIG_LIBDIR@} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:- at STAGING_DIR@} $(dirname $0)/pkgconf @STATIC@ $@
+STAGING_DIR="$(readlink -f "$(dirname "$0")/../..")/@STAGING_SUBDIR@"
+TARGET_PKG_CONFIG_LIBDIR="$( echo @PKG_CONFIG_LIBDIR@ | sed -e "s@^@${STAGING_DIR}/@ ; s@:@:${STAGING_DIR}/@g" )"
+PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${TARGET_PKG_CONFIG_LIBDIR}} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${STAGING_DIR}} $(dirname $0)/pkgconf @STATIC@ $@
diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
index c8b0cba..d27672c 100644
--- a/package/pkgconf/pkgconf.mk
+++ b/package/pkgconf/pkgconf.mk
@@ -19,8 +19,12 @@ endef
 define HOST_PKGCONF_INSTALL_WRAPPER
 	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
 		$(HOST_DIR)/usr/bin/pkg-config
-	$(SED) 's, at PKG_CONFIG_LIBDIR@,$(STAGING_DIR)/usr/lib/pkgconfig:$(STAGING_DIR)/usr/share/pkgconfig,' \
-		-e 's, at STAGING_DIR@,$(STAGING_DIR),' \
+	# @PKG_CONFIG_LIBDIR@ is a colon-separated list of paths to search.
+	# These paths are relative to the root of the staging directory; they will
+	# be automatically prefixed with the staging directory path by the wrapper
+	# at runtime to ensure its relocatability.
+	$(SED) 's, at PKG_CONFIG_LIBDIR@,/usr/lib/pkgconfig:/usr/share/pkgconfig,' \
+		-e 's, at STAGING_SUBDIR@,$(STAGING_SUBDIR),' \
 		$(HOST_DIR)/usr/bin/pkg-config
 endef
 
-- 
2.7.2

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

* [Buildroot] [PATCH v7 08/18] support/scripts: add fix-rpath script + a bunch of helpers
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (6 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 09/18] core: add HOST_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS Samuel Martin
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This commit introduces a fix-rpath shell-script able to scan a tree,
detect ELF files, check their RPATH and fix it in a proper way.

Along to the fix-rpath script, it also adds a bunch of shell helper
functions grouped into modules. This will help writing scripts handling
RPATH and other things, while allowing to share and reuse these
functions between scripts.

These helpers are namespaced within the filename of the module in which
they are gathered.

This change adds 6 modules:
- source.sh: provides simple helper to easily source another module, taking
    care of not sourcing again an already-sourced one;
- log.sh: provides logging helpers;
- utils.sh: provides simple functions to filter ELF files in a list;
- readelf.sh: provides functions retrieving ELF details from a file;
- patchelf.sh: provides function updating ELF files;
- sdk.sh: provides RPATH computation functions.

These 6 modules are used by the fix-rpath script.
Follow-up patches will make some scripts leveraging these modules.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7 (includes changes started during BR dev. days):
- log module:
  - debug env. var. renamed: DEBUG -> SHELL_DEBUG
  - trace now prints the message level
- patchelf module:
  - get rid of XRPATH/XORIGIN stuff (Arnout)
  - set_rpath now handles ELF files with no .dynamic section
  - cleanup and minor fix in sanitize_rpath
- readelf module:
  - move {filter,is}_elf* functions from utils to readelf modules
  - add list_sections and has_section functions (needed in
    sanitize_rpath)
- sdk modules:
  - get rid of XRPATH/XORIGIN stuff (Arnout)
  - minor functions doc. fixes
- source module:
  - move "Could not load module" error message in load_module (Arnout)
- utils module:
  - move {filter,is}_elf* functions from utils to readelf modules
  - add list_has function
- fix-rpath:
  - runs with "set -e" (exit on error)
  - improve help text
  - slight refactoring because of modules changes

changes v5->v6:
- fully rewritten in shell

changes v4->v5:
- add verbose support
- rename shrink_rpath -> clear_rpath
- add sanitize_rpath function

changes v3->v4:
- fix typos and license (Baruch)

changes v2->v3:
- no change
---
 support/scripts/fix-rpath         | 116 +++++++++++++++++++++++++
 support/scripts/shell/log.sh      |  61 +++++++++++++
 support/scripts/shell/patchelf.sh | 178 ++++++++++++++++++++++++++++++++++++++
 support/scripts/shell/readelf.sh  | 174 +++++++++++++++++++++++++++++++++++++
 support/scripts/shell/sdk.sh      |  68 +++++++++++++++
 support/scripts/shell/source.sh   |  77 +++++++++++++++++
 support/scripts/shell/utils.sh    |  60 +++++++++++++
 7 files changed, 734 insertions(+)
 create mode 100755 support/scripts/fix-rpath
 create mode 100644 support/scripts/shell/log.sh
 create mode 100644 support/scripts/shell/patchelf.sh
 create mode 100644 support/scripts/shell/readelf.sh
 create mode 100644 support/scripts/shell/sdk.sh
 create mode 100644 support/scripts/shell/source.sh
 create mode 100644 support/scripts/shell/utils.sh

diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
new file mode 100755
index 0000000..2ef89ab
--- /dev/null
+++ b/support/scripts/fix-rpath
@@ -0,0 +1,116 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+set -e
+
+usage() {
+  cat <<EOF >&2
+Usage:  ${0} TREE_KIND TREE_ROOT
+
+Description:
+
+        This script scans a tree and sanitize ELF files' RPATH found in there.
+
+        Sanitization behaves the same whatever the kindd of the processed tree, but
+        the resulting RPATH differs.
+
+        Sanitization action:
+        - remove RPATH pointing outside of the tree
+        - for RPATH pointing in the tree:
+          - if they point to standard location (/lib, /usr/lib): remove them
+          - otherwise: make them relative using \$ORIGIN
+
+        For the target tree:
+        - scan the whole tree for sanitization
+
+        For the staging tree :
+        - scan the whole tree for sanitization
+
+        For the host tree:
+        - skip the staging tree for sanitization
+        - add \$HOST_DIR/{lib,usr/lib} to RPATH (as relative pathes)
+
+Arguments:
+
+        TREE_KIND   Kind of tree to be processed.
+                    Allowed values: host, target, staging
+
+        TREE_ROOT   Path to the root of the tree to be scaned
+
+Environment:
+
+        HOST_READELF    readelf program to use for host ELF files
+                        (default: readelf)
+
+        HOST_READELF    readelf program to use for host ELF files
+                        (default: readelf)
+
+        PATCHELF        patchelf program to use
+                        (default: patchelf)
+EOF
+}
+
+source "${0%/*}/shell/source.sh"
+
+source.load_module readelf
+source.load_module patchelf
+
+: ${HOST_READELF:=readelf}
+: ${TARGET_READELF:=readelf}
+: ${PATCHELF:=patchelf}
+
+main() {
+    local tree="${1}"
+    local basedir="$( readlink -f "${2}" )"
+
+    local find_args=( "${basedir}" )
+    local sanitize_extra_args=()
+    local readelf
+
+    case "${tree}" in
+        host)
+            # do not process the sysroot (only contains target binaries)
+            find_args+=( "-name" "sysroot" "-prune" "-o" )
+
+            # do not process the external toolchain installation directory to
+            # to avoid breaking it.
+            find_args+=( "-path" "*/opt/ext-toolchain" "-prune" "-o" )
+
+            # make sure RPATH will point to ${hostdir}/lib and ${hostdir}/usr/lib
+            sanitize_extra_args+=( "keep_lib_and_usr_lib" )
+
+            readelf="${HOST_READELF}"
+            ;;
+        staging|target)
+            readelf="${TARGET_READELF}"
+            ;;
+        *)
+            usage
+            exit 1
+            ;;
+    esac
+
+    find_args+=( "-type" "f" "-print" )
+
+    while read file ; do
+        READELF="${readelf}" PATCHELF="${PATCHELF}" \
+            patchelf.sanitize_rpath "${basedir}" "${file}" ${sanitize_extra_args[@]}
+    done < <( find ${find_args[@]} | readelf.filter_elf )
+}
+
+main ${@}
diff --git a/support/scripts/shell/log.sh b/support/scripts/shell/log.sh
new file mode 100644
index 0000000..ada2553
--- /dev/null
+++ b/support/scripts/shell/log.sh
@@ -0,0 +1,61 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Logging helpers
+#
+# This module defines the following functions:
+#   log.trace
+#   log.debug
+#   log.info
+#   log.warn
+#   log.errorN
+#   log.error
+#
+# This module sets the following variables:
+#   my_name
+#
+# This module is sensitive to the following environment variables:
+#   DEBUG
+
+source.declare_module log
+
+# Debug level:
+# - 0 or empty: only show errors
+# - 1         : show errors and warnings
+# - 2         : show errors, warnings, and info
+# - 3         : show errors, warnings, info and debug
+: ${SHELL_DEBUG:=0}
+
+# Low level utility function
+log.trace()  {
+    local level="${1}" msg="${2}"
+    shift 2
+    printf "[%-5s] %s: ${msg}" "${level:0:5}" "${my_name}" "${@}"
+}
+
+# Public logging functions
+log.debug()  { :; }
+[ ${SHELL_DEBUG} -lt 3 ] || log.debug() { log.trace DEBUG "${@}" >&2; }
+log.info()   { :; }
+[ ${SHELL_DEBUG} -lt 2 ] || log.info()  { log.trace INFO "${@}" >&2; }
+log.warn()   { :; }
+[ ${SHELL_DEBUG} -lt 1 ] || log.warn()  { log.trace WARN "${@}" >&2; }
+log.errorN() { local ret="${1}" ; shift ; log.trace ERROR "${@}" ; exit ${ret} ; }
+log.error()  { log.errorN 1 "${@}"; }
+
+# Program name
+my_name="${0##*/}"
+
diff --git a/support/scripts/shell/patchelf.sh b/support/scripts/shell/patchelf.sh
new file mode 100644
index 0000000..8da9c07
--- /dev/null
+++ b/support/scripts/shell/patchelf.sh
@@ -0,0 +1,178 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Patchelf helpers
+#
+# This module defines the following functions:
+#   patchelf.set_rpath
+#   patchelf.update_rpath
+#   patchelf.sanitize_rpath
+#
+# This module is sensitive to the following environment variables:
+#   PATCHELF
+#   READELF
+
+source.declare_module patchelf
+
+source.load_module log
+source.load_module sdk
+source.load_module utils
+source.load_module readelf
+
+: ${PATCHELF:=patchelf}
+
+# patchelf.set_xrpath file rpath...
+#
+# Set RPATH in $file.
+# Automatically join all RPATH with the correct separator.
+#
+# file  : ELF file path
+# rpath : RPATH element
+#
+# environment:
+#   PATCHELF: patchelf program path
+patchelf.set_rpath() {
+    local file="${1}"
+    shift
+    local rpath="$(sed -e 's/ +/:/g' <<<"${@}")"
+    # Sanity check: patchelf needs the ELF file to have a .dynamic section.
+    # So, check for it and behaves in a proper way:
+    # - returns immediatly if no .dynamic section, and RPATH is empty;
+    # - bail out if no .dynamic section and RPATH is not empty.
+    if ! readelf.has_section "${file}" '.dynamic' ; then
+        if test -z "${rpath}" ; then
+            return 0
+        else
+            local fmt="Trying to set a RPATH to a ELF file with no .dynamic section\n"
+            fmt="${fmt}\tfile : %s\n"
+            fmt="${fmt}\tRPATH: %s\n"
+            log.error "${fmt}" "${file}" "${rpath}"
+        fi
+    fi
+    "${PATCHELF}" --set-rpath "${rpath}" "${file}"
+}
+
+# patchelf.update_rpath basedir binary libdirs...
+#
+# Set RPATH in $binary computing them from the paths $libdirs (and $basedir).
+# Existing RPATH in $file will be overwritten if any.
+#
+# basedir : absolute path of the tree in which the $bindir and $libdirs must be
+# binary  : ELF file absolute path
+# libdirs : list of library location (absolute paths)
+#
+# environment:
+#   PATCHELF: patchelf program path
+patchelf.update_rpath() {
+    local basedir="${1}"
+    local binary="${2}"
+    shift 2
+    local libdirs=( ${@} )
+    log.debug "  basedir: %s\n" "${basedir}"
+    log.debug "      elf: %s\n" "${binary}"
+    log.debug "  libdirs: %s\n" "${libdirs[*]}"
+    log.info  "    rpath: %s\n" \
+        "$( sdk.compute_rpath "${basedir}" "${binary%/*}" ${libdirs[@]} )"
+    patchelf.set_rpath "${binary}" \
+        "$( sdk.compute_rpath "${basedir}" "${binary%/*}" ${libdirs[@]} )"
+}
+
+# patchelf.sanitize_rpath basedir binary [keep_lib_usr_lib]
+#
+# Scan $binary's RPATH, remove any of them pointing outside of $basedir.
+# If $keep_lib_usr_lib in not empty, the library directories $basedir/lib and
+# $basedir/usr/lib will be added to the RPATH.
+#
+# Note:
+#     Absolute paths is needed to correctly handle symlinks and or mount-bind in
+#     the $basedir path.
+#
+# basedir          : absolute path of the tree in which the $bindir and $libdirs
+#                    must be
+# binary           : ELF file absolute path
+# keep_lib_usr_lib : add to RPATH $basedir/lib and $basedir/usr/lib
+#
+# environment:
+#   PATCHELF: patchelf program path
+#   READELF : readelf program path
+patchelf.sanitize_rpath() {
+    local basedir="$( readlink -f "${1}" )"
+    local binary="${2}"
+    local keep_lib_usr_lib="${3}"
+
+    readelf.is_elf_shared_object "${binary}" ||
+        readelf.is_elf_executable "${binary}" ||
+            return 0
+
+    local path abspath rpath
+    local libdirs=()
+
+    if test -n "${keep_lib_usr_lib}" ; then
+        libdirs+=( "${basedir}/lib" "${basedir}/usr/lib" )
+    fi
+
+    log.info "ELF: %s\n" "${binary}"
+
+    local rpaths="$( readelf.get_rpath "${binary}" )"
+
+    for rpath in ${rpaths//:/ } ; do
+        # figure out if we should keep or discard the path; there are several
+        # cases to handled:
+        # - $path starts with "$ORIGIN":
+        #     The original build-system already took care of setting a relative
+        #     RPATH, resolve it and test if it is worthwhile to keep it;
+        # - $basedir/$path exists:
+        #     The original build-system already took care of setting an absolute
+        #     RPATH (absolute in the final rootfs), resolve it and test if it is
+        #     worthwhile to keep it;
+        # - $path start with $basedir:
+        #     The original build-system added some absolute RPATH (absolute on
+        #     the build machine). While this is wrong, it can still be fixed; so
+        #     test if it is worthwhile to keep it;
+        # - $path points somewhere else:
+        #     (can be anywhere: build trees, staging tree, host location,
+        #     non-existing location, etc.)
+        #     Just discard such a path.
+        if grep -q '^$ORIGIN/' <<<"${rpath}" ; then
+            path="${binary%/*}/${rpath#*ORIGIN/}"
+        elif test -e "${basedir}/${rpath}" ; then
+            path="${basedir}/${rpath}"
+        elif grep -q "^${basedir}/" <<<"$( readlink -f "${rpath}" )" ; then
+            path="${rpath}"
+        else
+            log.debug "\tDROPPED [out-of-tree]: %s\n" "${rpath}"
+            continue
+        fi
+
+        abspath="$( readlink -f "${path}" )"
+
+        # discard path pointing to default locations handled by ld-linux
+        if grep -qE "^${basedir}/(lib|usr/lib)$" <<<"${abspath}" ; then
+            log.debug \
+                "\tDROPPED [std libdirs]: %s (%s)\n" "${rpath}" "${abspath}"
+            continue
+        fi
+
+        log.debug "\tKEPT %s (%s)\n" "${rpath}" "${abspath}"
+
+        libdirs+=( "${abspath}" )
+
+    done
+
+    libdirs=( $( utils.list_reduce ${libdirs[@]} ) )
+
+    patchelf.update_rpath "${basedir}" "${binary}" ${libdirs[@]}
+}
diff --git a/support/scripts/shell/readelf.sh b/support/scripts/shell/readelf.sh
new file mode 100644
index 0000000..541b3e1
--- /dev/null
+++ b/support/scripts/shell/readelf.sh
@@ -0,0 +1,174 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Readelf helpers
+#
+# This module defines the following functions:
+#   readelf._filter_elf_regexp
+#   readelf.filter_elf
+#   readelf.filter_elf_executable
+#   readelf.filter_elf_shared_object
+#   readelf.is_elf_executable
+#   readelf.is_elf_shared_object
+#   readelf.get_rpath
+#   readelf.list_sections
+#   readelf.has_section
+#
+# This module is sensitive to the following environment variables:
+#   READELF
+source.declare_module readelf
+
+# When calling readelf(1) program, the user's locale will be overriden with the
+# C locale, so we are sure we can reliably parse its output.
+: ${READELF:=readelf}
+
+# readelf._filter_elf_regexp filter_cmd file...
+#
+# Filters ELF files WRT the given regular extended expression.
+# This funtion can take one or several files, or read them from stdin.
+#
+# filter_cmd : filter command (usually based on grep)
+# file       : list of files to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf._filter_elf_regexp() {
+    local regexp="${1}"
+    shift
+    local in file
+    test ${#} -gt 0 && in='printf "%s\n" "${@}"' || in='dd 2>/dev/null'
+    eval "${in}" |
+        while read file ; do
+            LC_ALL=C ${READELF} -h "${file}" 2>/dev/null |
+                grep -qE "${regexp}" ||
+                    continue
+            printf "%s\n" "${file}"
+        done
+}
+
+# readelf.filter_elf file...
+#
+# Filters ELF files; if $file is an ELF file, $file is printed, else it is
+# discarded.
+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf.filter_elf() {
+    readelf._filter_elf_regexp "Class:\s+ELF" "${@}"
+}
+
+# readelf.filter_elf_shared_object file...
+#
+# Filters ELF files; if $file is an ELF file, $file is printed, else it is
+# discarded.
+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf.filter_elf_shared_object() {
+    readelf._filter_elf_regexp "Type:\s+DYN\s\(Shared\sobject\sfile\)" "${@}"
+}
+
+# readelf.filter_elf_executable file...
+#
+# Filters ELF files; if $file is an ELF file, $file is printed, else it is
+# discarded.
+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf.filter_elf_executable() {
+    readelf._filter_elf_regexp "Type:\s+EXEC\s\(Executable\sfile\)" "${@}"
+}
+
+# readelf.is_elf_shared_object file
+#
+# Returns 0 if $file is an ELF file, non-0 otherwise.
+#
+# file : path of file to be tested
+#
+# environment:
+#   READELF: readelf program path
+readelf.is_elf_shared_object() {
+    test "$( readelf.filter_elf_shared_object "${1}" )" != ""
+}
+
+# readelf.is_elf_executable file
+#
+# Returns 0 if $file is an ELF file, non-0 otherwise.
+#
+# file : path of file to be tested
+#
+# environment:
+#   READELF: readelf program path
+readelf.is_elf_executable() {
+    test "$( readelf.filter_elf_executable "${1}" )" != ""
+}
+
+# readelf.get_rpath file
+#
+# Return the unsplitted RPATH/RUNPATH of $file.
+#
+# To split the returned RPATH string and store them in an array, do:
+#
+#     paths=( $(readelf.get_rpath "${file}" | sed -e 's/:/ /g') )
+#
+# file : ELF file path
+#
+# environment:
+#   READELF: readelf program path
+readelf.get_rpath() {
+    local file="${1}"
+    LC_ALL=C "${READELF}" --dynamic "${file}" |
+        sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d ; s//\3/'
+}
+
+# readelf.list_sections file
+#
+# Returns the list of ELF sections in $file.
+#
+# file    : ELF file path
+#
+# environment:
+#   READELF: readelf program path
+readelf.list_sections() {
+    local file="${1}"
+    LC_ALL=C "${READELF}" --sections "${file}" |
+        sed -re '/^  \[ *[0-9]+\] (\S+).*/!d ; s//\1/' |
+        sort
+}
+
+# readelf.has_section file section
+#
+# Return 0 if $file has a section named $section
+#
+# file    : ELF file path
+# section : ELF section name
+#
+# environment:
+#   READELF: readelf program path
+readelf.has_section() {
+    log._trace_func
+    local file="${1}" section_name="${2}"
+    readelf.list_sections "${file}" | grep -q "^${section_name}$"
+}
diff --git a/support/scripts/shell/sdk.sh b/support/scripts/shell/sdk.sh
new file mode 100644
index 0000000..b2f699c
--- /dev/null
+++ b/support/scripts/shell/sdk.sh
@@ -0,0 +1,68 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# SDK helpers
+#
+# This module defines the following functions:
+#   sdk.compute_relative_path
+#   sdk.compute_rpath
+
+source.declare_module sdk
+
+# sdk.compute_relative_path basedir path start
+#
+# Computes and prints the relative path between $start and $path within $basedir.
+#
+# basedir : absolute path of the tree in which the $path and $start must be
+# path    : destination absolute path
+# start   : origin absolute path
+sdk.compute_relative_path() {
+    local basedir="${1}"
+    local path="${2}"
+    local start="${3}"
+    # sanity checks: make sure $path and $start starts with $basedir
+    grep -q "^${basedir}" <<<"${path}" || return 1
+    grep -q "^${basedir}" <<<"${start}" || return 1
+    local i
+    local backward="${start#${basedir}}"
+    local relative=()
+    for i in ${backward//\// } ; do
+        # don't need to check for empty items they are already discarded
+        test "${i}" != '.' || continue
+        relative+=( ".." )
+    done
+    relative+=( ${path#${basedir}} )
+    sed -r -e 's:[ /]+:/:g' <<<"${relative[@]}"
+}
+
+# sdk.compute_rpath basedir bindir libdirs...
+#
+# Computes and prints the list of RPATH.
+#
+# basedir : absolute path of the tree in which the $bindir and $libdirs must be
+# bindir  : binary directory absolute path
+# libdirs : list of library directories (absolute paths)
+sdk.compute_rpath() {
+    local basedir="${1}"
+    local bindir="${2}"
+    shift 2
+    local libdirs=( ${@} )
+    local rpath=()
+    for libdir in ${libdirs[@]} ; do
+        rpath+=( "\$ORIGIN/$(sdk.compute_relative_path "${basedir}" "${libdir}" "${bindir}")" )
+    done
+    sed -e 's/ /:/g' <<<"${rpath[@]}"
+}
diff --git a/support/scripts/shell/source.sh b/support/scripts/shell/source.sh
new file mode 100644
index 0000000..9f798b8
--- /dev/null
+++ b/support/scripts/shell/source.sh
@@ -0,0 +1,77 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Source helpers
+#
+# This module defines the following functions:
+#   source.declare_module
+#   source.load_module
+#
+# This module is sensitive to the following environment variables:
+#   TOPDIR
+
+# Assuming the script sourcing this file is in support/scripts/
+: ${TOPDIR:=$( readlink -f "${0}" | sed -re 's:(/[^/]+){3}$::' )}
+
+# source.declare_module module_name
+#
+# Declare a shell module.
+# Set the variable '_source_${module_name}'.
+# Should be called once per module, in the global scope.
+#
+# module_name : Module name (allowed char.: [_a-zA-Z0-9])
+source.declare_module() {
+    local module_name="${1}"
+    # use printf from bash to set the variable in the environment:
+    printf -v "_source_${module_name}" "%s" "${module_name}"
+}
+
+# source.load_module module_name
+#
+# Load the given shell module, making available all functions declared
+# in it, ensuring it is not reloaded if it already is.
+# Should be called in the global scope.
+# Need the TOPDIR environment variable.
+#
+# param module_name: Module name
+source.load_module() {
+    local module_name="${1}"
+    local loaded="loaded=\${_source_${module_name}}"
+    eval "${loaded}"
+    local module_file="${TOPDIR}/support/scripts/shell/${module_name}.sh"
+
+    if [ ! -f "${module_file}" ] ; then
+        cat <<EOF >&2
+error:  Could load module '${module_name}',
+        ${module_file} does not exists.
+
+        Maybe TOPDIR does not point to Buildroot's '\$(TOPDIR)'.
+
+        Or this script '${0##*/}' is most not installed in Buildroot's
+        '\$(TOPDIR)/support/scripts' directory.
+
+        You can fix this by:
+        - either installing '${0##*/}' in the support/scripts/ directory;
+        - or setting the TOPDIR variable in the '${0##*/}' script, before
+          sourcing anything.
+EOF
+        exit 1
+    fi
+
+    test -n "${loaded}" || source "${module_file}"
+}
+
+source.declare_module source
diff --git a/support/scripts/shell/utils.sh b/support/scripts/shell/utils.sh
new file mode 100644
index 0000000..9e9aaab
--- /dev/null
+++ b/support/scripts/shell/utils.sh
@@ -0,0 +1,60 @@
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Miscellaneous helpers
+#
+# This module defines the following functions:
+#   utils.list_has
+#   utils.list_reduce
+
+source.declare_module utils
+
+# utils.list_has value list_items...
+#
+# Returns 0 if $list_items contains $value, returns 1 otherwise.
+#
+# value      : item to be checked if it is in the list
+# list_items : list of items
+utils.list_has() {
+    local key=$1
+    shift
+    for val in $@ ; do
+        if test "$val" = "$key" ; then
+            return 0
+        fi
+    done
+    return 1
+}
+
+# utils.list_reduce input_list
+#
+# Prints the $input_list list with duplicated items removed.
+# Order is preserved, WRT the first occurence of duplicated items.
+#
+# input_list : list of items
+utils.list_reduce() {
+    local -a lout # return list
+    local i
+
+    for i in ${@} ; do
+        if utils.list_has "${i}" ${lout[@]} ; then
+            continue
+        fi
+        lout+=( "${i}" )
+    done
+
+    echo ${lout[@]}
+}
-- 
2.7.2

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

* [Buildroot] [PATCH v7 09/18] core: add HOST_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (7 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 08/18] support/scripts: add fix-rpath script + a bunch of helpers Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 10/18] core: add {TARGET, STAGING}_SANITIZE_RPATH_HOOK " Samuel Martin
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This patch adds host-patchelf as a target-finalize dependency, and
introduces the HOST_SANITIZE_RPATH_HOOK hook to fix the ELF files'
RPATH from the HOST_DIR location (excluding the sysroot).

After running this hook, the RPATH from any host ELF files is relative to
the binary location itself.

Notes:
- we avoid to fix RPATH in the sysroot.
- we do not try to fix RPATH in the external toolchain installation
  location as they may have been built in a way, this is already correct;
  furthermore, fixing RPATH in those programs may result in breaking them.
- the whole host directory is processed because a number of
  host-package install programs that could be useful in places
  different from $(HOST_DIR)/{bin,sbin,usr/bin,usr/sbin}.
- the shared libraries are also processed in case they have a 'main'
  function.

As a step toward a fully relocatable SDK, this change allows to get the
toolchain relocatable, but not yet the whole SDK.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- remove the DEBUG env. var. specific handling in Makefile, now just
  place the variable in the Buildroot environment, to enable debug logs.
  This is documented in a follow-up patch of this series.

changes v5->v6:
- update for the new script version
- add debug mode support

changes v4->v5:
- add verbose support

changes v3->v4:
- add host-patchelf to PACKAGES instead of target-finalize (Baruch)
- add comment

changes v2->v3:
- move hook in Makefile (Baruch)
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Makefile b/Makefile
index f1de4ed..268e14e 100644
--- a/Makefile
+++ b/Makefile
@@ -616,6 +616,21 @@ endef
 TARGET_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 
+# RPATH fixing
+# - The host hook sets RPATH in host ELF binaries, using relative paths to the
+#   library locations.
+# - The target hook sanitizes RPATH in target ELF binaries, removing paths
+#   pointing to package's build directories or the sysroot's libdirs.
+PACKAGES += host-patchelf
+
+define HOST_SANITIZE_RPATH_HOOK
+	PATCHELF=$(HOST_DIR)/usr/bin/patchelf \
+	READELF=readelf \
+		$(TOPDIR)/support/scripts/fix-rpath host $(HOST_DIR)
+endef
+
+TARGET_FINALIZE_HOOKS += HOST_SANITIZE_RPATH_HOOK
+
 $(TARGETS_ROOTFS): target-finalize
 
 target-finalize: $(PACKAGES)
-- 
2.7.2

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

* [Buildroot] [PATCH v7 10/18] core: add {TARGET, STAGING}_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (8 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 09/18] core: add HOST_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 11/18] package/speex: remove no longer needed hook Samuel Martin
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This patch introduces the TARGET_SANITIZE_RPATH_HOOK and
STAGING_SANITIZE_RPATH_HOOK hooks fixing the ELF files' RPATH of
binaries from, respectively, the TARGET_DIR and the STAGING_DIR
locations.

After running this hook, the RPATH from any target ELF files from both
the target and the staging locations won't contain any occurence of the
sysroot or some build locations.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- remove the DEBUG env. var. specific handling in Makefile

changes v5->v6:
- update for the new script version
- add debug mode support

changes v4->v5:
- target hook now sanitizes the rpath (Baruch)
- add verbose support
- update comment

changes v3->v4:
- rebase
- add comment

changes v2->v3:
- move hook in Makefile (Baruch)
---
 Makefile | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Makefile b/Makefile
index 268e14e..e4e9c65 100644
--- a/Makefile
+++ b/Makefile
@@ -631,6 +631,24 @@ endef
 
 TARGET_FINALIZE_HOOKS += HOST_SANITIZE_RPATH_HOOK
 
+# Function sanitizing target/staging ELF files' RPATH.
+# i.e. it removes paths pointing to the staging or build location from the ELF
+# files' RPATH.
+define TARGET_SANITIZE_RPATH_HOOK
+	PATCHELF=$(HOST_DIR)/usr/bin/patchelf \
+	READELF=$(TARGET_READELF) \
+		$(TOPDIR)/support/scripts/fix-rpath target $(TARGET_DIR)
+endef
+
+define STAGING_SANITIZE_RPATH_HOOK
+	PATCHELF=$(HOST_DIR)/usr/bin/patchelf \
+	READELF=$(TARGET_READELF) \
+		$(TOPDIR)/support/scripts/fix-rpath staging $(STAGING_DIR)
+endef
+
+TARGET_FINALIZE_HOOKS += TARGET_SANITIZE_RPATH_HOOK \
+       STAGING_SANITIZE_RPATH_HOOK
+
 $(TARGETS_ROOTFS): target-finalize
 
 target-finalize: $(PACKAGES)
-- 
2.7.2

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

* [Buildroot] [PATCH v7 11/18] package/speex: remove no longer needed hook
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (9 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 10/18] core: add {TARGET, STAGING}_SANITIZE_RPATH_HOOK " Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 12/18] toolchain: add post-install hooks making the toolchain relocatable Samuel Martin
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

Remove the LIBTOOL_FIXUP hook since RPATH are now sanitized in
target-finalize hooks.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- none

changes v4->v5:
- new patch (suggested by Baruch)
---
 package/speex/speex.mk | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/package/speex/speex.mk b/package/speex/speex.mk
index a34bfac..24782ec 100644
--- a/package/speex/speex.mk
+++ b/package/speex/speex.mk
@@ -24,11 +24,6 @@ ifeq ($(BR2_PACKAGE_SPEEX_ARM5E),y)
 SPEEX_CONF_OPTS += --enable-arm5e-asm
 endif
 
-define SPEEX_LIBTOOL_FIXUP
-	$(SED) 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' $(@D)/libtool
-	$(SED) 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' $(@D)/libtool
-endef
-
 SPEEX_POST_CONFIGURE_HOOKS += SPEEX_LIBTOOL_FIXUP
 
 define SPEEX_BUILD_CMDS
-- 
2.7.2

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

* [Buildroot] [PATCH v7 12/18] toolchain: add post-install hooks making the toolchain relocatable
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (10 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 11/18] package/speex: remove no longer needed hook Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 13/18] support/scripts: update check-host-rpath to use the shell helpers Samuel Martin
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This change adds the required dependency and post-target-install hooks
to render the toolchain relocatable.

Notes:
- Although the toolchain package is a virtual package, it does support
  hooks.
- This has to be post-target-install hooks be the toolchain package is
  handled as a "target" package.

Ultimately, this patch allows to generate relocatable toolchains, just
by doing:

  $ make menuconfig
  $ make toolchain

Then, this toolchain can be used/shared/deployed on anywhere*.

*: on any Linux system with an ABI-compatible set of system libraries
   (usually including: glibc, libgcc, zlib, flex and libsigsegv).

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>

---
changes v6->v7:
- new patch
---
 toolchain/toolchain/toolchain.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
index c22713b..618aa64 100644
--- a/toolchain/toolchain/toolchain.mk
+++ b/toolchain/toolchain/toolchain.mk
@@ -12,6 +12,11 @@ endif
 
 TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO
 
+# dependency and hooks needed to make the toolchain relocatable
+TOOLCHAIN_DEPENDENCIES += host-patchelf
+TOOLCHAIN_POST_INSTALL_TARGET_HOOKS += HOST_SANITIZE_RPATH_HOOK \
+	STAGING_SANITIZE_RPATH_HOOK
+
 $(eval $(virtual-package))
 
 toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
-- 
2.7.2

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

* [Buildroot] [PATCH v7 13/18] support/scripts: update check-host-rpath to use the shell helpers
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (11 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 12/18] toolchain: add post-install hooks making the toolchain relocatable Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command Samuel Martin
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- minor updates after shell modules shell changes

changes v5->v6:
- new patch
---
 support/scripts/check-host-rpath | 71 +++++++++++++++-------------------------
 support/scripts/shell/readelf.sh | 69 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 44 deletions(-)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 48d69da..0bbac30 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -1,12 +1,29 @@
 #!/usr/bin/env bash
 
+# Copyright (C) 2015 Yann E. MORIN <yann.morin.1998@free.fr>
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
 # This script scans $(HOST_DIR)/{bin,sbin} for all ELF files, and checks
 # they have an RPATH to $(HOST_DIR)/usr/lib if they need libraries from
 # there.
 
-# Override the user's locale so we are sure we can parse the output of
-# readelf(1) and file(1)
-export LC_ALL=C
+source "${0%/*}/shell/source.sh"
+
+source.load_module readelf
 
 main() {
     local pkg="${1}"
@@ -18,54 +35,20 @@ main() {
 
     ret=0
     while read file; do
-        elf_needs_rpath "${file}" "${hostdir}" || continue
-        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        READELF=readelf readelf.needs_rpath "${file}" "${hostdir}" || continue
+        READELF=readelf readelf.has_rpath "${file}" "${hostdir}" && continue
         if [ ${ret} -eq 0 ]; then
             ret=1
             printf "***\n"
-            printf "*** ERROR: package %s installs executables without proper RPATH:\n" "${pkg}"
+            printf \
+                "*** ERROR: package %s installs executables without proper RPATH:\n" \
+                "${pkg}"
         fi
         printf "***   %s\n" "${file}"
-    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -exec file {} + 2>/dev/null \
-              |sed -r -e '/^([^:]+):.*\<ELF\>.*\<executable\>.*/!d'                \
-                      -e 's//\1/'                                                  \
-            )
+    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print |
+                  readelf.filter_elf_executable )
 
     return ${ret}
 }
 
-elf_needs_rpath() {
-    local file="${1}"
-    local hostdir="${2}"
-    local lib
-
-    while read lib; do
-        [ -e "${hostdir}/usr/lib/${lib}" ] && return 0
-    done < <( readelf -d "${file}"                                         \
-              |sed -r -e '/^.* \(NEEDED\) .*Shared library: \[(.+)\]$/!d;' \
-                     -e 's//\1/;'                                          \
-            )
-
-    return 1
-}
-
-check_elf_has_rpath() {
-    local file="${1}"
-    local hostdir="${2}"
-    local rpath dir
-
-    while read rpath; do
-        for dir in ${rpath//:/ }; do
-            # Remove duplicate and trailing '/' for proper match
-            dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
-            [ "${dir}" = "${hostdir}/usr/lib" ] && return 0
-        done
-    done < <( readelf -d "${file}"                                              \
-              |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
-                      -e 's//\3/;'                                              \
-            )
-
-    return 1
-}
-
 main "${@}"
diff --git a/support/scripts/shell/readelf.sh b/support/scripts/shell/readelf.sh
index 541b3e1..09b680e 100644
--- a/support/scripts/shell/readelf.sh
+++ b/support/scripts/shell/readelf.sh
@@ -24,11 +24,15 @@
 #   readelf.is_elf_executable
 #   readelf.is_elf_shared_object
 #   readelf.get_rpath
+#   readelf.get_neededs
+#   readelf.needs_rpath
+#   readelf.has_rpath
 #   readelf.list_sections
 #   readelf.has_section
 #
 # This module is sensitive to the following environment variables:
 #   READELF
+
 source.declare_module readelf
 
 # When calling readelf(1) program, the user's locale will be overriden with the
@@ -143,6 +147,71 @@ readelf.get_rpath() {
         sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d ; s//\3/'
 }
 
+# readelf.get_neededs file
+#
+# Return the list of the NEEDED libraries of $file.
+#
+# file : ELF file path
+#
+# environment:
+#   READELF: readelf program path
+readelf.get_neededs() {
+    local file="${1}"
+    LC_ALL=C "${READELF}" --dynamic "${file}" |
+        sed -r -e '/^.* \(NEEDED\) .*Shared library: \[(.+)\]$/!d ; s//\1/'
+}
+
+# readelf.needs_rpath file basedir
+#
+# Return 0 if $file needs to have RPATH set, 1 otherwise.
+#
+# file    : path of file to be tested
+# basedir : path of the tree in which $basedir/lib and $basedir/usr/lib are
+#           checked for belonging to RPATH
+#
+# environment:
+#   READELF: readelf program path
+readelf.needs_rpath() {
+    local file="${1}"
+    local basedir="${2}"
+    local lib
+
+    while read lib; do
+        [ -e "${basedir}/lib/${lib}" ] && return 0
+        [ -e "${basedir}/usr/lib/${lib}" ] && return 0
+    done < <( readelf.get_neededs "${file}" )
+    return 1
+}
+
+# readelf.has_rpath file basedir
+#
+# Return 0 if $file has RPATH already set to $basedir/lib or $basedir/usr/lib,
+# 1 otherwise.
+#
+# file    : path of file to be tested
+# basedir : path of the tree in which $basedir/lib and $basedir/usr/lib are
+#           checked for belonging to RPATH
+#
+# environment:
+#   READELF: readelf program path
+readelf.has_rpath() {
+    local file="${1}"
+    local basedir="${2}"
+    local rpath dir
+
+    while read rpath; do
+        for dir in ${rpath//:/ }; do
+            # Remove duplicate and trailing '/' for proper match
+            dir="$( sed -r -e "s:/+:/:g; s:/$::" <<<"${dir}" )"
+            [ "${dir}" = "${basedir}/lib" ] && return 0
+            [ "${dir}" = "${basedir}/usr/lib" ] && return 0
+            grep -q '^\$ORIGIN/' <<<"${dir}" && return 0
+        done
+    done < <( readelf.get_rpath "${file}" )
+
+    return 1
+}
+
 # readelf.list_sections file
 #
 # Returns the list of ELF sections in $file.
-- 
2.7.2

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

* [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (12 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 13/18] support/scripts: update check-host-rpath to use the shell helpers Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-10  0:15   ` Arnout Vandecappelle
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin} Samuel Martin
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

Silent find command when HOST_DIR/usr/sbin does not exist.

Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- new patch
---
 support/scripts/check-host-rpath | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 0bbac30..2d3ddec 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -45,7 +45,7 @@ main() {
                 "${pkg}"
         fi
         printf "***   %s\n" "${file}"
-    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print |
+    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print 2>/dev/null |
                   readelf.filter_elf_executable )
 
     return ${ret}
-- 
2.7.2

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

* [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin}
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (13 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-10  0:16   ` Arnout Vandecappelle
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers Samuel Martin
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

At least syslinux is installing stuff in HOST_DIR/sbin.

Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- new patch
---
 support/scripts/check-host-rpath | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 2d3ddec..f5f16ff 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -45,7 +45,7 @@ main() {
                 "${pkg}"
         fi
         printf "***   %s\n" "${file}"
-    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print 2>/dev/null |
+    done < <( find "${hostdir}"/{,usr/}{bin,sbin} -type f -print 2>/dev/null |
                   readelf.filter_elf_executable )
 
     return ${ret}
-- 
2.7.2

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

* [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (14 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin} Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-27 22:38   ` Arnout Vandecappelle
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 17/18] core: add check-leaks-in-{target, host, staging} targets Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script Samuel Martin
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- {filer,is}_elf* functions moved from utils to readelf module
- update sdk.check_host_leaks

changes v5->v6:
- new patch
---
 support/scripts/check-host-leaks | 63 +++++++++++++++++++++++++++++
 support/scripts/shell/readelf.sh | 85 ++++++++++++++++++++++++++++++++++++++++
 support/scripts/shell/sdk.sh     | 70 +++++++++++++++++++++++++++++++++
 support/scripts/shell/utils.sh   | 16 ++++++++
 4 files changed, 234 insertions(+)
 create mode 100755 support/scripts/check-host-leaks

diff --git a/support/scripts/check-host-leaks b/support/scripts/check-host-leaks
new file mode 100755
index 0000000..42da272
--- /dev/null
+++ b/support/scripts/check-host-leaks
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+usage() {
+  cat <<EOF >&2
+Usage:  ${0} TREE_DIR TOPDIR BASE_DIR HOST_DIR STAGING_DIR
+
+Description:
+
+        This script scans a tree for host paths leaks into it.
+        Print out leaks along side to their kind.
+
+Arguments:
+
+        TREE_DIR        Path to the root of the tree to be scaned
+
+        TOPDIR          Buildroot's TOPDIR
+
+        BASE_DIR        Buildroot's build output base location
+
+        HOST_DIR        Buildroot's host directory
+
+        STAGING_DIR     Buildroot's staging directory
+
+EOF
+}
+
+source "${0%/*}/shell/source.sh"
+
+READELF=readelf
+
+source.load_module utils
+source.load_module sdk
+
+main() {
+    if test ${#} -ne 5 ; then
+        usage
+        exit 1
+    fi
+    local to_scan="${1}"
+    local topdir="${2}" basedir="${3}" hostdir="${4}" stagingdir="${5}"
+    local gnu_target_name=$( utils.guess_gnu_target_name "${stagingdir}" )
+    local tree path
+    sdk.check_host_leaks "${gnu_target_name}" "${to_scan}" \
+        "${topdir}" "${basedir}" "${hostdir}"
+}
+
+main "${@}"
diff --git a/support/scripts/shell/readelf.sh b/support/scripts/shell/readelf.sh
index 09b680e..e07276f 100644
--- a/support/scripts/shell/readelf.sh
+++ b/support/scripts/shell/readelf.sh
@@ -21,14 +21,19 @@
 #   readelf.filter_elf
 #   readelf.filter_elf_executable
 #   readelf.filter_elf_shared_object
+#   readelf.filter_elf_static_library
+#   readelf.filter_elf_object
 #   readelf.is_elf_executable
 #   readelf.is_elf_shared_object
+#   readelf.is_elf_static_library
+#   readelf.is_elf_object
 #   readelf.get_rpath
 #   readelf.get_neededs
 #   readelf.needs_rpath
 #   readelf.has_rpath
 #   readelf.list_sections
 #   readelf.has_section
+#   readelf.string_section
 #
 # This module is sensitive to the following environment variables:
 #   READELF
@@ -105,6 +110,40 @@ readelf.filter_elf_executable() {
     readelf._filter_elf_regexp "Type:\s+EXEC\s\(Executable\sfile\)" "${@}"
 }
 
+# readelf.filter_elf_static_library file...
+#
+# Filters ELF files; if $file is an ELF file, $file is printed, else it is
+# discarded.
+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf.filter_elf_static_library() {
+    readelf._filter_elf_regexp "Type:\s+REL\s\(Relocatable\sfile\)" "${@}" |
+    readelf._filter_elf_regexp "^File:\s+\S+\)$"
+}
+
+# readelf.filter_elf_object file...
+#
+# Filters ELF files; if $file is an ELF file, $file is printed, else it is
+# discarded.
+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:
+#   READELF: readelf program path
+readelf.filter_elf_object() {
+    readelf._filter_elf_regexp "Type:\s+REL\s\(Relocatable\sfile\)" "${@}" |
+    while read file ; do
+        LC_ALL=C ${READELF} -h "${file}" 2>/dev/null |
+            grep -qE "^File:\s+\S+\)$" ||
+        printf "%s\n" "${file}"
+   done
+}
+
 # readelf.is_elf_shared_object file
 #
 # Returns 0 if $file is an ELF file, non-0 otherwise.
@@ -129,6 +168,38 @@ readelf.is_elf_executable() {
     test "$( readelf.filter_elf_executable "${1}" )" != ""
 }
 
+# readelf.is_elf file
+#
+# Returns 0 if $file is an ELF file, non-0 otherwise.
+#
+# file : path of file to be tested
+#
+# environment:
+#   READELF: readelf program path
+readelf.is_elf() {
+    test "$( readelf.filter_elf "${1}" )" != ""
+}
+
+# readelf.is_elf_static_library file
+#
+# Return 0 if $file is a Linux static libraries, i.e. an ar-archive
+# containing *.o files.
+#
+# file : path of file to be tested
+readelf.is_elf_static_library() {
+    test "$( readelf.filter_elf_static_library "${1}" )" != ""
+}
+
+# readelf.is_elf_object file
+#
+# Return 0 if $file is a Linux static libraries, i.e. an ar-archive
+# containing *.o files.
+#
+# file : path of file to be tested
+readelf.is_elf_object() {
+    test "$( readelf.filter_elf_object "${1}" )" != ""
+}
+
 # readelf.get_rpath file
 #
 # Return the unsplitted RPATH/RUNPATH of $file.
@@ -241,3 +312,17 @@ readelf.has_section() {
     local file="${1}" section_name="${2}"
     readelf.list_sections "${file}" | grep -q "^${section_name}$"
 }
+
+# readelf.string_section file section
+#
+# Return the given $section of $file.
+#
+# file    : ELF file path
+# section : ELF section name
+#
+# environment:
+#   READELF: readelf program path
+readelf.string_section() {
+    local file="${1}" section="${2}"
+    LC_ALL=C "${READELF}" --string-dump "${section}" "${file}" 2>/dev/null
+}
diff --git a/support/scripts/shell/sdk.sh b/support/scripts/shell/sdk.sh
index b2f699c..ea96ebf 100644
--- a/support/scripts/shell/sdk.sh
+++ b/support/scripts/shell/sdk.sh
@@ -19,9 +19,16 @@
 # This module defines the following functions:
 #   sdk.compute_relative_path
 #   sdk.compute_rpath
+#   sdk.check_host_leaks
+#
+# This module is sensitive to the following environment variables:
+#   READELF
 
 source.declare_module sdk
 
+source.load_module utils
+source.load_module readelf
+
 # sdk.compute_relative_path basedir path start
 #
 # Computes and prints the relative path between $start and $path within $basedir.
@@ -66,3 +73,66 @@ sdk.compute_rpath() {
     done
     sed -e 's/ /:/g' <<<"${rpath[@]}"
 }
+
+# sdk.check_host_leaks gnu_target_name root pattern_basedir...
+#
+# Scan the $root tree for the given $pattern_basedir pattern leaks.
+# The $gnu_target_name is used to skip the sysroot location when
+# scanning the host tree.
+# Categorize the type of leaks.
+# Print the leaks categories and and the matching file path.
+#
+# gnu_target_name : GNU target name (e.g. arm-buildroot-linux-gnueabihf)
+# root            : path to the root of the tree to be scanned
+# pattern_basedir : list of patterns to be searched
+sdk.check_host_leaks() {
+    local target="${1}" root="${2}"
+    local patterns=()
+    local pattern
+    shift 2
+    for pattern in ${@} ; do
+        patterns+=( "${pattern}" "$( readlink -f "${pattern}" )" )
+    done
+    patterns=( $( utils.list_reduce ${patterns[@]} ) )
+
+    local regexp="$( sed -re 's/^/(/ ; s/$/)/ ; s/ +/|/g' <<<"${patterns[*]}" )"
+    ( cd "${root}"
+        local f leak
+        while read f ; do
+            leak=
+            if test -h "${f}" ; then leak="symlink"
+            elif readelf.is_elf "${f}" ;then
+                if readelf.is_elf_executable "${f}" ; then leak="ELF/exe"
+                elif readelf.is_elf_shared_object "${f}" ; then leak="ELF/*.so"
+                elif readelf.is_elf_static_library "${f}" ; then leak="ELF/*.a"
+                elif readelf.is_elf_object "${f}" ; then
+                    case "${f}" in
+                        *.ko) leak="ELF/*.ko" ;;
+                        *) leak="ELF/*.o" ;;
+                    esac
+                else leak="ELF/?"
+                fi
+                local section
+                local sections=()
+                for section in $( readelf.list_sections "${f}" ) ; do
+                    if readelf.string_section "${f}" "${section}" |
+                            grep -qaE "${regexp}" ; then
+                        sections+=( "${section}" )
+                    fi
+                done
+                leak="${leak} [${sections[*]}]"
+            else
+                case "${f}" in
+                    *.la) leak="*.la" ;;
+                    *.pc) leak="*.pc" ;;
+                    *.py) leak="*.py" ;;
+                    *.pyc) leak="*.pyc" ;;
+                esac
+            fi
+            if test -z "${leak}" ; then
+                leak="? [$( file -z "${f}" | sed -e 's/.*: //' )]"
+            fi
+            printf "%-70s : %-120s\n" "${leak}" "${f}"
+        done < <( grep -raEl "${regexp}" . | sed -re "/${target}\/sysroot\//d ; s:^\.:${root}:" )
+    ) | sort
+}
diff --git a/support/scripts/shell/utils.sh b/support/scripts/shell/utils.sh
index 9e9aaab..8e0a443 100644
--- a/support/scripts/shell/utils.sh
+++ b/support/scripts/shell/utils.sh
@@ -19,6 +19,7 @@
 # This module defines the following functions:
 #   utils.list_has
 #   utils.list_reduce
+#   utils.guess_gnu_target_name
 
 source.declare_module utils
 
@@ -58,3 +59,18 @@ utils.list_reduce() {
 
     echo ${lout[@]}
 }
+
+# utils.guess_gnu_target_name sysroot
+#
+# Guess the GNU target name from the given sysroot location.
+# This assumes the sysroot is located in:
+#   <somewhere>/<gnu_target_name>/sysroot
+#
+# sysroot : path to the sysroot
+utils.guess_gnu_target_name() {
+    local sysroot="${1}"
+    sysroot="$( readlink -f "${sysroot}" )"
+    local gnu_target_name="${sysroot%/sysroot*}"
+    gnu_target_name="${gnu_target_name##*/}"
+    printf "%s" "${gnu_target_name}"
+}
-- 
2.7.2

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

* [Buildroot] [PATCH v7 17/18] core: add check-leaks-in-{target, host, staging} targets
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (15 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script Samuel Martin
  17 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

This new targets allow identifying what need to be fixed to fulfill the
ultimate goal of removing any trace of the host in the target filesystem
or the SDK.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- none

changes v5->v6:
- new patch
---
 Makefile             | 15 +++++++++++++++
 package/pkg-utils.mk |  5 +++++
 2 files changed, 20 insertions(+)

diff --git a/Makefile b/Makefile
index e4e9c65..45e8d41 100644
--- a/Makefile
+++ b/Makefile
@@ -616,6 +616,21 @@ endef
 TARGET_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 
+# Checks for build machine leaks into target filesystem, SDK
+check-leaks-in-target:
+	@$(call MESSAGE,Checking leaks in the target)
+	$(Q)$(call check-for-build-machine-leaks-in,$(TARGET_DIR))
+
+check-leaks-in-host:
+	@$(call MESSAGE,Checking leaks in the host)
+	$(Q)$(call check-for-build-machine-leaks-in,$(HOST_DIR))
+
+check-leaks-in-staging:
+	@$(call MESSAGE,Checking leaks in the staging)
+	$(Q)$(call check-for-build-machine-leaks-in,$(STAGING_DIR))
+
+check-leaks:	check-leaks-in-target check-leaks-in-host check-leaks-in-staging
+
 # RPATH fixing
 # - The host hook sets RPATH in host ELF binaries, using relative paths to the
 #   library locations.
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index f88313a..6a2cbb6 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -150,3 +150,8 @@ define legal-license-file # pkg, filename, file-fullpath, {HOST|TARGET}
 	mkdir -p $(LICENSE_FILES_DIR_$(4))/$(1)/$(dir $(2)) && \
 	cp $(3) $(LICENSE_FILES_DIR_$(4))/$(1)/$(2)
 endef
+
+define check-for-build-machine-leaks-in
+	$(TOPDIR)/support/scripts/check-host-leaks $(1) \
+		$(TOPDIR) $(BASE_DIR) $(HOST_DIR) $(STAGING_DIR)
+endef
-- 
2.7.2

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

* [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script
  2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
                   ` (16 preceding siblings ...)
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 17/18] core: add check-leaks-in-{target, host, staging} targets Samuel Martin
@ 2016-03-09 22:58 ` Samuel Martin
  2016-03-10  0:21   ` Arnout Vandecappelle
  17 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-09 22:58 UTC (permalink / raw)
  To: buildroot

The method describes here is only true for scripts using the shell
modules from support/scripts/shell/*.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v6->v7:
- new patch
---
 docs/manual/debugging-buildroot.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/docs/manual/debugging-buildroot.txt b/docs/manual/debugging-buildroot.txt
index f575fc4..c497284 100644
--- a/docs/manual/debugging-buildroot.txt
+++ b/docs/manual/debugging-buildroot.txt
@@ -41,3 +41,18 @@ The script has access to the following variables:
   - +BINARIES_DIR+: the place where all binary files (aka images) are
     stored
   - +BASE_DIR+: the base output directory
+
+=== Debugging shell scripts in Buildroot
+
+For shell scripts using the _shell modules_ (available in
++support/scripts/shell/+), there is a couple of environment variables
+you can set to trace the scripts
+
+  - +SHELL_DEBUG+ adjustes the debug level, from 0 to 3 (only errors to
+    all debug messages)
+
+To enable these debug outputs:
+
+----
+SHELL_DEBUG=3 make
+----
-- 
2.7.2

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

* [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation Samuel Martin
@ 2016-03-09 23:09   ` Yann E. MORIN
  2016-03-12  8:46     ` Samuel Martin
  0 siblings, 1 reply; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-09 23:09 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> This patch removes unnecessary files generated during installation, that
> also are causing build machine paths leaking into the Buildroot's SDK.
> 
> Note that similar cleanup commands can be found in other distributions
> (e.g. gentoo [1]); thought this is not a valid reason.
> 
> [1] https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-kernel/linux-headers/linux-headers-4.4.ebuild#n36
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v6->v7:
> - improve commit log
> 
> changes v5->v6:
> - new patch
> ---
>  package/linux-headers/linux-headers.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
> index 0900778..7633257 100644
> --- a/package/linux-headers/linux-headers.mk
> +++ b/package/linux-headers/linux-headers.mk
> @@ -123,6 +123,7 @@ define LINUX_HEADERS_INSTALL_STAGING_CMDS
>  			HOSTCXX="$(HOSTCXX)" \
>  			INSTALL_HDR_PATH=$(STAGING_DIR)/usr \
>  			headers_install)
> +	find $(STAGING_DIR)/usr \( -name .install -o -name ..install.cmd \) -delete

I would further restrict that to $(STAGING_DIR)/usr/include .

Also, Gentoo also has code to get rid of empty directories:
    find $(STAGING_DIR)/usr -depth -type d -delete 2>/dev/null

Although we don't really care about empty dirs in staging, TBH...

Regards,
Yann E. MORIN.

>  endef
>  
>  ifeq ($(BR2_KERNEL_HEADERS_VERSION)$(BR2_KERNEL_HEADERS_AS_KERNEL),y)
> -- 
> 2.7.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
@ 2016-03-09 23:10   ` Yann E. MORIN
  2016-03-09 23:51   ` Arnout Vandecappelle
  2016-03-10 22:50   ` Thomas Petazzoni
  2 siblings, 0 replies; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-09 23:10 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> This changes saves a shell call and uses a variable automatically set
> by make [1].
> 
> [1] http://www.gnu.org/software/make/manual/make.html#Quick-Reference
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> 
> ---
> changes v6->v7:
> - none
> 
> changes v5->v6:
> - new patch
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index f2822a2..98c8dc7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,7 +69,7 @@ endif
>  .NOTPARALLEL:
>  
>  # absolute path
> -TOPDIR := $(shell pwd)
> +TOPDIR := $(CURDIR)
>  CONFIG_CONFIG_IN = Config.in
>  CONFIG = support/kconfig
>  DATE := $(shell date +%Y%m%d)
> -- 
> 2.7.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path Samuel Martin
@ 2016-03-09 23:15   ` Yann E. MORIN
  2016-03-12  8:48     ` Samuel Martin
       [not found]   ` <CAFt09wNvtXGcfZ3PFZOgGu+bDvp6mEr44uwxa_ryjeP6DDQKBQ@mail.gmail.com>
  1 sibling, 1 reply; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-09 23:15 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
> they can be resolved differently, depending on each package build-system
> (whether it uses the given paths or get the absolute canonical ones).
> 
> Thus, to make easier tracking down host machine paths leaking into the
> host, target or staging trees, the CURDIR and O variables are set to
> their absolute canonical paths.
> 
> Note that this change takes care of the makefile wrapper installed in
> $(O) to avoid unneeded make recursion.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v6->v7:
> - none
> 
> changes v5->v6:
> - new patch
> ---
>  Makefile | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 98c8dc7..52a92f6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -26,16 +26,30 @@
>  
>  # Trick for always running with a fixed umask
>  UMASK = 0022
> -ifneq ($(shell umask),$(UMASK))
> +
> +# Check if we need to re-enter make for one or several of the following reasons:
> +# 1- Wrong (too restrictive) umask:
> +#    This prevents Buildroot and packages from creating files and directories.
> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +#
> +# Note:
> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
> +#   wrapper installed in the $(O).
> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))

This is a bit difficult to parse. What about setting intermediate
variables, like so:

    CUR_UMASK := $(shell umask)
    REAL_CURDIR := $(realpath $(CURDIR))
    O := $(patsubst %/.,%,$(O))
    REAL_O := $(realpath $(O))

    ifneq ($(CUR_UMASK):$(CURDIR):$(O),$(UMASK):$(REAL_CURDIR):$(REAL_O))

>  .PHONY: _all $(MAKECMDGOALS)
>  
>  $(MAKECMDGOALS): _all
>  	@:
>  
>  _all:
> -	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
> +	umask $(UMASK) && \

Please keep the command silent, do not remove the leading @

Regards,
Yann E. MORIN.

> +		$(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
> +			$(MAKECMDGOALS) O=$(realpath $(O))
>  
> -else # umask
> +else # umask / $(CURDIR) / $(O)
>  
>  # This is our default rule, so must come first
>  all:
> @@ -1007,4 +1021,4 @@ include docs/manual/manual.mk
>  
>  .PHONY: $(noconfig_targets)
>  
> -endif #umask
> +endif #umask / $(CURDIR) / $(O)
> -- 
> 2.7.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible Samuel Martin
@ 2016-03-09 23:20   ` Yann E. MORIN
  2016-03-10 20:47   ` Thomas Petazzoni
  1 sibling, 0 replies; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-09 23:20 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> This change turns the staging/ symlink to a relative one when
> $(STAGING_DIR) points to some location in the Buildroot's output
> directory (i.e.: in $(BASE_DIR)/).
> 
> In any other case, the symlink target remains the absolute path
> to the sysroot location.
> 
> This slight change is a step forward toward a relocatable SDK.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v6->v7:
> - improve commit log (ThomasP)
> 
> changes v5->v6:
> - no changes
> 
> changes v4->v5:
> - add support when BR2_HOST_DIR is set somewhere out of BASE_DIR
> 
> changes v3->v4:
> - no change
> 
> changes v2->v3:
> - no change
> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 52a92f6..f1de4ed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -518,7 +518,11 @@ $(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIS
>  # Populating the staging with the base directories is handled by the skeleton package
>  $(STAGING_DIR):
>  	@mkdir -p $(STAGING_DIR)
> +ifeq ($(BASE_DIR)/host/$(STAGING_SUBDIR),$(STAGING_DIR))

I'm not too fond of this. Here, you duplicate the logic used to set
STAGING_DIR, with the assumption that it is a sucbset of host/ . This is
indeed true, but I don;t like duplicating the logic.

What about:

    ifeq ($(patsubst $(BASE_DIR)/%,%,$(STAGING_DIR)),$(STAGING_DIR))
    # Not a subdir, absolute symlink
    else
    # A subdir, relative symlink
    endif

Regards,
Yann E. MORIN.

> +	@ln -snf host/$(STAGING_SUBDIR) $(BASE_DIR)/staging
> +else
>  	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> +endif
>  
>  RSYNC_VCS_EXCLUSIONS = \
>  	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
> -- 
> 2.7.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable Samuel Martin
@ 2016-03-09 23:24   ` Yann E. MORIN
  2016-03-10  0:07     ` Arnout Vandecappelle
  0 siblings, 1 reply; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-09 23:24 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix computed
> relatively to the script location.
> 
> This patch hook only fixes *-config scripts located in the staging area,
> the target ones are already removed. A follow-up change will fix those
> from the HOST_DIR location.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v6->v7:
> - none
> 
> changes v5->v6:
> - new patch
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 3904c09..ffa21ee 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -240,7 +240,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>  		$(call MESSAGE,"Fixing package configuration files") ;\
>  			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
>  				-e "s,$(STAGING_DIR), at STAGING_DIR@,g" \
> -				-e "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
> +				-e "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \

I'd prefer we use the $() form when calling subshells:

    -e "s,^\(exec_\)\?prefix=.*,\1prefix=$$(dirname \$$0)/../../usr,g" 

I know we switched to using `` in some locations, but that was because
those locations may be called with various levels of $(eval), and that
would cause $$() to be evaluated too early in some situations.

However, here we are in a real rule, so there's no such ambiguity.

But I won't block it just for that. I you agree with my proposed change,
that's OK; if you don't, that's OK too:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

>  				-e "s,-I/usr/,-I at STAGING_DIR@/usr/,g" \
>  				-e "s,-L/usr/,-L at STAGING_DIR@/usr/,g" \
>  				-e "s, at STAGING_DIR@,$(STAGING_DIR),g" \
> -- 
> 2.7.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
  2016-03-09 23:10   ` Yann E. MORIN
@ 2016-03-09 23:51   ` Arnout Vandecappelle
  2016-03-10 20:45     ` Thomas Petazzoni
  2016-03-10 22:50   ` Thomas Petazzoni
  2 siblings, 1 reply; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-09 23:51 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> This changes saves a shell call and uses a variable automatically set
> by make [1].

  Wouldn't it be better to just use $(CURDIR) everywhere then?

  Regards,
  Arnout

>
> [1] http://www.gnu.org/software/make/manual/make.html#Quick-Reference
>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v6->v7:
> - none
>
> changes v5->v6:
> - new patch
> ---
>   Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f2822a2..98c8dc7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,7 +69,7 @@ endif
>   .NOTPARALLEL:
>
>   # absolute path
> -TOPDIR := $(shell pwd)
> +TOPDIR := $(CURDIR)
>   CONFIG_CONFIG_IN = Config.in
>   CONFIG = support/kconfig
>   DATE := $(shell date +%Y%m%d)
>


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

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

* [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable
  2016-03-09 23:24   ` Yann E. MORIN
@ 2016-03-10  0:07     ` Arnout Vandecappelle
  2016-03-12  9:22       ` Samuel Martin
  0 siblings, 1 reply; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-10  0:07 UTC (permalink / raw)
  To: buildroot

On 03/10/16 00:24, Yann E. MORIN wrote:
> Samuel, All,
>
> On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
>> This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix computed
>> relatively to the script location.
>>
>> This patch hook only fixes *-config scripts located in the staging area,
>> the target ones are already removed. A follow-up change will fix those
>> from the HOST_DIR location.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v6->v7:
>> - none
>>
>> changes v5->v6:
>> - new patch
>> ---
>>   package/pkg-generic.mk | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index 3904c09..ffa21ee 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -240,7 +240,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>>   		$(call MESSAGE,"Fixing package configuration files") ;\
>>   			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
>>   				-e "s,$(STAGING_DIR), at STAGING_DIR@,g" \
>> -				-e "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
>> +				-e "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \
>
> I'd prefer we use the $() form when calling subshells:
>
>      -e "s,^\(exec_\)\?prefix=.*,\1prefix=$$(dirname \$$0)/../../usr,g"
                                             ^ missing \ here

>
> I know we switched to using `` in some locations, but that was because
> those locations may be called with various levels of $(eval), and that
> would cause $$() to be evaluated too early in some situations.

  But we like things to be consistent, so I'd stick to `.

  OTOH, this particular bit is not going to be expanded by the shell, but is 
actually inserted into the script. So perhaps we don't _want_ to be consistent here.

  But like Yann says, it's not so important.

  Regards,
  Arnout


>
> However, here we are in a real rule, so there's no such ambiguity.
>
> But I won't block it just for that. I you agree with my proposed change,
> that's OK; if you don't, that's OK too:
>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Regards,
> Yann E. MORIN.
>
>>   				-e "s,-I/usr/,-I at STAGING_DIR@/usr/,g" \
>>   				-e "s,-L/usr/,-L at STAGING_DIR@/usr/,g" \
>>   				-e "s, at STAGING_DIR@,$(STAGING_DIR),g" \
>> --
>> 2.7.2
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>


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

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

* [Buildroot] [PATCH v7 06/18] core: make host *-config scripts relocatable
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 06/18] core: make host " Samuel Martin
@ 2016-03-10  0:12   ` Arnout Vandecappelle
  2016-03-10 12:30     ` Samuel Martin
  2016-03-10 20:50   ` Thomas Petazzoni
  1 sibling, 1 reply; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-10  0:12 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix computed
> relatively to the script location.
>
> This patch only fixes *-config scripts located in the host area, those
> from the target and staging area are already handled.
>
> This hook is a kind of brute-force sed on all files matching '*-config'
> in the $(HOST_DIR)/usr/bin directory. This allows correctly handle
> script whose name may depends on the host system, not the target
> configuration (e.g. ncurses can be configured for the target to be
> ncurses6w, so ncurses6w-config ; whereas for the host it will be
> ncurses5, so ncurses5-config).
> Since there way less host packages than target ones, it is reasonable to
> assume that processing all *-config scripts found when a host package
> (whose target package set the *_CONFIG_SCRIPTS variable) is fairly short
> and does not add its overhead not even noticeable.

  I don't agree with this on the principle that we like to handle things 
explicitly, rather than having some automagic stuff that is hard to pinpoint. So 
I think we should just set the appropriate HOST_*_CONFIG_SCRIPTS variables. To 
alleviate the situation, we could add a ?= in generic-package, so only the cases 
where the host script deviates from the target script (or where there is no 
target package) have to be handled. Probably just ncurses.

  Regards,
  Arnout

>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v6->v7:
> - none
>
> changes v5->v6:
> - new patch
> ---
>   package/pkg-generic.mk | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index ffa21ee..845c8d3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -207,6 +207,18 @@ $(BUILD_DIR)/%/.stamp_host_installed:
>   	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
>   	+$($(PKG)_INSTALL_CMDS)
>   	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
> +	$(Q)if test -n "$($(RAWNAME)_CONFIG_SCRIPTS)" ; then \
> +		$(call MESSAGE,"Fixing package configuration files") ;\
> +			ls $(HOST_DIR)/usr/bin/*-config 2>/dev/null |\
> +			xargs --no-run-if-empty \
> +			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
> +				-e "s,$(HOST_DIR), at HOST_DIR@,g" \
> +				-e "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \
> +				-e "s,-I/usr/,-I at HOST_DIR@/usr/,g" \
> +				-e "s,-L/usr/,-L at HOST_DIR@/usr/,g" \
> +				-e "s, at HOST_DIR@,$(HOST_DIR),g" \
> +				-e "s, at BASE_DIR@,$(BASE_DIR),g" ;\
> +	fi
>   	@$(call step_end,install-host)
>   	$(Q)touch $@
>
>


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

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

* [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command Samuel Martin
@ 2016-03-10  0:15   ` Arnout Vandecappelle
  0 siblings, 0 replies; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-10  0:15 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> Silent find command when HOST_DIR/usr/sbin does not exist.

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

  Can this be applied right away, maybe?

  Regards,
  Arnout

>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v6->v7:
> - new patch
> ---
>   support/scripts/check-host-rpath | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 0bbac30..2d3ddec 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -45,7 +45,7 @@ main() {
>                   "${pkg}"
>           fi
>           printf "***   %s\n" "${file}"
> -    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print |
> +    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print 2>/dev/null |
>                     readelf.filter_elf_executable )
>
>       return ${ret}
>


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

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

* [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin}
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin} Samuel Martin
@ 2016-03-10  0:16   ` Arnout Vandecappelle
  0 siblings, 0 replies; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-10  0:16 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> At least syslinux is installing stuff in HOST_DIR/sbin.
>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

  Hold on with this one, I'll post a series that removes the usr/ part (or at 
least, makes it a symlink to .), so this won't be needed anymore.


  Regards,
  Arnout

>
> ---
> changes v6->v7:
> - new patch
> ---
>   support/scripts/check-host-rpath | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 2d3ddec..f5f16ff 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -45,7 +45,7 @@ main() {
>                   "${pkg}"
>           fi
>           printf "***   %s\n" "${file}"
> -    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -print 2>/dev/null |
> +    done < <( find "${hostdir}"/{,usr/}{bin,sbin} -type f -print 2>/dev/null |
>                     readelf.filter_elf_executable )
>
>       return ${ret}
>


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

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

* [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script Samuel Martin
@ 2016-03-10  0:21   ` Arnout Vandecappelle
  0 siblings, 0 replies; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-10  0:21 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> The method describes here is only true for scripts using the shell
> modules from support/scripts/shell/*.
>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v6->v7:
> - new patch
> ---
>   docs/manual/debugging-buildroot.txt | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/docs/manual/debugging-buildroot.txt b/docs/manual/debugging-buildroot.txt
> index f575fc4..c497284 100644
> --- a/docs/manual/debugging-buildroot.txt
> +++ b/docs/manual/debugging-buildroot.txt
> @@ -41,3 +41,18 @@ The script has access to the following variables:
>     - +BINARIES_DIR+: the place where all binary files (aka images) are
>       stored
>     - +BASE_DIR+: the base output directory
> +
> +=== Debugging shell scripts in Buildroot

  If you introduce a subsection, then everything that comes before it should 
also be moved to a subsection. E.g.

[[debugging-buildroot]]

== Debugging Buildroot

=== Running scripts before and after each build step

It is possible to instrument the steps +Buildroot+ does when building


> +
> +For shell scripts using the _shell modules_ (available in
> ++support/scripts/shell/+), there is a couple of environment variables
                                     ^^ are

  but actually, there is only one...

  Regards,
  Arnout

> +you can set to trace the scripts
> +
> +  - +SHELL_DEBUG+ adjustes the debug level, from 0 to 3 (only errors to
> +    all debug messages)
> +
> +To enable these debug outputs:
> +
> +----
> +SHELL_DEBUG=3 make
> +----
>


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

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

* [Buildroot] [PATCH v7 06/18] core: make host *-config scripts relocatable
  2016-03-10  0:12   ` Arnout Vandecappelle
@ 2016-03-10 12:30     ` Samuel Martin
  0 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-10 12:30 UTC (permalink / raw)
  To: buildroot

On Thu, Mar 10, 2016 at 1:12 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 03/09/16 23:58, Samuel Martin wrote:
>>
>> This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix
>> computed
>> relatively to the script location.
>>
>> This patch only fixes *-config scripts located in the host area, those
>> from the target and staging area are already handled.
>>
>> This hook is a kind of brute-force sed on all files matching '*-config'
>> in the $(HOST_DIR)/usr/bin directory. This allows correctly handle
>> script whose name may depends on the host system, not the target
>> configuration (e.g. ncurses can be configured for the target to be
>> ncurses6w, so ncurses6w-config ; whereas for the host it will be
>> ncurses5, so ncurses5-config).
>> Since there way less host packages than target ones, it is reasonable to
>> assume that processing all *-config scripts found when a host package
>> (whose target package set the *_CONFIG_SCRIPTS variable) is fairly short
>> and does not add its overhead not even noticeable.
>
>
>  I don't agree with this on the principle that we like to handle things
> explicitly, rather than having some automagic stuff that is hard to
> pinpoint.

Right! I forgot this point that someone (certainly you) already
pinpointed during the last BR dev. days.

> So I think we should just set the appropriate
> HOST_*_CONFIG_SCRIPTS variables. To alleviate the situation, we could add a
> ?= in generic-package, so only the cases where the host script deviates from
> the target script (or where there is no target package) have to be handled.
> Probably just ncurses.

Let's do so


-- 
Samuel

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

* [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR
  2016-03-09 23:51   ` Arnout Vandecappelle
@ 2016-03-10 20:45     ` Thomas Petazzoni
  0 siblings, 0 replies; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-10 20:45 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 10 Mar 2016 00:51:11 +0100, Arnout Vandecappelle wrote:
> On 03/09/16 23:58, Samuel Martin wrote:
> > This changes saves a shell call and uses a variable automatically set
> > by make [1].
> 
>   Wouldn't it be better to just use $(CURDIR) everywhere then?

We could but I personally find TOPDIR to be clearer: it clearly
identifies the top-level directory of the Buildroot source tree. While
CURDIR could easily be mistaken as something else.

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

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

* [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible Samuel Martin
  2016-03-09 23:20   ` Yann E. MORIN
@ 2016-03-10 20:47   ` Thomas Petazzoni
  2016-03-12  9:11     ` Samuel Martin
  1 sibling, 1 reply; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-10 20:47 UTC (permalink / raw)
  To: buildroot

Samuel,

On Wed,  9 Mar 2016 23:58:45 +0100, Samuel Martin wrote:
> This change turns the staging/ symlink to a relative one when
> $(STAGING_DIR) points to some location in the Buildroot's output
> directory (i.e.: in $(BASE_DIR)/).
> 
> In any other case, the symlink target remains the absolute path
> to the sysroot location.
> 
> This slight change is a step forward toward a relocatable SDK.

Why is this important for a relocatable SDK ? The SDK is $(HOST_DIR),
and the staging symlink is just a convenience symlink which sits
outside of $(HOST_DIR), and hence outside the SDK.

So why should we care?

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

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

* [Buildroot] [PATCH v7 06/18] core: make host *-config scripts relocatable
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 06/18] core: make host " Samuel Martin
  2016-03-10  0:12   ` Arnout Vandecappelle
@ 2016-03-10 20:50   ` Thomas Petazzoni
  1 sibling, 0 replies; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-10 20:50 UTC (permalink / raw)
  To: buildroot

Samuel,

On Wed,  9 Mar 2016 23:58:47 +0100, Samuel Martin wrote:

> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index ffa21ee..845c8d3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -207,6 +207,18 @@ $(BUILD_DIR)/%/.stamp_host_installed:
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_CMDS)
>  	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
> +	$(Q)if test -n "$($(RAWNAME)_CONFIG_SCRIPTS)" ; then \

Using RAWNAME is not good here, because it prevents from having a
HOST_<pkg>_CONFIG_SCRIPTS value that is different from
<pkg>_CONFIG_SCRIPTS.

So, what you should do is:

 1/ Change the inner-generic-package macro to define
    HOST_<pkg>_CONFIG_SCRIPTS as having the value of
    <pkg>_CONFIG_SCRIPTS if not already defined. Like we do for the
    _SITE, _VERSION, _SOURCE, etc. variables.

 2/ Use $($(PKG)_CONFIG_SCRIPTS) here.

Best regards,

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

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

* [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable Samuel Martin
@ 2016-03-10 20:53   ` Thomas Petazzoni
  2016-03-10 21:33     ` Peter Korsgaard
  0 siblings, 1 reply; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-10 20:53 UTC (permalink / raw)
  To: buildroot

Samuel,

On Wed,  9 Mar 2016 23:58:48 +0100, Samuel Martin wrote:

> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> old mode 100644
> new mode 100755
> index 4dec487..aa4e484
> --- a/package/pkgconf/pkg-config.in
> +++ b/package/pkgconf/pkg-config.in
> @@ -1,2 +1,4 @@
>  #!/bin/sh
> -PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:- at PKG_CONFIG_LIBDIR@} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:- at STAGING_DIR@} $(dirname $0)/pkgconf @STATIC@ $@
> +STAGING_DIR="$(readlink -f "$(dirname "$0")/../..")/@STAGING_SUBDIR@"

I think Peter said on IRC that $0 is not guaranteed to be correct, and
that /proc/self/exe should be used instead.

> +TARGET_PKG_CONFIG_LIBDIR="$( echo @PKG_CONFIG_LIBDIR@ | sed -e "s@^@${STAGING_DIR}/@ ; s@:@:${STAGING_DIR}/@g" )"

I must say I hate those spaces after ( and before ). Especially when
the line just above doesn't have those spaces.

> +PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${TARGET_PKG_CONFIG_LIBDIR}} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${STAGING_DIR}} $(dirname $0)/pkgconf @STATIC@ $@
> diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
> index c8b0cba..d27672c 100644
> --- a/package/pkgconf/pkgconf.mk
> +++ b/package/pkgconf/pkgconf.mk
> @@ -19,8 +19,12 @@ endef
>  define HOST_PKGCONF_INSTALL_WRAPPER
>  	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
>  		$(HOST_DIR)/usr/bin/pkg-config
> -	$(SED) 's, at PKG_CONFIG_LIBDIR@,$(STAGING_DIR)/usr/lib/pkgconfig:$(STAGING_DIR)/usr/share/pkgconfig,' \
> -		-e 's, at STAGING_DIR@,$(STAGING_DIR),' \
> +	# @PKG_CONFIG_LIBDIR@ is a colon-separated list of paths to search.
> +	# These paths are relative to the root of the staging directory; they will
> +	# be automatically prefixed with the staging directory path by the wrapper
> +	# at runtime to ensure its relocatability.
> +	$(SED) 's, at PKG_CONFIG_LIBDIR@,/usr/lib/pkgconfig:/usr/share/pkgconfig,' \

I don't understand this first part of the expression. Why are you doing
a replacement if you're replacing by something that always has the same
value? Why don't you simply
hardcode /usr/lib/pkgconfig:/usr/share/pkgconfig in the wrapper?

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

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

* [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable
  2016-03-10 20:53   ` Thomas Petazzoni
@ 2016-03-10 21:33     ` Peter Korsgaard
  0 siblings, 0 replies; 57+ messages in thread
From: Peter Korsgaard @ 2016-03-10 21:33 UTC (permalink / raw)
  To: buildroot

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

 >> +STAGING_DIR="$(readlink -f "$(dirname "$0")/../..")/@STAGING_SUBDIR@"

 > I think Peter said on IRC that $0 is not guaranteed to be correct, and
 > that /proc/self/exe should be used instead.

Yes (and that is generally true), but as far as I can see it is
guaranteed for scripts on Linux by the logic in binfmt-script.c


 >> +TARGET_PKG_CONFIG_LIBDIR="$( echo @PKG_CONFIG_LIBDIR@ | sed -e
 >> "s@^@${STAGING_DIR}/@ ; s@:@:${STAGING_DIR}/@g" )"

 > I must say I hate those spaces after ( and before ). Especially when
 > the line just above doesn't have those spaces.

Consistency would be good, yes!

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
  2016-03-09 23:10   ` Yann E. MORIN
  2016-03-09 23:51   ` Arnout Vandecappelle
@ 2016-03-10 22:50   ` Thomas Petazzoni
  2 siblings, 0 replies; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-10 22:50 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Wed,  9 Mar 2016 23:58:43 +0100, Samuel Martin wrote:
> This changes saves a shell call and uses a variable automatically set
> by make [1].
> 
> [1] http://www.gnu.org/software/make/manual/make.html#Quick-Reference
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v6->v7:
> - none

Applied to master, thanks.

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

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

* [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation
  2016-03-09 23:09   ` Yann E. MORIN
@ 2016-03-12  8:46     ` Samuel Martin
  2016-03-22 22:10       ` Yann E. MORIN
  0 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-12  8:46 UTC (permalink / raw)
  To: buildroot

Yann, all,

On Thu, Mar 10, 2016 at 12:09 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Samuel, All,
>
> On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
>> This patch removes unnecessary files generated during installation, that
>> also are causing build machine paths leaking into the Buildroot's SDK.
>>
>> Note that similar cleanup commands can be found in other distributions
>> (e.g. gentoo [1]); thought this is not a valid reason.
>>
>> [1] https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-kernel/linux-headers/linux-headers-4.4.ebuild#n36
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v6->v7:
>> - improve commit log
>>
>> changes v5->v6:
>> - new patch
>> ---
>>  package/linux-headers/linux-headers.mk | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
>> index 0900778..7633257 100644
>> --- a/package/linux-headers/linux-headers.mk
>> +++ b/package/linux-headers/linux-headers.mk
>> @@ -123,6 +123,7 @@ define LINUX_HEADERS_INSTALL_STAGING_CMDS
>>                       HOSTCXX="$(HOSTCXX)" \
>>                       INSTALL_HDR_PATH=$(STAGING_DIR)/usr \
>>                       headers_install)
>> +     find $(STAGING_DIR)/usr \( -name .install -o -name ..install.cmd \) -delete
>
> I would further restrict that to $(STAGING_DIR)/usr/include . not
Well, I just reuse what is set for INSTALL_HDR_PATH, making any
assumption on what the package's install rules do.

>
> Also, Gentoo also has code to get rid of empty directories:
>     find $(STAGING_DIR)/usr -depth -type d -delete 2>/dev/null
In the Gentoo case, this is run on the package content, whereas in BR,
this would remove all empty directories in the shared sysroot.
So, why doing it for this package and not for the others?
Also, if we want to do this, it would be in the global infra, not here ;-)

>
> Although we don't really care about empty dirs in staging, TBH...
Indeed.

Regards,

-- 
Samuel

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-09 23:15   ` Yann E. MORIN
@ 2016-03-12  8:48     ` Samuel Martin
  0 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-12  8:48 UTC (permalink / raw)
  To: buildroot

Yann, all,

On Thu, Mar 10, 2016 at 12:15 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Samuel, All,
>
> On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
>> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>> they can be resolved differently, depending on each package build-system
>> (whether it uses the given paths or get the absolute canonical ones).
>>
>> Thus, to make easier tracking down host machine paths leaking into the
>> host, target or staging trees, the CURDIR and O variables are set to
>> their absolute canonical paths.
>>
>> Note that this change takes care of the makefile wrapper installed in
>> $(O) to avoid unneeded make recursion.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v6->v7:
>> - none
>>
>> changes v5->v6:
>> - new patch
>> ---
>>  Makefile | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 98c8dc7..52a92f6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -26,16 +26,30 @@
>>
>>  # Trick for always running with a fixed umask
>>  UMASK = 0022
>> -ifneq ($(shell umask),$(UMASK))
>> +
>> +# Check if we need to re-enter make for one or several of the following reasons:
>> +# 1- Wrong (too restrictive) umask:
>> +#    This prevents Buildroot and packages from creating files and directories.
>> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>> +#    This makes harder tracking and fixing host machine path leaks.
>> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>> +#    This makes harder tracking and fixing host machine path leaks.
>> +#
>> +# Note:
>> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>> +#   wrapper installed in the $(O).
>> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>
> This is a bit difficult to parse. What about setting intermediate
> variables, like so:
>
>     CUR_UMASK := $(shell umask)
>     REAL_CURDIR := $(realpath $(CURDIR))
>     O := $(patsubst %/.,%,$(O))
>     REAL_O := $(realpath $(O))
>
>     ifneq ($(CUR_UMASK):$(CURDIR):$(O),$(UMASK):$(REAL_CURDIR):$(REAL_O))
Fair enough

>
>>  .PHONY: _all $(MAKECMDGOALS)
>>
>>  $(MAKECMDGOALS): _all
>>       @:
>>
>>  _all:
>> -     @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>> +     umask $(UMASK) && \
>
> Please keep the command silent, do not remove the leading @
I'll fix it

Regards,

-- 
Samuel

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

* [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible
  2016-03-10 20:47   ` Thomas Petazzoni
@ 2016-03-12  9:11     ` Samuel Martin
  2016-03-12 13:12       ` Thomas Petazzoni
  0 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-12  9:11 UTC (permalink / raw)
  To: buildroot

Thomas, all,

On Thu, Mar 10, 2016 at 9:47 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Samuel,
>
> On Wed,  9 Mar 2016 23:58:45 +0100, Samuel Martin wrote:
>> This change turns the staging/ symlink to a relative one when
>> $(STAGING_DIR) points to some location in the Buildroot's output
>> directory (i.e.: in $(BASE_DIR)/).
>>
>> In any other case, the symlink target remains the absolute path
>> to the sysroot location.
>>
>> This slight change is a step forward toward a relocatable SDK.
>
> Why is this important for a relocatable SDK ? The SDK is $(HOST_DIR),
> and the staging symlink is just a convenience symlink which sits
> outside of $(HOST_DIR), and hence outside the SDK.
Right, but what about consistency in BR? if we do all the job for
having a full relocatable SDK, why should we keep this symlink an
absolute one?
Also, one of my tests in the relocatable SDK work is moving around the
whole output directory; so I need this patch in this case. Maybe this
is not a valid case?

Regards,

-- 
Samuel

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

* [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable
  2016-03-10  0:07     ` Arnout Vandecappelle
@ 2016-03-12  9:22       ` Samuel Martin
  0 siblings, 0 replies; 57+ messages in thread
From: Samuel Martin @ 2016-03-12  9:22 UTC (permalink / raw)
  To: buildroot

Yann, Arnout, all,

On Thu, Mar 10, 2016 at 1:07 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 03/10/16 00:24, Yann E. MORIN wrote:
>>
>> Samuel, All,
>>
>> On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
>>>
>>> This change adjusts the _CONFIG_SCRIPTS hook to set add {exec_,}prefix
>>> computed
>>> relatively to the script location.
>>>
>>> This patch hook only fixes *-config scripts located in the staging area,
>>> the target ones are already removed. A follow-up change will fix those
>>> from the HOST_DIR location.
>>>
>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>
>>> ---
>>> changes v6->v7:
>>> - none
>>>
>>> changes v5->v6:
>>> - new patch
>>> ---
>>>   package/pkg-generic.mk | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>> index 3904c09..ffa21ee 100644
>>> --- a/package/pkg-generic.mk
>>> +++ b/package/pkg-generic.mk
>>> @@ -240,7 +240,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>>>                 $(call MESSAGE,"Fixing package configuration files") ;\
>>>                         $(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
>>>                                 -e "s,$(STAGING_DIR), at STAGING_DIR@,g" \
>>> -                               -e
>>> "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
>>> +                               -e
>>> "s,^\(exec_\)\?prefix=.*,\1prefix=\`dirname \$$0\`/../../usr,g" \
>>
>>
>> I'd prefer we use the $() form when calling subshells:
So am I ;-)

>>
>>      -e "s,^\(exec_\)\?prefix=.*,\1prefix=$$(dirname \$$0)/../../usr,g"
>
>                                             ^ missing \ here
>
>>
>> I know we switched to using `` in some locations, but that was because
>> those locations may be called with various levels of $(eval), and that
>> would cause $$() to be evaluated too early in some situations.
>
I don't remember the exact reason of this choice... either consistency
WRT the `` use, and/or to make the line easier to read avoiding
duplicating $$(...) and \... :-)

>
>  But we like things to be consistent, so I'd stick to `.
>
>  OTOH, this particular bit is not going to be expanded by the shell, but is
> actually inserted into the script. So perhaps we don't _want_ to be
> consistent here.

I don't have strong opinion about this, I'll check what we usually do
in other wrappers.

>
>  But like Yann says, it's not so important.
>
>  Regards,
>  Arnout
>
>
>>
>> However, here we are in a real rule, so there's no such ambiguity.
>>
>> But I won't block it just for that. I you agree with my proposed change,
>> that's OK; if you don't, that's OK too:
>>
>> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Thx.

Regards,


-- 
Samuel

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

* [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible
  2016-03-12  9:11     ` Samuel Martin
@ 2016-03-12 13:12       ` Thomas Petazzoni
  0 siblings, 0 replies; 57+ messages in thread
From: Thomas Petazzoni @ 2016-03-12 13:12 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, 12 Mar 2016 10:11:29 +0100, Samuel Martin wrote:

> > Why is this important for a relocatable SDK ? The SDK is $(HOST_DIR),
> > and the staging symlink is just a convenience symlink which sits
> > outside of $(HOST_DIR), and hence outside the SDK.
> Right, but what about consistency in BR? if we do all the job for
> having a full relocatable SDK, why should we keep this symlink an
> absolute one?

Because there is no point in adding more complexity in BR if it's
useless.

> Also, one of my tests in the relocatable SDK work is moving around the
> whole output directory; so I need this patch in this case. Maybe this
> is not a valid case?

I don't see the point of moving the entire output/ directory, because
there are lots and lots and lots of absolute paths all over the place
in the build directory of packages.

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

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
       [not found]   ` <CAFt09wNvtXGcfZ3PFZOgGu+bDvp6mEr44uwxa_ryjeP6DDQKBQ@mail.gmail.com>
@ 2016-03-21  1:45     ` Matthew Weber
  2016-03-21  5:21       ` Samuel Martin
  2016-03-22 22:07       ` Yann E. MORIN
  0 siblings, 2 replies; 57+ messages in thread
From: Matthew Weber @ 2016-03-21  1:45 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
> Samuel,
>
> On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>> they can be resolved differently, depending on each package build-system
>> (whether it uses the given paths or get the absolute canonical ones).
>>
>> Thus, to make easier tracking down host machine paths leaking into the
>> host, target or staging trees, the CURDIR and O variables are set to
>> their absolute canonical paths.
>>
>> Note that this change takes care of the makefile wrapper installed in
>> $(O) to avoid unneeded make recursion.
>>
>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>> ---
>> changes v6->v7:
>> - none
>>
>> changes v5->v6:
>> - new patch
>> ---
>>  Makefile | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 98c8dc7..52a92f6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -26,16 +26,30 @@
>>
>>  # Trick for always running with a fixed umask
>>  UMASK = 0022
>> -ifneq ($(shell umask),$(UMASK))
>> +
>> +# Check if we need to re-enter make for one or several of the following reasons:
>> +# 1- Wrong (too restrictive) umask:
>> +#    This prevents Buildroot and packages from creating files and directories.
>> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>> +#    This makes harder tracking and fixing host machine path leaks.
>> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>> +#    This makes harder tracking and fixing host machine path leaks.
>> +#
>> +# Note:
>> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>> +#   wrapper installed in the $(O).
>> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>>  .PHONY: _all $(MAKECMDGOALS)
>>
>>  $(MAKECMDGOALS): _all
>>         @:
>>
>>  _all:
>> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>> +       umask $(UMASK) && \
>> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>
> I've applied your v7 patches with these changes to the tip of
> buildroot and currently run into the following error.  I'm still
> poking around trying to understand why.
>
> $ make V=1 qemu_x86_defconfig
> umask 0022 && \
>         make -C /opt/buildroot --no-print-directory \
>                 qemu_x86_defconfig O=
> mkdir: missing operand
> Try 'mkdir --help' for more information.
> Makefile:156: *** output directory "" does not exist.  Stop.
> Makefile:48: recipe for target '_all' failed
> make: *** [_all] Error 2
>
> <snip>
>
> --
> Thanks,
> Matt

Sorry, didn't CC.  2nd attempt, see above.

-- 
Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-21  1:45     ` Matthew Weber
@ 2016-03-21  5:21       ` Samuel Martin
  2016-03-21 11:40         ` Matthew Weber
  2016-03-22 22:07       ` Yann E. MORIN
  1 sibling, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-21  5:21 UTC (permalink / raw)
  To: buildroot

On Mon, Mar 21, 2016 at 2:45 AM, Matthew Weber <matt@thewebers.ws> wrote:
> Samuel, All,
>
> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>> Samuel,
>>
>> On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>>> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>>> they can be resolved differently, depending on each package build-system
>>> (whether it uses the given paths or get the absolute canonical ones).
>>>
>>> Thus, to make easier tracking down host machine paths leaking into the
>>> host, target or staging trees, the CURDIR and O variables are set to
>>> their absolute canonical paths.
>>>
>>> Note that this change takes care of the makefile wrapper installed in
>>> $(O) to avoid unneeded make recursion.
>>>
>>> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>
>>> ---
>>> changes v6->v7:
>>> - none
>>>
>>> changes v5->v6:
>>> - new patch
>>> ---
>>>  Makefile | 22 ++++++++++++++++++----
>>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 98c8dc7..52a92f6 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -26,16 +26,30 @@
>>>
>>>  # Trick for always running with a fixed umask
>>>  UMASK = 0022
>>> -ifneq ($(shell umask),$(UMASK))
>>> +
>>> +# Check if we need to re-enter make for one or several of the following reasons:
>>> +# 1- Wrong (too restrictive) umask:
>>> +#    This prevents Buildroot and packages from creating files and directories.
>>> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>>> +#    This makes harder tracking and fixing host machine path leaks.
>>> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>>> +#    This makes harder tracking and fixing host machine path leaks.
>>> +#
>>> +# Note:
>>> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>>> +#   wrapper installed in the $(O).
>>> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>>>  .PHONY: _all $(MAKECMDGOALS)
>>>
>>>  $(MAKECMDGOALS): _all
>>>         @:
>>>
>>>  _all:
>>> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>>> +       umask $(UMASK) && \
>>> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>>> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>>
>> I've applied your v7 patches with these changes to the tip of
>> buildroot and currently run into the following error.  I'm still
>> poking around trying to understand why.
>>
>> $ make V=1 qemu_x86_defconfig
>> umask 0022 && \
>>         make -C /opt/buildroot --no-print-directory \
>>                 qemu_x86_defconfig O=
>> mkdir: missing operand
>> Try 'mkdir --help' for more information.
>> Makefile:156: *** output directory "" does not exist.  Stop.
>> Makefile:48: recipe for target '_all' failed
>> make: *** [_all] Error 2
>>
>> <snip>

hmm... :-/
I cannot reproduce your issue (running make from inside or outside the
Buildroot directory).
Did you apply the whole series, or have you narrowed down this issue
to this patch?

>>
>> --
>> Thanks,
>> Matt
>
> Sorry, didn't CC.  2nd attempt, see above.
>
> --
> Thanks,
> Matt

Thanks for the feedback
Regards,

-- 
Samuel

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-21  5:21       ` Samuel Martin
@ 2016-03-21 11:40         ` Matthew Weber
  2016-03-22  3:03           ` Matthew Weber
  0 siblings, 1 reply; 57+ messages in thread
From: Matthew Weber @ 2016-03-21 11:40 UTC (permalink / raw)
  To: buildroot

Samuel,

On Mon, Mar 21, 2016 at 12:21 AM, Samuel Martin <s.martin49@gmail.com> wrote:
> On Mon, Mar 21, 2016 at 2:45 AM, Matthew Weber <matt@thewebers.ws> wrote:
>> Samuel, All,
>>
>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>> Samuel,
>>>

<snip>
>
> hmm... :-/
> I cannot reproduce your issue (running make from inside or outside the
> Buildroot directory).
> Did you apply the whole series, or have you narrowed down this issue
> to this patch?
>

Shoot, looks like my master is out of sync.  I'm missing commits since
March9th, so I don't have the 02/18 of your series.  I'll retest later
today when I get back to this.

Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-21 11:40         ` Matthew Weber
@ 2016-03-22  3:03           ` Matthew Weber
  2016-03-22  5:55             ` Samuel Martin
  0 siblings, 1 reply; 57+ messages in thread
From: Matthew Weber @ 2016-03-22  3:03 UTC (permalink / raw)
  To: buildroot

Samuel,

On Mon, Mar 21, 2016 at 6:40 AM, Matthew Weber <matt@thewebers.ws> wrote:
> Samuel,
>
> On Mon, Mar 21, 2016 at 12:21 AM, Samuel Martin <s.martin49@gmail.com> wrote:
>> On Mon, Mar 21, 2016 at 2:45 AM, Matthew Weber <matt@thewebers.ws> wrote:
>>> Samuel, All,
>>>
>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>> Samuel,
>>>>
>
> <snip>
>>

Narrowed it down to removing "O=$(realpath $(O))" on line 50 in the
Makefile resolves the build error.  If I understand it correctly this
is declaring O= empty and it causes the logic on 127 to evaluate to
False and O= ends up as empty.

I applied your 02/18 patch on top of
0e1547a87cfd51e4fedf1cb7837f77e4f66c4052 when I tested.


-- 
Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-22  3:03           ` Matthew Weber
@ 2016-03-22  5:55             ` Samuel Martin
  2016-03-22 11:10               ` Matthew Weber
  0 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-22  5:55 UTC (permalink / raw)
  To: buildroot

Hi Matthew,

On Tue, Mar 22, 2016 at 4:03 AM, Matthew Weber <matt@thewebers.ws> wrote:
> Samuel,
>
> On Mon, Mar 21, 2016 at 6:40 AM, Matthew Weber <matt@thewebers.ws> wrote:
>> Samuel,
>>
>> On Mon, Mar 21, 2016 at 12:21 AM, Samuel Martin <s.martin49@gmail.com> wrote:
>>> On Mon, Mar 21, 2016 at 2:45 AM, Matthew Weber <matt@thewebers.ws> wrote:
>>>> Samuel, All,
>>>>
>>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>>> Samuel,
>>>>>
>>
>> <snip>
>>>
>
> Narrowed it down to removing "O=$(realpath $(O))" on line 50 in the
> Makefile resolves the build error.  If I understand it correctly this
> is declaring O= empty and it causes the logic on 127 to evaluate to
> False and O= ends up as empty.
>
> I applied your 02/18 patch on top of
> 0e1547a87cfd51e4fedf1cb7837f77e4f66c4052 when I tested.
>
Thanks for investigating this.
Unfortunately, I could not reproduce this issue here with make-4.1. :-/
(I wonder whether this could be related to the make version...)

Anyway, following Yann's review, I have reworked this patch [1].
If you give a try to this patch, care to tell me if it fixes your issue.

[1] https://github.com/tSed/buildroot/commit/c55eeee30fe6106354c0f2cb6ce6ecb46972503a

Regards,

>
> --
> Thanks,
> Matt


-- 
Samuel

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-22  5:55             ` Samuel Martin
@ 2016-03-22 11:10               ` Matthew Weber
  0 siblings, 0 replies; 57+ messages in thread
From: Matthew Weber @ 2016-03-22 11:10 UTC (permalink / raw)
  To: buildroot

Samuel,

On Tue, Mar 22, 2016 at 12:55 AM, Samuel Martin <s.martin49@gmail.com> wrote:
> Hi Matthew,
>
> On Tue, Mar 22, 2016 at 4:03 AM, Matthew Weber <matt@thewebers.ws> wrote:
>> Samuel,
>>
>> On Mon, Mar 21, 2016 at 6:40 AM, Matthew Weber <matt@thewebers.ws> wrote:
>>> Samuel,
>>>
>>> On Mon, Mar 21, 2016 at 12:21 AM, Samuel Martin <s.martin49@gmail.com> wrote:
>>>> On Mon, Mar 21, 2016 at 2:45 AM, Matthew Weber <matt@thewebers.ws> wrote:
>>>>> Samuel, All,
>>>>>
>>>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>>>> Samuel,
>>>>>>
>>>
>>> <snip>
>>>>
>>
>> Narrowed it down to removing "O=$(realpath $(O))" on line 50 in the
>> Makefile resolves the build error.  If I understand it correctly this
>> is declaring O= empty and it causes the logic on 127 to evaluate to
>> False and O= ends up as empty.
>>
>> I applied your 02/18 patch on top of
>> 0e1547a87cfd51e4fedf1cb7837f77e4f66c4052 when I tested.
>>
> Thanks for investigating this.
> Unfortunately, I could not reproduce this issue here with make-4.1. :-/
> (I wonder whether this could be related to the make version...)
>
> Anyway, following Yann's review, I have reworked this patch [1].
> If you give a try to this patch, care to tell me if it fixes your issue.
>
> [1] https://github.com/tSed/buildroot/commit/c55eeee30fe6106354c0f2cb6ce6ecb46972503a
>

Looks like I have Make 4.0.

Thanks for the patch, output looks like same with that version.  I'll
jump on another buildmachine at work today and see if I can reproduce
this.

-- 
Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-21  1:45     ` Matthew Weber
  2016-03-21  5:21       ` Samuel Martin
@ 2016-03-22 22:07       ` Yann E. MORIN
  2016-03-22 23:11         ` Matthew Weber
  1 sibling, 1 reply; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-22 22:07 UTC (permalink / raw)
  To: buildroot

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
> >> they can be resolved differently, depending on each package build-system
> >> (whether it uses the given paths or get the absolute canonical ones).
> >>
> >> Thus, to make easier tracking down host machine paths leaking into the
> >> host, target or staging trees, the CURDIR and O variables are set to
> >> their absolute canonical paths.
> >>
> >> Note that this change takes care of the makefile wrapper installed in
> >> $(O) to avoid unneeded make recursion.
> >>
> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> >>
> >> ---
> >> changes v6->v7:
> >> - none
> >>
> >> changes v5->v6:
> >> - new patch
> >> ---
> >>  Makefile | 22 ++++++++++++++++++----
> >>  1 file changed, 18 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 98c8dc7..52a92f6 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -26,16 +26,30 @@
> >>
> >>  # Trick for always running with a fixed umask
> >>  UMASK = 0022
> >> -ifneq ($(shell umask),$(UMASK))
> >> +
> >> +# Check if we need to re-enter make for one or several of the following reasons:
> >> +# 1- Wrong (too restrictive) umask:
> >> +#    This prevents Buildroot and packages from creating files and directories.
> >> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
> >> +#    This makes harder tracking and fixing host machine path leaks.
> >> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
> >> +#    This makes harder tracking and fixing host machine path leaks.
> >> +#
> >> +# Note:
> >> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
> >> +#   wrapper installed in the $(O).
> >> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
> >>  .PHONY: _all $(MAKECMDGOALS)
> >>
> >>  $(MAKECMDGOALS): _all
> >>         @:
> >>
> >>  _all:
> >> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
> >> +       umask $(UMASK) && \
> >> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
> >> +                       $(MAKECMDGOALS) O=$(realpath $(O))
> >
> > I've applied your v7 patches with these changes to the tip of
> > buildroot and currently run into the following error.  I'm still
> > poking around trying to understand why.
> >
> > $ make V=1 qemu_x86_defconfig
> > umask 0022 && \
> >         make -C /opt/buildroot --no-print-directory \
> >                 qemu_x86_defconfig O=

Why are you opassing an empty O= option here?

Can you reproduce without it?

Regards,
Yann E. MORIN.

> > mkdir: missing operand
> > Try 'mkdir --help' for more information.
> > Makefile:156: *** output directory "" does not exist.  Stop.
> > Makefile:48: recipe for target '_all' failed
> > make: *** [_all] Error 2
> >
> > <snip>
> >
> > --
> > Thanks,
> > Matt
> 
> Sorry, didn't CC.  2nd attempt, see above.
> 
> -- 
> Thanks,
> Matt
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation
  2016-03-12  8:46     ` Samuel Martin
@ 2016-03-22 22:10       ` Yann E. MORIN
  0 siblings, 0 replies; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-22 22:10 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-03-12 09:46 +0100, Samuel Martin spake thusly:
> On Thu, Mar 10, 2016 at 12:09 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:
> >> This patch removes unnecessary files generated during installation, that
> >> also are causing build machine paths leaking into the Buildroot's SDK.
> >>
> >> Note that similar cleanup commands can be found in other distributions
> >> (e.g. gentoo [1]); thought this is not a valid reason.
> >>
> >> [1] https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-kernel/linux-headers/linux-headers-4.4.ebuild#n36
> >>
> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> >>
> >> ---
> >> changes v6->v7:
> >> - improve commit log
> >>
> >> changes v5->v6:
> >> - new patch
> >> ---
> >>  package/linux-headers/linux-headers.mk | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/package/linux-headers/linux-headers.mk b/package/linux-headers/linux-headers.mk
> >> index 0900778..7633257 100644
> >> --- a/package/linux-headers/linux-headers.mk
> >> +++ b/package/linux-headers/linux-headers.mk
> >> @@ -123,6 +123,7 @@ define LINUX_HEADERS_INSTALL_STAGING_CMDS
> >>                       HOSTCXX="$(HOSTCXX)" \
> >>                       INSTALL_HDR_PATH=$(STAGING_DIR)/usr \
> >>                       headers_install)
> >> +     find $(STAGING_DIR)/usr \( -name .install -o -name ..install.cmd \) -delete
> >
> > I would further restrict that to $(STAGING_DIR)/usr/include . not
> Well, I just reuse what is set for INSTALL_HDR_PATH, making any
> assumption on what the package's install rules do.

Well, this is only installing headers, so in $(STAGING_DIR)/usr/include.

> > Also, Gentoo also has code to get rid of empty directories:
> >     find $(STAGING_DIR)/usr -depth -type d -delete 2>/dev/null
> In the Gentoo case, this is run on the package content, whereas in BR,
> this would remove all empty directories in the shared sysroot.
> So, why doing it for this package and not for the others?
> Also, if we want to do this, it would be in the global infra, not here ;-)
> 
> >
> > Although we don't really care about empty dirs in staging, TBH...
> Indeed.

Yep, I was just pointing this out, since you referenced Gentoo. ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-22 22:07       ` Yann E. MORIN
@ 2016-03-22 23:11         ` Matthew Weber
  2016-03-23  0:54           ` Matthew Weber
  2016-03-23 17:41           ` Yann E. MORIN
  0 siblings, 2 replies; 57+ messages in thread
From: Matthew Weber @ 2016-03-22 23:11 UTC (permalink / raw)
  To: buildroot

Yann, All

On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Matthew, All,
>
> On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>> >> they can be resolved differently, depending on each package build-system
>> >> (whether it uses the given paths or get the absolute canonical ones).
>> >>
>> >> Thus, to make easier tracking down host machine paths leaking into the
>> >> host, target or staging trees, the CURDIR and O variables are set to
>> >> their absolute canonical paths.
>> >>
>> >> Note that this change takes care of the makefile wrapper installed in
>> >> $(O) to avoid unneeded make recursion.
>> >>
>> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>> >>
>> >> ---
>> >> changes v6->v7:
>> >> - none
>> >>
>> >> changes v5->v6:
>> >> - new patch
>> >> ---
>> >>  Makefile | 22 ++++++++++++++++++----
>> >>  1 file changed, 18 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/Makefile b/Makefile
>> >> index 98c8dc7..52a92f6 100644
>> >> --- a/Makefile
>> >> +++ b/Makefile
>> >> @@ -26,16 +26,30 @@
>> >>
>> >>  # Trick for always running with a fixed umask
>> >>  UMASK = 0022
>> >> -ifneq ($(shell umask),$(UMASK))
>> >> +
>> >> +# Check if we need to re-enter make for one or several of the following reasons:
>> >> +# 1- Wrong (too restrictive) umask:
>> >> +#    This prevents Buildroot and packages from creating files and directories.
>> >> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>> >> +#    This makes harder tracking and fixing host machine path leaks.
>> >> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>> >> +#    This makes harder tracking and fixing host machine path leaks.
>> >> +#
>> >> +# Note:
>> >> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>> >> +#   wrapper installed in the $(O).
>> >> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>> >>  .PHONY: _all $(MAKECMDGOALS)
>> >>
>> >>  $(MAKECMDGOALS): _all
>> >>         @:
>> >>
>> >>  _all:
>> >> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>> >> +       umask $(UMASK) && \
>> >> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>> >> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>> >
>> > I've applied your v7 patches with these changes to the tip of
>> > buildroot and currently run into the following error.  I'm still
>> > poking around trying to understand why.
>> >
>> > $ make V=1 qemu_x86_defconfig
>> > umask 0022 && \
>> >         make -C /opt/buildroot --no-print-directory \
>> >                 qemu_x86_defconfig O=
>
> Why are you opassing an empty O= option here?
>

I'm not passing the O= .

However, if I modify the patch and remove that from the target in the
Makefile ie. remove the "O=$(realpath $(O))" , things build ok.

> Can you reproduce without it?
>
> Regards,
> Yann E. MORIN.
>
>> > mkdir: missing operand
>> > Try 'mkdir --help' for more information.
>> > Makefile:156: *** output directory "" does not exist.  Stop.
>> > Makefile:48: recipe for target '_all' failed
>> > make: *** [_all] Error 2
>> >
>> > <snip>
>> >
>> > --
>> > Thanks,
>> > Matt
>>
>> Sorry, didn't CC.  2nd attempt, see above.
>>
>> --
>> Thanks,
>> Matt
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'



-- 
Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-22 23:11         ` Matthew Weber
@ 2016-03-23  0:54           ` Matthew Weber
  2016-03-23  1:36             ` Matthew Weber
  2016-03-23 17:41           ` Yann E. MORIN
  1 sibling, 1 reply; 57+ messages in thread
From: Matthew Weber @ 2016-03-23  0:54 UTC (permalink / raw)
  To: buildroot

Yann, Samuel, All

On Tue, Mar 22, 2016 at 6:11 PM, Matthew Weber <matt@thewebers.ws> wrote:
> Yann, All
>
> On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> Matthew, All,
>>
>> On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>>> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>>> >> they can be resolved differently, depending on each package build-system
>>> >> (whether it uses the given paths or get the absolute canonical ones).
>>> >>
>>> >> Thus, to make easier tracking down host machine paths leaking into the
>>> >> host, target or staging trees, the CURDIR and O variables are set to
>>> >> their absolute canonical paths.
>>> >>
>>> >> Note that this change takes care of the makefile wrapper installed in
>>> >> $(O) to avoid unneeded make recursion.
>>> >>
>>> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>> >>
>>> >> ---
>>> >> changes v6->v7:
>>> >> - none
>>> >>
>>> >> changes v5->v6:
>>> >> - new patch
>>> >> ---
>>> >>  Makefile | 22 ++++++++++++++++++----
>>> >>  1 file changed, 18 insertions(+), 4 deletions(-)
>>> >>
>>> >> diff --git a/Makefile b/Makefile
>>> >> index 98c8dc7..52a92f6 100644
>>> >> --- a/Makefile
>>> >> +++ b/Makefile
>>> >> @@ -26,16 +26,30 @@
>>> >>
>>> >>  # Trick for always running with a fixed umask
>>> >>  UMASK = 0022
>>> >> -ifneq ($(shell umask),$(UMASK))
>>> >> +
>>> >> +# Check if we need to re-enter make for one or several of the following reasons:
>>> >> +# 1- Wrong (too restrictive) umask:
>>> >> +#    This prevents Buildroot and packages from creating files and directories.
>>> >> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>> >> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>> >> +#
>>> >> +# Note:
>>> >> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>>> >> +#   wrapper installed in the $(O).
>>> >> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>>> >>  .PHONY: _all $(MAKECMDGOALS)
>>> >>
>>> >>  $(MAKECMDGOALS): _all
>>> >>         @:
>>> >>
>>> >>  _all:
>>> >> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>>> >> +       umask $(UMASK) && \
>>> >> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>>> >> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>>> >
>>> > I've applied your v7 patches with these changes to the tip of
>>> > buildroot and currently run into the following error.  I'm still
>>> > poking around trying to understand why.
>>> >
>>> > $ make V=1 qemu_x86_defconfig
>>> > umask 0022 && \
>>> >         make -C /opt/buildroot --no-print-directory \
>>> >                 qemu_x86_defconfig O=
>>
>> Why are you opassing an empty O= option here?
>>
>
> I'm not passing the O= .
>
> However, if I modify the patch and remove that from the target in the
> Makefile ie. remove the "O=$(realpath $(O))" , things build ok.

I tested this tonight on another server with Make version 3.81 and
reproduced with a fresh buildroot checkout
(f34c69da2bfa2a485671dbbcf1ee24ad7409b63b) plus applying the commit
mentioned as the latest from Samuel's fork.

Same behavior, if I edit the Makefile and remove the "O=$(realpath
$(O))" , the defconfig sets up as expected

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-23  0:54           ` Matthew Weber
@ 2016-03-23  1:36             ` Matthew Weber
  2016-03-25  6:31               ` Samuel Martin
  0 siblings, 1 reply; 57+ messages in thread
From: Matthew Weber @ 2016-03-23  1:36 UTC (permalink / raw)
  To: buildroot

Yann, Samuel

On Tue, Mar 22, 2016 at 7:54 PM, Matthew Weber <matt@thewebers.ws> wrote:
> Yann, Samuel, All
>
> On Tue, Mar 22, 2016 at 6:11 PM, Matthew Weber <matt@thewebers.ws> wrote:
>> Yann, All
>>
>> On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>> Matthew, All,
>>>
>>> On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
>>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>>>> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>>>> >> they can be resolved differently, depending on each package build-system
>>>> >> (whether it uses the given paths or get the absolute canonical ones).
>>>> >>
>>>> >> Thus, to make easier tracking down host machine paths leaking into the
>>>> >> host, target or staging trees, the CURDIR and O variables are set to
>>>> >> their absolute canonical paths.
>>>> >>
>>>> >> Note that this change takes care of the makefile wrapper installed in
>>>> >> $(O) to avoid unneeded make recursion.
>>>> >>
>>>> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>> >>
>>>> >> ---
>>>> >> changes v6->v7:
>>>> >> - none
>>>> >>
>>>> >> changes v5->v6:
>>>> >> - new patch
>>>> >> ---
>>>> >>  Makefile | 22 ++++++++++++++++++----
>>>> >>  1 file changed, 18 insertions(+), 4 deletions(-)
>>>> >>
>>>> >> diff --git a/Makefile b/Makefile
>>>> >> index 98c8dc7..52a92f6 100644
>>>> >> --- a/Makefile
>>>> >> +++ b/Makefile
>>>> >> @@ -26,16 +26,30 @@
>>>> >>
>>>> >>  # Trick for always running with a fixed umask
>>>> >>  UMASK = 0022
>>>> >> -ifneq ($(shell umask),$(UMASK))
>>>> >> +
>>>> >> +# Check if we need to re-enter make for one or several of the following reasons:
>>>> >> +# 1- Wrong (too restrictive) umask:
>>>> >> +#    This prevents Buildroot and packages from creating files and directories.
>>>> >> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>>> >> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>>> >> +#
>>>> >> +# Note:
>>>> >> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>>>> >> +#   wrapper installed in the $(O).
>>>> >> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>>>> >>  .PHONY: _all $(MAKECMDGOALS)
>>>> >>
>>>> >>  $(MAKECMDGOALS): _all
>>>> >>         @:
>>>> >>
>>>> >>  _all:
>>>> >> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>>>> >> +       umask $(UMASK) && \
>>>> >> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>>>> >> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>>>> >
>>>> > I've applied your v7 patches with these changes to the tip of
>>>> > buildroot and currently run into the following error.  I'm still
>>>> > poking around trying to understand why.
>>>> >
>>>> > $ make V=1 qemu_x86_defconfig
>>>> > umask 0022 && \
>>>> >         make -C /opt/buildroot --no-print-directory \
>>>> >                 qemu_x86_defconfig O=
>>>
>>> Why are you opassing an empty O= option here?
>>>
>>
>> I'm not passing the O= .
>>
>> However, if I modify the patch and remove that from the target in the
>> Makefile ie. remove the "O=$(realpath $(O))" , things build ok.
>
> I tested this tonight on another server with Make version 3.81 and
> reproduced with a fresh buildroot checkout
> (f34c69da2bfa2a485671dbbcf1ee24ad7409b63b) plus applying the commit
> mentioned as the latest from Samuel's fork.
>
> Same behavior, if I edit the Makefile and remove the "O=$(realpath
> $(O))" , the defconfig sets up as expected

One additional detail, I cloned https://github.com/tSed/buildroot.git
and checkout c55eeee30fe6106354c0f2cb6ce6ecb46972503a.  Then "make
qemu_x86_defconfig" and reproduced it.  I'll start dissecting the
other commits past that one.  Something must be masking it.

-- 
Thanks,
Matt

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-22 23:11         ` Matthew Weber
  2016-03-23  0:54           ` Matthew Weber
@ 2016-03-23 17:41           ` Yann E. MORIN
  1 sibling, 0 replies; 57+ messages in thread
From: Yann E. MORIN @ 2016-03-23 17:41 UTC (permalink / raw)
  To: buildroot

Matthew, All,

On 2016-03-22 18:11 -0500, Matthew Weber spake thusly:
> On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
> >> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
> >> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
> >> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
> >> >> they can be resolved differently, depending on each package build-system
> >> >> (whether it uses the given paths or get the absolute canonical ones).
> >> >>
> >> >> Thus, to make easier tracking down host machine paths leaking into the
> >> >> host, target or staging trees, the CURDIR and O variables are set to
> >> >> their absolute canonical paths.
> >> >>
> >> >> Note that this change takes care of the makefile wrapper installed in
> >> >> $(O) to avoid unneeded make recursion.
> >> >>
> >> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
[--SNIP--]
> >> > I've applied your v7 patches with these changes to the tip of
> >> > buildroot and currently run into the following error.  I'm still
> >> > poking around trying to understand why.
> >> >
> >> > $ make V=1 qemu_x86_defconfig
> >> > umask 0022 && \
> >> >         make -C /opt/buildroot --no-print-directory \
> >> >                 qemu_x86_defconfig O=
> >
> > Why are you opassing an empty O= option here?
> >
> 
> I'm not passing the O= .

Damned, I see. It's passed by the sub-make. My bad.

Otherwise, I know why it fails: we should not try to canonicalise $(O)
if it is not set. That's that simple.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-23  1:36             ` Matthew Weber
@ 2016-03-25  6:31               ` Samuel Martin
  2016-03-26 12:21                 ` Arnout Vandecappelle
  0 siblings, 1 reply; 57+ messages in thread
From: Samuel Martin @ 2016-03-25  6:31 UTC (permalink / raw)
  To: buildroot

On Wed, Mar 23, 2016 at 2:36 AM, Matthew Weber <matt@thewebers.ws> wrote:
> Yann, Samuel
>
> On Tue, Mar 22, 2016 at 7:54 PM, Matthew Weber <matt@thewebers.ws> wrote:
>> Yann, Samuel, All
>>
>> On Tue, Mar 22, 2016 at 6:11 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>> Yann, All
>>>
>>> On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>> Matthew, All,
>>>>
>>>> On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
>>>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>>> > On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>>>>> >> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
>>>>> >> they can be resolved differently, depending on each package build-system
>>>>> >> (whether it uses the given paths or get the absolute canonical ones).
>>>>> >>
>>>>> >> Thus, to make easier tracking down host machine paths leaking into the
>>>>> >> host, target or staging trees, the CURDIR and O variables are set to
>>>>> >> their absolute canonical paths.
>>>>> >>
>>>>> >> Note that this change takes care of the makefile wrapper installed in
>>>>> >> $(O) to avoid unneeded make recursion.
>>>>> >>
>>>>> >> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>>>> >>
>>>>> >> ---
>>>>> >> changes v6->v7:
>>>>> >> - none
>>>>> >>
>>>>> >> changes v5->v6:
>>>>> >> - new patch
>>>>> >> ---
>>>>> >>  Makefile | 22 ++++++++++++++++++----
>>>>> >>  1 file changed, 18 insertions(+), 4 deletions(-)
>>>>> >>
>>>>> >> diff --git a/Makefile b/Makefile
>>>>> >> index 98c8dc7..52a92f6 100644
>>>>> >> --- a/Makefile
>>>>> >> +++ b/Makefile
>>>>> >> @@ -26,16 +26,30 @@
>>>>> >>
>>>>> >>  # Trick for always running with a fixed umask
>>>>> >>  UMASK = 0022
>>>>> >> -ifneq ($(shell umask),$(UMASK))
>>>>> >> +
>>>>> >> +# Check if we need to re-enter make for one or several of the following reasons:
>>>>> >> +# 1- Wrong (too restrictive) umask:
>>>>> >> +#    This prevents Buildroot and packages from creating files and directories.
>>>>> >> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
>>>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>>>> >> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
>>>>> >> +#    This makes harder tracking and fixing host machine path leaks.
>>>>> >> +#
>>>>> >> +# Note:
>>>>> >> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
>>>>> >> +#   wrapper installed in the $(O).
>>>>> >> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>>>>> >>  .PHONY: _all $(MAKECMDGOALS)
>>>>> >>
>>>>> >>  $(MAKECMDGOALS): _all
>>>>> >>         @:
>>>>> >>
>>>>> >>  _all:
>>>>> >> -       @umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
>>>>> >> +       umask $(UMASK) && \
>>>>> >> +               $(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
>>>>> >> +                       $(MAKECMDGOALS) O=$(realpath $(O))
>>>>> >
>>>>> > I've applied your v7 patches with these changes to the tip of
>>>>> > buildroot and currently run into the following error.  I'm still
>>>>> > poking around trying to understand why.
>>>>> >
>>>>> > $ make V=1 qemu_x86_defconfig
>>>>> > umask 0022 && \
>>>>> >         make -C /opt/buildroot --no-print-directory \
>>>>> >                 qemu_x86_defconfig O=
>>>>
>>>> Why are you opassing an empty O= option here?
>>>>
>>>
>>> I'm not passing the O= .
>>>
>>> However, if I modify the patch and remove that from the target in the
>>> Makefile ie. remove the "O=$(realpath $(O))" , things build ok.
>>
>> I tested this tonight on another server with Make version 3.81 and
>> reproduced with a fresh buildroot checkout
>> (f34c69da2bfa2a485671dbbcf1ee24ad7409b63b) plus applying the commit
>> mentioned as the latest from Samuel's fork.
>>
>> Same behavior, if I edit the Makefile and remove the "O=$(realpath
>> $(O))" , the defconfig sets up as expected
>
> One additional detail, I cloned https://github.com/tSed/buildroot.git
> and checkout c55eeee30fe6106354c0f2cb6ce6ecb46972503a.  Then "make
> qemu_x86_defconfig" and reproduced it.  I'll start dissecting the
> other commits past that one.  Something must be masking it.
>

I think I get what happens:

According to [2,3], realpath return an empty string in case of ENOENT
(file does not exist), hence the empty $(O) because output/ is not yet
created when re-entering make.

However, I'm still not sure how to fix this, either pass O=$(O)
instead of O=$(realpath $(O)) when re-entering make, or mkdir $(O)
before re-entering make. (I still need to think/test about it).
Any suggestions more than welcome.

Note: even after downgrading make to make-3.81, I still don't trigger
this issue. I figured this out using this tricks [4]... :-/

[2] https://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html
[3] http://man7.org/linux/man-pages/man3/realpath.3.html
[4] http://code.bulix.org/epvj9d-94510?raw

Regards,

> --
> Thanks,
> Matt

-- 
Samuel

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

* [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path
  2016-03-25  6:31               ` Samuel Martin
@ 2016-03-26 12:21                 ` Arnout Vandecappelle
  0 siblings, 0 replies; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-26 12:21 UTC (permalink / raw)
  To: buildroot

On 03/25/16 07:31, Samuel Martin wrote:
> On Wed, Mar 23, 2016 at 2:36 AM, Matthew Weber <matt@thewebers.ws> wrote:
>> Yann, Samuel
>>
>> On Tue, Mar 22, 2016 at 7:54 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>> Yann, Samuel, All
>>>
>>> On Tue, Mar 22, 2016 at 6:11 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>> Yann, All
>>>>
>>>> On Tue, Mar 22, 2016 at 5:07 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>> Matthew, All,
>>>>>
>>>>> On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:
>>>>>> On Sun, Mar 20, 2016 at 8:44 PM, Matthew Weber <matt@thewebers.ws> wrote:
>>>>>>> On Wed, Mar 9, 2016 at 4:58 PM, Samuel Martin <s.martin49@gmail.com> wrote:
>>>>>>>> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
[snip]
>>>>>>>> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
[snip]
>> One additional detail, I cloned https://github.com/tSed/buildroot.git
>> and checkout c55eeee30fe6106354c0f2cb6ce6ecb46972503a.  Then "make
>> qemu_x86_defconfig" and reproduced it.  I'll start dissecting the
>> other commits past that one.  Something must be masking it.
>>
>
> I think I get what happens:

  No you don't :-)


> According to [2,3], realpath return an empty string in case of ENOENT
> (file does not exist), hence the empty $(O) because output/ is not yet
> created when re-entering make.

  But since $(O) itself is also the empty string, the condition won't even 
trigger because 022:/tmp/buildroot: is equal to 022:/tmp/buildroot:

  Matthew, however, has umask 077 I think, so for him the condition will trigger 
even when O is not set.

  The same would happen if there would be a symlink in $PWD.

> However, I'm still not sure how to fix this, either pass O=$(O)
> instead of O=$(realpath $(O))

  Wasn't the point of this patch to give O a full path? So skipping that would 
defeat the purpose...

> when re-entering make, or mkdir $(O)
> before re-entering make. (I still need to think/test about it).
> Any suggestions more than welcome.

  Perhaps the whole ifneq ("$(origin O)", "command line") section should move 
before the conditional recursion, so that we always pass the appropriate O= 
using EXTRAMAKEARGS. Of course, this means that CONFIG_DIR and NEED_WRAPPER have 
to be passed in EXTRAMAKEARGS as well. So it's a rather complicated change.


  Regards,
  Arnout


>
> Note: even after downgrading make to make-3.81, I still don't trigger
> this issue. I figured this out using this tricks [4]... :-/
>
> [2] https://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html
> [3] http://man7.org/linux/man-pages/man3/realpath.3.html
> [4] http://code.bulix.org/epvj9d-94510?raw
>
> Regards,
>
>> --
>> Thanks,
>> Matt
>


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

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

* [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers
  2016-03-09 22:58 ` [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers Samuel Martin
@ 2016-03-27 22:38   ` Arnout Vandecappelle
  0 siblings, 0 replies; 57+ messages in thread
From: Arnout Vandecappelle @ 2016-03-27 22:38 UTC (permalink / raw)
  To: buildroot

On 03/09/16 23:58, Samuel Martin wrote:
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>
> ---
> changes v6->v7:
> - {filer,is}_elf* functions moved from utils to readelf module
> - update sdk.check_host_leaks
>
> changes v5->v6:
> - new patch
> ---
>   support/scripts/check-host-leaks | 63 +++++++++++++++++++++++++++++
>   support/scripts/shell/readelf.sh | 85 ++++++++++++++++++++++++++++++++++++++++
>   support/scripts/shell/sdk.sh     | 70 +++++++++++++++++++++++++++++++++
>   support/scripts/shell/utils.sh   | 16 ++++++++
>   4 files changed, 234 insertions(+)

  This patch is a bit large for easy review, but it's hard to split it up in a 
useful way...

>   create mode 100755 support/scripts/check-host-leaks
>
> diff --git a/support/scripts/check-host-leaks b/support/scripts/check-host-leaks
> new file mode 100755
> index 0000000..42da272
> --- /dev/null
> +++ b/support/scripts/check-host-leaks
> @@ -0,0 +1,63 @@
> +#!/usr/bin/env bash
> +
> +# Copyright (C) 2016 Samuel Martin <s.martin49@gmail.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +usage() {
> +  cat <<EOF >&2
> +Usage:  ${0} TREE_DIR TOPDIR BASE_DIR HOST_DIR STAGING_DIR
> +
> +Description:
> +
> +        This script scans a tree for host paths leaks into it.
                                                    leaked

> +        Print out leaks along side to their kind.
            Prints          alongside with the kind of leak.

> +
> +Arguments:
> +
> +        TREE_DIR        Path to the root of the tree to be scaned
                                                               scanned

> +
> +        TOPDIR          Buildroot's TOPDIR
> +
> +        BASE_DIR        Buildroot's build output base location
                                        output directory

> +
> +        HOST_DIR        Buildroot's host directory
> +
> +        STAGING_DIR     Buildroot's staging directory
> +
> +EOF
> +}
> +
> +source "${0%/*}/shell/source.sh"
> +
> +READELF=readelf

  I don't think it makes much sense to put this in an environment variable. We 
don't currently do that in check-host-rpath either.

> +
> +source.load_module utils
> +source.load_module sdk
> +
> +main() {
> +    if test ${#} -ne 5 ; then
> +        usage
> +        exit 1
> +    fi
> +    local to_scan="${1}"
> +    local topdir="${2}" basedir="${3}" hostdir="${4}" stagingdir="${5}"
> +    local gnu_target_name=$( utils.guess_gnu_target_name "${stagingdir}" )
> +    local tree path
> +    sdk.check_host_leaks "${gnu_target_name}" "${to_scan}" \
> +        "${topdir}" "${basedir}" "${hostdir}"

  I don't think it makes much sense to move the entire body of this script to 
the sdk module; it's probably easier to follow if it's fully inlined here.

> +}
> +
> +main "${@}"
> diff --git a/support/scripts/shell/readelf.sh b/support/scripts/shell/readelf.sh
> index 09b680e..e07276f 100644
> --- a/support/scripts/shell/readelf.sh
> +++ b/support/scripts/shell/readelf.sh
> @@ -21,14 +21,19 @@
>   #   readelf.filter_elf
>   #   readelf.filter_elf_executable
>   #   readelf.filter_elf_shared_object
> +#   readelf.filter_elf_static_library
> +#   readelf.filter_elf_object
>   #   readelf.is_elf_executable
>   #   readelf.is_elf_shared_object
> +#   readelf.is_elf_static_library
> +#   readelf.is_elf_object
>   #   readelf.get_rpath
>   #   readelf.get_neededs
>   #   readelf.needs_rpath
>   #   readelf.has_rpath
>   #   readelf.list_sections
>   #   readelf.has_section
> +#   readelf.string_section
>   #
>   # This module is sensitive to the following environment variables:
>   #   READELF
> @@ -105,6 +110,40 @@ readelf.filter_elf_executable() {
>       readelf._filter_elf_regexp "Type:\s+EXEC\s\(Executable\sfile\)" "${@}"
>   }
>
> +# readelf.filter_elf_static_library file...
> +#
> +# Filters ELF files; if $file is an ELF file, $file is printed, else it is
> +# discarded.

  I don't think this is the correct comment...

> +# This funtion can take one or several arguments, or read them from stdin.
> +#
> +# file : path of file to be filtered
> +#
> +# environment:
> +#   READELF: readelf program path
> +readelf.filter_elf_static_library() {
> +    readelf._filter_elf_regexp "Type:\s+REL\s\(Relocatable\sfile\)" "${@}" |
> +    readelf._filter_elf_regexp "^File:\s+\S+\)$"
> +}
> +
> +# readelf.filter_elf_object file...
> +#
> +# Filters ELF files; if $file is an ELF file, $file is printed, else it is
> +# discarded.

  Same here.

> +# This funtion can take one or several arguments, or read them from stdin.
> +#
> +# file : path of file to be filtered
> +#
> +# environment:
> +#   READELF: readelf program path
> +readelf.filter_elf_object() {
> +    readelf._filter_elf_regexp "Type:\s+REL\s\(Relocatable\sfile\)" "${@}" |
> +    while read file ; do
> +        LC_ALL=C ${READELF} -h "${file}" 2>/dev/null |
> +            grep -qE "^File:\s+\S+\)$" ||
> +        printf "%s\n" "${file}"

  Indentation is confusing here.

> +   done
> +}
> +
>   # readelf.is_elf_shared_object file
>   #
>   # Returns 0 if $file is an ELF file, non-0 otherwise.
> @@ -129,6 +168,38 @@ readelf.is_elf_executable() {
>       test "$( readelf.filter_elf_executable "${1}" )" != ""
>   }
>
> +# readelf.is_elf file
> +#
> +# Returns 0 if $file is an ELF file, non-0 otherwise.
> +#
> +# file : path of file to be tested
> +#
> +# environment:
> +#   READELF: readelf program path
> +readelf.is_elf() {
> +    test "$( readelf.filter_elf "${1}" )" != ""
> +}
> +
> +# readelf.is_elf_static_library file
> +#
> +# Return 0 if $file is a Linux static libraries, i.e. an ar-archive
> +# containing *.o files.
> +#
> +# file : path of file to be tested
> +readelf.is_elf_static_library() {
> +    test "$( readelf.filter_elf_static_library "${1}" )" != ""

  Since this is the only place where filter_elf_static_library is used, I think 
it would be nicer to create a function readelf._match_elf_regexp that takes only 
a single file as argument, and returns 0 or 1 rather than printing the result. 
That would also simplify the _object function.


> +}
> +
> +# readelf.is_elf_object file
> +#
> +# Return 0 if $file is a Linux static libraries, i.e. an ar-archive
> +# containing *.o files.
> +#
> +# file : path of file to be tested
> +readelf.is_elf_object() {
> +    test "$( readelf.filter_elf_object "${1}" )" != ""
> +}
> +
>   # readelf.get_rpath file
>   #
>   # Return the unsplitted RPATH/RUNPATH of $file.
> @@ -241,3 +312,17 @@ readelf.has_section() {
>       local file="${1}" section_name="${2}"
>       readelf.list_sections "${file}" | grep -q "^${section_name}$"
>   }
> +
> +# readelf.string_section file section
> +#
> +# Return the given $section of $file.
> +#
> +# file    : ELF file path
> +# section : ELF section name
> +#
> +# environment:
> +#   READELF: readelf program path
> +readelf.string_section() {
> +    local file="${1}" section="${2}"
> +    LC_ALL=C "${READELF}" --string-dump "${section}" "${file}" 2>/dev/null
> +}
> diff --git a/support/scripts/shell/sdk.sh b/support/scripts/shell/sdk.sh
> index b2f699c..ea96ebf 100644
> --- a/support/scripts/shell/sdk.sh
> +++ b/support/scripts/shell/sdk.sh
> @@ -19,9 +19,16 @@
>   # This module defines the following functions:
>   #   sdk.compute_relative_path
>   #   sdk.compute_rpath
> +#   sdk.check_host_leaks
> +#
> +# This module is sensitive to the following environment variables:
> +#   READELF
>
>   source.declare_module sdk
>
> +source.load_module utils
> +source.load_module readelf
> +
>   # sdk.compute_relative_path basedir path start
>   #
>   # Computes and prints the relative path between $start and $path within $basedir.
> @@ -66,3 +73,66 @@ sdk.compute_rpath() {
>       done
>       sed -e 's/ /:/g' <<<"${rpath[@]}"
>   }
> +
> +# sdk.check_host_leaks gnu_target_name root pattern_basedir...

  pattern_basedir is not actually a pattern, it's just a directory. So remove 
the pattern_ bit.

> +#
> +# Scan the $root tree for the given $pattern_basedir pattern leaks.
> +# The $gnu_target_name is used to skip the sysroot location when
> +# scanning the host tree.
> +# Categorize the type of leaks.
> +# Print the leaks categories and and the matching file path.
> +#
> +# gnu_target_name : GNU target name (e.g. arm-buildroot-linux-gnueabihf)

  Since you actually match against ${target}/sysroot below, perhaps it's better 
to pass that as an argument? It makes more sense.

> +# root            : path to the root of the tree to be scanned
> +# pattern_basedir : list of patterns to be searched
> +sdk.check_host_leaks() {
> +    local target="${1}" root="${2}"
> +    local patterns=()
> +    local pattern

  Here to, not pattern but dir.

> +    shift 2
> +    for pattern in ${@} ; do
> +        patterns+=( "${pattern}" "$( readlink -f "${pattern}" )" )

  Why the readlink -f ? Doesn't patch 3/18 make sure that host, target and 
staging dirs already are a canonical absolute path?

> +    done
> +    patterns=( $( utils.list_reduce ${patterns[@]} ) )
> +
> +    local regexp="$( sed -re 's/^/(/ ; s/$/)/ ; s/ +/|/g' <<<"${patterns[*]}" )"
> +    ( cd "${root}"
> +        local f leak
> +        while read f ; do
> +            leak=
> +            if test -h "${f}" ; then leak="symlink"
> +            elif readelf.is_elf "${f}" ;then
> +                if readelf.is_elf_executable "${f}" ; then leak="ELF/exe"
> +                elif readelf.is_elf_shared_object "${f}" ; then leak="ELF/*.so"
> +                elif readelf.is_elf_static_library "${f}" ; then leak="ELF/*.a"
> +                elif readelf.is_elf_object "${f}" ; then
> +                    case "${f}" in
> +                        *.ko) leak="ELF/*.ko" ;;
> +                        *) leak="ELF/*.o" ;;
> +                    esac
> +                else leak="ELF/?"
> +                fi
> +                local section
> +                local sections=()
> +                for section in $( readelf.list_sections "${f}" ) ; do
> +                    if readelf.string_section "${f}" "${section}" |
> +                            grep -qaE "${regexp}" ; then
> +                        sections+=( "${section}" )
> +                    fi
> +                done
> +                leak="${leak} [${sections[*]}]"
> +            else
> +                case "${f}" in
> +                    *.la) leak="*.la" ;;
> +                    *.pc) leak="*.pc" ;;
> +                    *.py) leak="*.py" ;;
> +                    *.pyc) leak="*.pyc" ;;
> +                esac
> +            fi
> +            if test -z "${leak}" ; then
> +                leak="? [$( file -z "${f}" | sed -e 's/.*: //' )]"
> +            fi

  Is it really worthwhile to go through all this effort to specify the type of 
leak? I mean, there simply shouldn't be any leaks at all. In case that there 
actually is one, it's no great effort to do a readelf manually to find the cause.

> +            printf "%-70s : %-120s\n" "${leak}" "${f}"
> +        done < <( grep -raEl "${regexp}" . | sed -re "/${target}\/sysroot\//d ; s:^\.:${root}:" )

  I hate it if the input of a while read is@the end of the loop: it means you 
have to scroll down to see what is going on, then scroll up again to find what 
is done with it.

> +    ) | sort
> +}
> diff --git a/support/scripts/shell/utils.sh b/support/scripts/shell/utils.sh
> index 9e9aaab..8e0a443 100644
> --- a/support/scripts/shell/utils.sh
> +++ b/support/scripts/shell/utils.sh
> @@ -19,6 +19,7 @@
>   # This module defines the following functions:
>   #   utils.list_has
>   #   utils.list_reduce
> +#   utils.guess_gnu_target_name
>
>   source.declare_module utils
>
> @@ -58,3 +59,18 @@ utils.list_reduce() {
>
>       echo ${lout[@]}
>   }
> +
> +# utils.guess_gnu_target_name sysroot

  Why not just pass GNU_TARGET_NAME as an argument to check-host-leaks? Or even 
better, STAGING_SUBDIR.

  Regards,
  Arnout

> +#
> +# Guess the GNU target name from the given sysroot location.
> +# This assumes the sysroot is located in:
> +#   <somewhere>/<gnu_target_name>/sysroot
> +#
> +# sysroot : path to the sysroot
> +utils.guess_gnu_target_name() {
> +    local sysroot="${1}"
> +    sysroot="$( readlink -f "${sysroot}" )"
> +    local gnu_target_name="${sysroot%/sysroot*}"
> +    gnu_target_name="${gnu_target_name##*/}"
> +    printf "%s" "${gnu_target_name}"
> +}
>


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

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

end of thread, other threads:[~2016-03-27 22:38 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 22:58 [Buildroot] [PATCH v7 00/18] Relocatable SDK / build machine leaks Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 01/18] package/linux-headers: cleanup installation Samuel Martin
2016-03-09 23:09   ` Yann E. MORIN
2016-03-12  8:46     ` Samuel Martin
2016-03-22 22:10       ` Yann E. MORIN
2016-03-09 22:58 ` [Buildroot] [PATCH v7 02/18] core: use $(CURDIR) to set TOPDIR Samuel Martin
2016-03-09 23:10   ` Yann E. MORIN
2016-03-09 23:51   ` Arnout Vandecappelle
2016-03-10 20:45     ` Thomas Petazzoni
2016-03-10 22:50   ` Thomas Petazzoni
2016-03-09 22:58 ` [Buildroot] [PATCH v7 03/18] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path Samuel Martin
2016-03-09 23:15   ` Yann E. MORIN
2016-03-12  8:48     ` Samuel Martin
     [not found]   ` <CAFt09wNvtXGcfZ3PFZOgGu+bDvp6mEr44uwxa_ryjeP6DDQKBQ@mail.gmail.com>
2016-03-21  1:45     ` Matthew Weber
2016-03-21  5:21       ` Samuel Martin
2016-03-21 11:40         ` Matthew Weber
2016-03-22  3:03           ` Matthew Weber
2016-03-22  5:55             ` Samuel Martin
2016-03-22 11:10               ` Matthew Weber
2016-03-22 22:07       ` Yann E. MORIN
2016-03-22 23:11         ` Matthew Weber
2016-03-23  0:54           ` Matthew Weber
2016-03-23  1:36             ` Matthew Weber
2016-03-25  6:31               ` Samuel Martin
2016-03-26 12:21                 ` Arnout Vandecappelle
2016-03-23 17:41           ` Yann E. MORIN
2016-03-09 22:58 ` [Buildroot] [PATCH v7 04/18] core: staging symlink uses a relative path when possible Samuel Martin
2016-03-09 23:20   ` Yann E. MORIN
2016-03-10 20:47   ` Thomas Petazzoni
2016-03-12  9:11     ` Samuel Martin
2016-03-12 13:12       ` Thomas Petazzoni
2016-03-09 22:58 ` [Buildroot] [PATCH v7 05/18] core: make staging *-config scripts relocatable Samuel Martin
2016-03-09 23:24   ` Yann E. MORIN
2016-03-10  0:07     ` Arnout Vandecappelle
2016-03-12  9:22       ` Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 06/18] core: make host " Samuel Martin
2016-03-10  0:12   ` Arnout Vandecappelle
2016-03-10 12:30     ` Samuel Martin
2016-03-10 20:50   ` Thomas Petazzoni
2016-03-09 22:58 ` [Buildroot] [PATCH v7 07/18] package/pkgconf: make the pkg-config wrapper relocatable Samuel Martin
2016-03-10 20:53   ` Thomas Petazzoni
2016-03-10 21:33     ` Peter Korsgaard
2016-03-09 22:58 ` [Buildroot] [PATCH v7 08/18] support/scripts: add fix-rpath script + a bunch of helpers Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 09/18] core: add HOST_SANITIZE_RPATH_HOOK to TARGET_FINALIZE_HOOKS Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 10/18] core: add {TARGET, STAGING}_SANITIZE_RPATH_HOOK " Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 11/18] package/speex: remove no longer needed hook Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 12/18] toolchain: add post-install hooks making the toolchain relocatable Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 13/18] support/scripts: update check-host-rpath to use the shell helpers Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 14/18] support/scripts/check-host-rpath: silent find command Samuel Martin
2016-03-10  0:15   ` Arnout Vandecappelle
2016-03-09 22:58 ` [Buildroot] [PATCH v7 15/18] support/scripts/check-host-rpath: also check HOST_DIR/{bin, sbin} Samuel Martin
2016-03-10  0:16   ` Arnout Vandecappelle
2016-03-09 22:58 ` [Buildroot] [PATCH v7 16/18] support/scripts: add check-host-leaks script + all needed helpers Samuel Martin
2016-03-27 22:38   ` Arnout Vandecappelle
2016-03-09 22:58 ` [Buildroot] [PATCH v7 17/18] core: add check-leaks-in-{target, host, staging} targets Samuel Martin
2016-03-09 22:58 ` [Buildroot] [PATCH v7 18/18] docs/manual: document how to debug shell script Samuel Martin
2016-03-10  0:21   ` Arnout Vandecappelle

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