Skip to contentSkip to conversation

PATH cleanup

PATCH Superseded Samuel Martin 4 participants 15 messages Jan 30, 2014 – Feb 4, 2014

Subsystems (2)
Kbuild ext2 file system

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Samuel Martin

author
#0 lore raw

v3 cover PATH cleanup

All,

Here is the 3rd round of the PATH cleanup series.
In this round, there is no change, but rebasing on top of master and commit log rewording/enhancement.

Yours,
Samuel

Samuel Martin (6):
Makefile: introduce BR2_PATH
Makefile: add $(HOST_DIR)/sbin to BR2_PATH
*.mk: replace (TARGET|HOST)_PATH by BR2_PATH
infra: remove unused {TARGET,HOST}_PATH definition pkg-cmake.mk: add PATH in the configure command environment
Makefile: export PATH

 Makefile                 |  4 ++++
 fs/ext2/ext2.mk          |  2 +-
 package/Makefile.in      | 12 ++++--------
 package/bustle/bustle.mk |  2 +-
 package/libhid/libhid.mk |  2 +-
 package/pkg-cmake.mk     |  2 ++
 package/pkg-python.mk    |  8 ++++----
 package/sdl/sdl.mk       |  2 +-
 8 files changed, 18 insertions(+), 16 deletions(-)

--
1.8.5.3

author
#1 lore raw

PATCH v3 1/6 Makefile: introduce BR2_PATH

Since the HOST_PATH and TARGET_PATH variables almost contain the same things, let's factorize this in a single BR2_PATH.

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

---
changes v2 -> v3:
- rebase

changes v1 -> v2:
- rebase

---
 Makefile            | 3 +++
 package/Makefile.in | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)
Makefile +3 -0
index dc57cf4..0d638ce 100644
@@ -305,6 +305,9 @@ TAR_OPTIONS=$(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 # packages compiled for the host go here
 HOST_DIR:=$(call qstrip,$(BR2_HOST_DIR))
 
+# Set BR2_PATH (including host bindirs)
+BR2_PATH:="$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
+
 # locales to generate
 GENERATE_LOCALE=$(call qstrip,$(BR2_GENERATE_LOCALE))
package/Makefile.in +2 -2
index 2e433fd..a940f54 100644
@@ -145,7 +145,7 @@ TARGET_CROSS=$(HOST_DIR)/usr/bin/$(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PREFIX))
 endif
 
 # Quotes are needed for spaces et al in path components.
-TARGET_PATH="$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
+TARGET_PATH = $(BR2_PATH)
 
 # Define TARGET_xx variables for all common binutils/gcc
 TARGET_AR       = $(TARGET_CROSS)ar

@@ -200,7 +200,7 @@ HOST_CFLAGS   ?= -O2
 HOST_CFLAGS   += $(HOST_CPPFLAGS)
 HOST_CXXFLAGS += $(HOST_CFLAGS)
 HOST_LDFLAGS  += -L$(HOST_DIR)/lib -L$(HOST_DIR)/usr/lib -Wl,-rpath,$(HOST_DIR)/usr/lib
-HOST_PATH=$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(PATH)
+HOST_PATH = $(BR2_PATH)
 
 # hostcc version as an integer - E.G. 4.3.2 => 432
 HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
Signature · 1 line
1.8.5.3
#2 lore raw

PATCH v3 1/6 Makefile: introduce BR2_PATH

Dear Samuel Martin,

Quoted 2 lines · Samuel Martin

Since the HOST_PATH and TARGET_PATH variables almost contain the same things, let's factorize this in a single BR2_PATH.

I am wondering about the name BR2_PATH. Normally, BR2_<something> is used for variables coming from Kconfig options. Introducing the BR2_PATH variable, which doesn't come from a Kconfig option seems to violate this unwritten rule.

