Relocatable SDK / build machine leaks

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 <[email protected]>

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

changes v5->v6:
- new patch

---
 package/linux-headers/linux-headers.mk | 1 +
 1 file changed, 1 insertion(+)

package/linux-headers/linux-headers.mk +0 -0

index 0900778..7633257 100644

package/linux-headers/linux-headers.mk +0 -0

 endef
 
 ifeq ($(BR2_KERNEL_HEADERS_VERSION)$(BR2_KERNEL_HEADERS_AS_KERNEL),y)

Signature · 6 lines

2.7.2

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

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:

Quoted 20 lines

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 <[email protected]>

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

changes v5->v6:
- new patch

---
 package/linux-headers/linux-headers.mk | 1 +
 1 file changed, 1 insertion(+)

package/linux-headers/linux-headers.mk define LINUX_HEADERS_INSTALL_STAGING_CMDS · line 126

Quoted 9 lines

package/linux-headers/linux-headers.mk +1 -0

index 0900778..7633257 100644
@@ -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

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

Quoted 21 lines

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:

Quoted 20 lines

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 <[email protected]>

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

changes v5->v6:
- new patch

---
 package/linux-headers/linux-headers.mk | 1 +
 1 file changed, 1 insertion(+)

Quoted 11 lines

package/linux-headers/linux-headers.mk define LINUX_HEADERS_INSTALL_STAGING_CMDS · line 126

Quoted 9 lines

package/linux-headers/linux-headers.mk +1 -0

index 0900778..7633257 100644
@@ -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.

Quoted 2 lines

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

Quoted 2 lines

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

Indeed.

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index f2822a2..98c8dc7 100644
@@ -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)

Signature · 6 lines

2.7.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
  Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Makefile +1 -1

index f2822a2..98c8dc7 100644
@@ -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)

Quoted 2 lines · Samuel Martin

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?

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 <[email protected]>

---
changes v6->v7:
- none

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +3 -1

index 98c8dc7..52a92f6 100644
@@ -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:

Makefile +4 -2

+		$(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)

Signature · 6 lines

2.7.2

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

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:

Quoted 35 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +3 -1

index 98c8dc7..52a92f6 100644
@@ -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:

Makefile line 41

Quoted 10 lines

Makefile +10 -0

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

Makefile line 48

Quoted 9 lines

Makefile +1 -1

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

Samuel,

Quoted 45 lines · Samuel Martin

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

 .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

Samuel, All,

Quoted 74 lines · Matthew Weber

Samuel,

Quoted 45 lines · Samuel Martin

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 4 lines

--
Thanks,
Matt

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

--
Thanks,
Matt

Quoted 5 lines · Matthew Weber

Samuel, All,

Quoted 2 lines · Matthew Weber

Samuel,

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?

Samuel,

Quoted 6 lines · Samuel Martin

Quoted 5 lines · Matthew Weber

Samuel, All,

Quoted 2 lines · Matthew Weber

Samuel,

<snip>

Quoted 1 line

Samuel,

Quoted 12 lines · Matthew Weber

Samuel,

Quoted 6 lines · Samuel Martin

Quoted 5 lines · Matthew Weber

Samuel, All,

Quoted 2 lines · Matthew Weber

Samuel,

<snip>

Quoted 1 line

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

Hi Matthew,

Quoted 16 lines · Matthew Weber

Samuel,

Quoted 12 lines · Matthew Weber

Samuel,

Quoted 6 lines · Samuel Martin

Quoted 5 lines · Matthew Weber

Samuel, All,

Quoted 2 lines · Matthew Weber

Samuel,

<snip>

Quoted 1 line

Quoted 8 lines

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

Quoted 61 lines · Samuel Martin

Quoted 45 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 4 lines

$ make V=1 qemu_x86_defconfig
umask 0022 && \
        make -C /opt/buildroot --no-print-directory \
                qemu_x86_defconfig O=

Quoted 11 lines

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.

Signature · 6 lines

Thanks,
Matt
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 62 lines · Matthew Weber

Quoted 61 lines · Samuel Martin

Quoted 45 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 4 lines

Quoted 4 lines

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

Quoted 14 lines

Quoted 11 lines

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.

Quoted 7 lines

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

'------------------------------^-------^------------------^--------------------'

Yann, All

Quoted 70 lines · Yann E. MORIN

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 66 lines · Matthew Weber

Quoted 65 lines · Samuel Martin

Quoted 45 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 2 lines

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.

Yann, Samuel, All

Quoted 73 lines · Matthew Weber

Yann, All

Quoted 70 lines · Yann E. MORIN

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 66 lines · Matthew Weber

Quoted 65 lines · Samuel Martin

Quoted 45 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 7 lines

Quoted 2 lines

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

Yann, Samuel

Quoted 89 lines · Matthew Weber

Yann, Samuel, All

Quoted 73 lines · Matthew Weber

Yann, All

Quoted 70 lines · Yann E. MORIN

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 66 lines · Matthew Weber

Quoted 65 lines · Samuel Martin

Quoted 45 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Makefile +13 -1

index 98c8dc7..52a92f6 100644
@@ -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)))

Makefile line 50

Quoted 10 lines

Makefile +3 -1

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

Quoted 7 lines

Quoted 2 lines

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.

Quoted 2 lines

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

Quoted 15 lines · Matthew Weber

Yann, Samuel

Quoted 12 lines · Matthew Weber

Yann, Samuel, All

Quoted 9 lines · Matthew Weber

Yann, All

Quoted 6 lines · Yann E. MORIN

Matthew, All,

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 3 lines · Matthew Weber

Quoted 2 lines · Samuel Martin

Quoted 1 line

When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

+ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))

Quoted 5 lines

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,

Quoted 3 lines

--
Thanks,
Matt

Quoted 7 lines

On 2016-03-20 20:45 -0500, Matthew Weber spake thusly:

Quoted 6 lines · Matthew Weber

Quoted 5 lines · Samuel Martin

Quoted 4 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

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 <[email protected]>

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

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=

Quoted 2 lines

Why are you opassing an empty O= option here?

I'm not passing the O= .

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 <[email protected]>

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

Makefile +0 -0

index 52a92f6..f1de4ed 100644

Makefile +3 -0

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

Signature · 6 lines

2.7.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/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.

Samuel,

Quoted 4 lines · Samuel Martin

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

Quoted 4 lines

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.

Quoted 3 lines

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?

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/pkg-generic.mk +0 -0

index 3904c09..ffa21ee 100644

package/pkg-generic.mk +0 -0

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

Signature · 6 lines

2.7.2

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

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:

Quoted 24 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
  package/pkg-generic.mk | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

package/pkg-generic.mk +0 -0

index 3904c09..ffa21ee 100644

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 243

Quoted 5 lines

package/pkg-generic.mk +1 -1

@@ -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" <[email protected]>

Regards,
Yann E. MORIN.

Quoted 10 lines

package/pkg-generic.mk +0 -1

  				-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

Quoted 25 lines · Yann E. MORIN

Samuel, All,

On 2016-03-09 23:58 +0100, Samuel Martin spake thusly:

Quoted 21 lines

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
  package/pkg-generic.mk | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 7 lines

Quoted 7 lines

package/pkg-generic.mk +0 -0

index 3904c09..ffa21ee 100644
@@ -240,7 +240,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
                $(call MESSAGE,"Fixing package configuration files") ;\
                        $(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \

Quoted 8 lines

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_staging_installed: · line 245

Quoted 5 lines

package/pkg-generic.mk +1 -1

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

Quoted 2 lines

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

^ missing \ here

Quoted 4 lines

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

Quoted 7 lines

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" <[email protected]>

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 <[email protected]>

---
changes v6->v7:
- none

changes v5->v6:
- new patch

---
  package/pkg-generic.mk | 12 ++++++++++++
  1 file changed, 12 insertions(+)

package/pkg-generic.mk +12 -0

index ffa21ee..845c8d3 100644
@@ -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 $@
 

Quoted 8 lines · Samuel Martin

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.

Quoted 10 lines

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.

package/pkgconf/pkg-config.in +1 -0

+PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${TARGET_PKG_CONFIG_LIBDIR}} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${STAGING_DIR}} $(dirname $0)/pkgconf @STATIC@ $@

package/pkgconf/pkgconf.mk +0 -0

index c8b0cba..d27672c 100644

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> 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.

Quoted 2 lines

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

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

Cc "Yann E. MORIN" <[email protected]>

Signed-off-by Samuel Martin <[email protected]>

---
changes v6->v7:
- new patch

---
  support/scripts/check-host-rpath | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

support/scripts/check-host-rpath +1 -1

index 0bbac30..2d3ddec 100755
@@ -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}

At least syslinux is installing stuff in HOST_DIR/sbin.

Cc "Yann E. MORIN" <[email protected]>

Signed-off-by Samuel Martin <[email protected]>

---
changes v6->v7:
- new patch

---
  support/scripts/check-host-rpath | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

support/scripts/check-host-rpath +1 -1

index 2d3ddec..f5f16ff 100755
@@ -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}

Signed-off-by Samuel Martin <[email protected]>

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

support/scripts/check-host-leaks new file +15 -0

new file mode 100755
index 0000000..42da272
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2016 Samuel Martin <[email protected]>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License

support/scripts/check-host-leaks new file +5 -0

+
+source.load_module utils
+source.load_module sdk
+
+main() {

support/scripts/check-host-leaks new file +3 -0

+}
+
+main "${@}"

support/scripts/shell/readelf.sh +5 -0

index 09b680e..e07276f 100644
@@ -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

support/scripts/shell/readelf.sh +5 -0

+# This funtion can take one or several arguments, or read them from stdin.
+#
+# file : path of file to be filtered
+#
+# environment:

support/scripts/shell/readelf.sh +13 -0

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

support/scripts/shell/readelf.sh +26 -0

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

support/scripts/shell/sdk.sh +7 -0

index b2f699c..ea96ebf 100644
@@ -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.

support/scripts/shell/sdk.sh +30 -0

+    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

support/scripts/shell/sdk.sh +2 -0

+    ) | sort
+}

support/scripts/shell/utils.sh +1 -0

index 9e9aaab..8e0a443 100644
@@ -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
 

support/scripts/shell/utils.sh +13 -0

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

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

Signed-off-by Samuel Martin <[email protected]>

---
changes v6->v7:
- new patch

---
  docs/manual/debugging-buildroot.txt | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

docs/manual/debugging-buildroot.txt +0 -0

index f575fc4..c497284 100644

docs/manual/debugging-buildroot.txt +10 -0

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