That being said, we can't call the variable just PATH, so I'm not sure which name we should choose. BR_PATH ? BUILDROOT_PATH ? PKG_PATH ?

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH v3 1/6 Makefile: introduce BR2_PATH

Quoted 7 lines

I am wondering about the name BR2_PATH. Normally, BR2_<something> is used for variables coming from Kconfig options. Introducing the BR2_PATH variable, which doesn't come from a Kconfig option seems to violate this unwritten rule.

as a side note, BR2_EXTERNAL also violates that rule...

BR2_EXTERNAL is still an unreleased feature so if this rule becomes official there is still time to rename...

#4 lore raw

PATCH v3 1/6 Makefile: introduce BR2_PATH

Dear Jeremy Rosen,

Quoted 9 lines · Jeremy Rosen
Quoted 4 lines

I am wondering about the name BR2_PATH. Normally, BR2_<something> is used for variables coming from Kconfig options. Introducing the BR2_PATH variable, which doesn't come from a Kconfig option seems to violate this unwritten rule.

as a side note, BR2_EXTERNAL also violates that rule...

BR2_EXTERNAL is still an unreleased feature so if this rule becomes official there is still time to rename...

BR2_EXTERNAL *is* a Kconfig option. See the main Config.in:

config BR2_EXTERNAL
string
option env="BR2_EXTERNAL"

The fact that we're passing it on the command line rather than defining it within menuconfig/xconfig is solely due to the fact that its value needs to be known before starting menuconfig/xconfig, because BR2_EXTERNAL is used to know which Config.in file to include.

So no, BR2_EXTERNAL doesn't violate this rule :-)

Also, BR2_EXTERNAL is really an externally visible variable, while BR2_PATH is a purely internal variable of Buildroot.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#5 lore raw

PATCH v3 1/6 Makefile: introduce BR2_PATH

In reply to Thomas Petazzoni

Hello Thomas, all,

Quoted 8 lines · Thomas Petazzoni

Dear Samuel Martin,

Quoted 2 lines · Samuel Martin

Since the HOST_PATH and TARGET_PATH variables almost contain the same things, let's factorize this in a single BR2_PATH.

I am wondering about the name BR2_PATH. Normally, BR2_<something> is used for variables coming from Kconfig options.

Yup, I also had some concerns about its name, I naively thought this would have been raised earlier... ;-)

Introducing the

Quoted 7 lines

BR2_PATH variable, which doesn't come from a Kconfig option seems to violate this unwritten rule.

That being said, we can't call the variable just PATH, so I'm not sure which name we should choose. BR_PATH ? BUILDROOT_PATH ? PKG_PATH ?

Well, it's a kind of mess (no offense)!
- BR_ prefix seems being used mostly for internal toolchain (wrapper) stuff;
- BR2_ prefix: widely used in all Config.in;
- BUILDROOT_ prefix: for things that can be set from the environment;
- PKG_ prefix: in my mind it's to close to what we can set for pkg-config - I don't really like this one.

Maybe it's time to write down the rules for the prefixes?

Anyway, I'll wait to get more inputs before repost the series.

Regards,

Signature · 4 lines
Samuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140131/a6e25b5b/attachment.html>
author
#6 lore raw

PATCH v3 2/6 Makefile: add $(HOST_DIR)/sbin to BR2_PATH

In reply to Samuel Martin

Extend BR2_PATH because few host-packages install programs in this location.

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

---
changes v2 -> v3:
- rebase

changes v1 -> v2:
- rebase

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index 0d638ce..5e8cfd8 100644
@@ -306,7 +306,7 @@ TAR_OPTIONS=$(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 HOST_DIR:=$(call qstrip,$(BR2_HOST_DIR))
 
 # Set BR2_PATH (including host bindirs)
-BR2_PATH:="$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
+BR2_PATH:="$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
 
 # locales to generate
 GENERATE_LOCALE=$(call qstrip,$(BR2_GENERATE_LOCALE))
Signature · 1 line
1.8.5.3
#7 lore raw

PATCH v3 2/6 Makefile: add $(HOST_DIR)/sbin to BR2_PATH

Dear Samuel Martin,

Quoted 2 lines · Samuel Martin

Extend BR2_PATH because few host-packages install programs in this location.

s/few/a few/

The meaning is completely different otherwise :)

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#8 lore raw

PATCH v3 3/6 *.mk: replace (TARGET|HOST)_PATH by BR2_PATH

In reply to Samuel Martin

Thanks to the 2 previous patches of the series, BR2_PATH conatins all locations in which host-packages may install programs.

This patch replaces the occurrences TARGET_PATH and HOST_PATH with BR2_PATH, everywhere these variables are used in the *.mk files.

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

---
changes v2 -> v3:
- rebase
- no one-line commit log (ThomasP)

changes v1 -> v2:
- rebase
- make substitution of new occurenrces (pkg-python.mk and sdl.mk)

---
 fs/ext2/ext2.mk          | 2 +-
 package/Makefile.in      | 8 ++++----
 package/bustle/bustle.mk | 2 +-
 package/libhid/libhid.mk | 2 +-
 package/pkg-python.mk    | 8 ++++----
 package/sdl/sdl.mk       | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)
fs/ext2/ext2.mk +1 -1
index 0af955f..7b96dda 100644
@@ -24,7 +24,7 @@ EXT2_ENV  = GEN=$(BR2_TARGET_ROOTFS_EXT2_GEN)
 EXT2_ENV += REV=$(BR2_TARGET_ROOTFS_EXT2_REV)
 
 define ROOTFS_EXT2_CMD
-	PATH=$(TARGET_PATH) $(EXT2_ENV) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
+	PATH=$(BR2_PATH) $(EXT2_ENV) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
 endef
 
 rootfs-ext2-symlink:
package/Makefile.in +4 -4
index a940f54..bf6efe1 100644
@@ -206,7 +206,7 @@ HOST_PATH = $(BR2_PATH)
 HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
 	sed -n 's/^.* \([0-9]*\)\.\([0-9]*\)\.\([0-9]*\)[ ]*.*$$/\1\2\3/p')
 
-TARGET_CONFIGURE_OPTS=PATH=$(TARGET_PATH) \
+TARGET_CONFIGURE_OPTS=PATH=$(BR2_PATH) \
 		AR="$(TARGET_AR)" \
 		AS="$(TARGET_AS)" \
 		LD="$(TARGET_LD)" \

@@ -244,10 +244,10 @@ TARGET_CONFIGURE_OPTS=PATH=$(TARGET_PATH) \
 		PERLLIB="$(HOST_DIR)/usr/lib/perl" \
 		STAGING_DIR="$(STAGING_DIR)"
 
-TARGET_MAKE_ENV=PATH=$(TARGET_PATH) \
+TARGET_MAKE_ENV=PATH=$(BR2_PATH) \
 		PERLLIB="$(HOST_DIR)/usr/lib/perl"
 
-HOST_CONFIGURE_OPTS=PATH=$(HOST_PATH) \
+HOST_CONFIGURE_OPTS=PATH=$(BR2_PATH) \
 		AR="$(HOSTAR)" \
 		AS="$(HOSTAS)" \
 		LD="$(HOSTLD)" \

@@ -270,7 +270,7 @@ HOST_CONFIGURE_OPTS=PATH=$(HOST_PATH) \
 		PERLLIB="$(HOST_DIR)/usr/lib/perl" \
 		LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib:$(LD_LIBRARY_PATH)"
 
-HOST_MAKE_ENV=PATH=$(HOST_PATH) \
+HOST_MAKE_ENV=PATH=$(BR2_PATH) \
 		LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib:$(LD_LIBRARY_PATH)" \
 		PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 		PKG_CONFIG_SYSROOT_DIR="/" \
package/bustle/bustle.mk +1 -1
index 2330c08..c488dfa 100644
@@ -11,7 +11,7 @@ BUSTLE_LICENSE_FILES = LICENSE
 BUSTLE_DEPENDENCIES = libglib2 libpcap host-pkgconf
 
 define BUSTLE_BUILD_CMDS
-	PATH=$(TARGET_PATH) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
+	PATH=$(BR2_PATH) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
 		PCAP_FLAGS='-lpcap' -C $(@D) dist/build/bustle-pcap
 endef
package/libhid/libhid.mk +1 -1
index 97aab5d..ba0ad6b 100644
@@ -11,7 +11,7 @@ LIBHID_INSTALL_STAGING = YES
 LIBHID_AUTORECONF = YES
 # configure runs libusb-config for cflags/ldflags. Ensure it picks up
 # the target version
-LIBHID_CONF_ENV = PATH=$(STAGING_DIR)/usr/bin:$(TARGET_PATH)
+LIBHID_CONF_ENV = PATH=$(STAGING_DIR)/usr/bin:$(BR2_PATH)
 LIBHID_CONF_OPT = \
 	--disable-swig \
 	--disable-werror \
package/pkg-python.mk +4 -4
index 79e6bcf..357ed8d 100644
@@ -22,7 +22,7 @@
 
 # Target distutils-based packages
 PKG_PYTHON_DISTUTILS_ENV = \
-	PATH="$(TARGET_PATH)" \
+	PATH="$(BR2_PATH)" \
 	CC="$(TARGET_CC)" \
 	CFLAGS="$(TARGET_CFLAGS)" \
 	LDFLAGS="$(TARGET_LDFLAGS)" \

@@ -40,14 +40,14 @@ PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
 
 # Host distutils-based packages
 HOST_PKG_PYTHON_DISTUTILS_ENV = \
-	PATH="$(HOST_PATH)"
+	PATH="$(BR2_PATH)"
 
 HOST_PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
 	--prefix=$(HOST_DIR)/usr
 
 # Target setuptools-based packages
 PKG_PYTHON_SETUPTOOLS_ENV = \
-	PATH="$(TARGET_PATH)" \
+	PATH="$(BR2_PATH)" \
 	PYTHONPATH="$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages" \
 	PYTHONXCPREFIX="$(STAGING_DIR)/usr/" \
 	CROSS_COMPILING=yes \

@@ -63,7 +63,7 @@ PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
 
 # Host setuptools-based packages
 HOST_PKG_PYTHON_SETUPTOOLS_ENV = \
-	PATH="$(HOST_PATH)" \
+	PATH="$(BR2_PATH)" \
 	PYTHONXCPREFIX="$(HOST_DIR)/usr/"
 
 HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
package/sdl/sdl.mk +1 -1
index ba5fbca..4789ea2 100644
@@ -14,7 +14,7 @@ SDL_INSTALL_STAGING = YES
 # we're patching configure.in, but package cannot autoreconf with our version of
 # autotools, so we have to do it manually instead of setting SD_AUTORECONF = YES
 define SDL_RUN_AUTOGEN
-	cd $(@D) && PATH=$(HOST_PATH) ./autogen.sh
+	cd $(@D) && PATH=$(BR2_PATH) ./autogen.sh
 endef
 
 SDL_POST_PATCH_HOOKS += SDL_RUN_AUTOGEN
Signature · 1 line
1.8.5.3
#9 lore raw

PATCH v3 3/6 *.mk: replace (TARGET|HOST)_PATH by BR2_PATH

Dear Samuel Martin,

Quoted 1 line · Samuel Martin

Thanks to the 2 previous patches of the series, BR2_PATH conatins

conatins -> contains

package/bustle/bustle.mk BUSTLE_LICENSE_FILES = LICENSE · line 14

Quoted 10 lines
package/bustle/bustle.mk +1 -1
index 2330c08..c488dfa 100644
@@ -11,7 +11,7 @@ BUSTLE_LICENSE_FILES = LICENSE
 BUSTLE_DEPENDENCIES = libglib2 libpcap host-pkgconf
 
 define BUSTLE_BUILD_CMDS
-	PATH=$(TARGET_PATH) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
+	PATH=$(BR2_PATH) $(MAKE) $(TARGET_CONFIGURE_OPTS) \

This should rather be changed to:

	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS)

since TARGET_MAKE_ENV contains the proper definition for PATH. This change could be done as a preparation commit.

package/bustle/bustle.mk LIBHID_INSTALL_STAGING = YES · line 14

Quoted 6 lines
package/bustle/bustle.mk +1 -1
@@ -11,7 +11,7 @@ LIBHID_INSTALL_STAGING = YES
 LIBHID_AUTORECONF = YES
 # configure runs libusb-config for cflags/ldflags. Ensure it picks up
 # the target version
-LIBHID_CONF_ENV = PATH=$(STAGING_DIR)/usr/bin:$(TARGET_PATH)
+LIBHID_CONF_ENV = PATH=$(STAGING_DIR)/usr/bin:$(BR2_PATH)

Having $(STAGING_DIR)/usr/bin in the PATH looks really wrong. Instead of doing that, we should point libhid configure script to the libusb-config script by fixing m4/md_check_libusb018b.m4.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#10 lore raw

PATCH v3 4/6 infra: remove unused {TARGET, HOST}_PATH definition

In reply to Samuel Martin

Since the variables TARGET_PATH and HOST_PATH are not used anymore, let's remove them.

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

---
changes v2 -> v3:
- rebase
- no one-line commit log (ThomasP)

changes v1 -> v2:
- rebase

---
 package/Makefile.in | 4 ----
 1 file changed, 4 deletions(-)
package/Makefile.in +0 -4
index bf6efe1..a313745 100644
@@ -144,9 +144,6 @@ else
 TARGET_CROSS=$(HOST_DIR)/usr/bin/$(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PREFIX))-
 endif
 
-# Quotes are needed for spaces et al in path components.
-TARGET_PATH = $(BR2_PATH)
-
 # Define TARGET_xx variables for all common binutils/gcc
 TARGET_AR       = $(TARGET_CROSS)ar
 TARGET_AS       = $(TARGET_CROSS)as

@@ -200,7 +197,6 @@ HOST_CFLAGS   ?= -O2
 HOST_CFLAGS   += $(HOST_CPPFLAGS)
 HOST_CXXFLAGS += $(HOST_CFLAGS)
 HOST_LDFLAGS  += -L$(HOST_DIR)/lib -L$(HOST_DIR)/usr/lib -Wl,-rpath,$(HOST_DIR)/usr/lib
-HOST_PATH = $(BR2_PATH)
 
 # hostcc version as an integer - E.G. 4.3.2 => 432
 HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
Signature · 1 line
1.8.5.3
author
#11 lore raw

PATCH v3 5/6 pkg-cmake.mk: add PATH in the configure command environment

In reply to Samuel Martin

Because BR2_PATH is not exported in the environment beforehand running cmake, it is necessary to add it on the cmake configure command.

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

---
changes v2 -> v3:
- rebase
- no one-line commit log (ThomasP)

changes v1 -> v2:
- rebase
- remove explicit PATH=... in configure commands in pkg-autotools.mk because it is already added via *_CONFIGURE_OPTS (ThomasP), and reword the commit message.

Signed-off-by Samuel Martin <[email protected]>
---
 package/pkg-cmake.mk | 2 ++
 1 file changed, 2 insertions(+)
package/pkg-cmake.mk +2 -0
index 13be557..16d9920 100644
@@ -61,6 +61,7 @@ ifeq ($(5),target)
 define $(2)_CONFIGURE_CMDS
 	(cd $$($$(PKG)_BUILDDIR) && \
 	rm -f CMakeCache.txt && \
+	PATH=$(BR2_PATH) \
 	$$($$(PKG)_CONF_ENV) $(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \
 		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
 		-DCMAKE_INSTALL_PREFIX="/usr" \

@@ -75,6 +76,7 @@ else
 define $(2)_CONFIGURE_CMDS
 	(cd $$($$(PKG)_BUILDDIR) && \
 	rm -f CMakeCache.txt && \
+	PATH=$(BR2_PATH) \
 	$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \
 		-DCMAKE_INSTALL_SO_NO_EXE=0 \
 		-DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \
Signature · 1 line
1.8.5.3
author
#12 lore raw

PATCH v3 6/6 Makefile: export PATH

In reply to Samuel Martin

This can be useful for:
- package using custom build system that are not aware that the host tree is in $(O)/host/;
- post-{build,image} scripts, in case some host-tools were specifically built to be used by these scripts.

Note that, this patch only exports the PATH variables; cleaning up all the explicit PATH declarations leading some commands could be done in a followup patch.

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

---
changes v2 -> v3:
- rebase
- extend commit log

changes v1 -> v2:
- rebase
- extend commit message

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)
Makefile +1 -0
index 5e8cfd8..fa33bc0 100644
@@ -333,6 +333,7 @@ export STAGING_DIR
 export HOST_DIR
 export BINARIES_DIR
 export BASE_DIR
+export PATH:=$(BR2_PATH)
 
 ################################################################################
 #
Signature · 1 line
1.8.5.3
#13 lore raw

PATCH v3 6/6 Makefile: export PATH

Dear Samuel Martin,

Quoted 9 lines · Samuel Martin

This can be useful for:
- package using custom build system that are not aware that the host tree is in $(O)/host/;
- post-{build,image} scripts, in case some host-tools were specifically built to be used by these scripts.

Note that, this patch only exports the PATH variables; cleaning up all the explicit PATH declarations leading some commands could be done in a followup patch.

So with this patch, passing PATH in all the package infrastructures (cmake, autotools, python) becomes unnecessary?

I must say I am not entirely sure we want to globally export PATH. Packages using custom build system should use TARGET_MAKE_ENV and HOST_MAKE_ENV, post-build and post-image script have HOST_DIR defined in their environment I believe.

But this is not a strong opinion, I'd just like to see arguments/opinions from others on this specific change (the other changes in the series, I'm happy with them on the principle, they are just a few remaining details to sort out).

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#14 lore raw

PATCH v3 6/6 Makefile: export PATH

Quoted 21 lines · Thomas Petazzoni

Dear Samuel Martin,

Quoted 9 lines · Samuel Martin

This can be useful for:
- package using custom build system that are not aware that the host tree is in $(O)/host/;
- post-{build,image} scripts, in case some host-tools were specifically built to be used by these scripts.

Note that, this patch only exports the PATH variables; cleaning up all the explicit PATH declarations leading some commands could be done in a followup patch.

So with this patch, passing PATH in all the package infrastructures (cmake, autotools, python) becomes unnecessary?

I must say I am not entirely sure we want to globally export PATH. Packages using custom build system should use TARGET_MAKE_ENV and HOST_MAKE_ENV, post-build and post-image script have HOST_DIR defined in their environment I believe.

Quoted 4 lines

But this is not a strong opinion, I'd just like to see arguments/opinions from others on this specific change (the other changes in the series, I'm happy with them on the principle, they are just a few remaining details to sort out).

I'm really fine with exporting it globally. It avoids mistakes, because
we will always use the things that we build ourselves - we can't be in a situation any more that some steps use the system tool and other steps use the host tool (well, we still can because of missing dependencies, but the risk is smaller). It also removes the need for things like setting $(TAR) explicitly (not that we should immediately remove that one, but we don't have to add it in the future whenever we add another host-or-system utility).

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F