All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v5 0/4] PATH cleanup
@ 2014-03-01 19:59 Samuel Martin
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment Samuel Martin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Samuel Martin @ 2014-03-01 19:59 UTC (permalink / raw)
  To: buildroot

Hi all,

Another round for this series, with this time, as main change: the
removing of any intermediat BR_PATH variable, and the export of PATH,
making the series even shorter. :)

Yours,
Samuel** BLURB HERE ***

Samuel Martin (4):
  bustle: use TARGET_MAKE_ENV instead of setting PATH in the make
    environment
  Makefile: export PATH including the Buildroot host bindirs
  Makefile: add $(HOST_DIR)/sbin to the PATH
  *.mk: remove all occurences of TARGET_PATH and HOST_PATH

 Makefile                 |  4 ++++
 fs/ext2/ext2.mk          |  2 +-
 package/Makefile.in      | 13 ++++---------
 package/bustle/bustle.mk |  2 +-
 package/gpm/gpm.mk       |  2 +-
 package/libhid/libhid.mk |  2 +-
 package/pkg-python.mk    |  8 ++------
 package/sdl/sdl.mk       |  2 +-
 8 files changed, 15 insertions(+), 20 deletions(-)

--
1.9.0

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

* [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment
  2014-03-01 19:59 [Buildroot] [PATCH v5 0/4] PATH cleanup Samuel Martin
@ 2014-03-01 19:59 ` Samuel Martin
  2014-03-01 22:47   ` Thomas Petazzoni
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs Samuel Martin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2014-03-01 19:59 UTC (permalink / raw)
  To: buildroot

TARGET_MAKE_ENV already contains the PATH definition among other useful
variables.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---
changes v4 -> v5:
- rebase
- add Acked-by tag

changes v3 -> v4:
- add this change to the series (ThomasP)
---
 package/bustle/bustle.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/bustle/bustle.mk b/package/bustle/bustle.mk
index 2330c08..7bc3e97 100644
--- a/package/bustle/bustle.mk
+++ b/package/bustle/bustle.mk
@@ -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) \
+	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
 		PCAP_FLAGS='-lpcap' -C $(@D) dist/build/bustle-pcap
 endef
 
-- 
1.9.0

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

* [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs
  2014-03-01 19:59 [Buildroot] [PATCH v5 0/4] PATH cleanup Samuel Martin
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment Samuel Martin
@ 2014-03-01 19:59 ` Samuel Martin
  2014-03-04 17:06   ` Arnout Vandecappelle
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH Samuel Martin
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH Samuel Martin
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2014-03-01 19:59 UTC (permalink / raw)
  To: buildroot

This patch exports the PATH variable containing the factorized content
of TARGET_PATH and HOST_PATH (because they were fairly similar).

TARGET_PATH and HOST_PATH are now set using only the PATH variable.

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

---
changes v4 -> v5:
- make my call: export PATH
  - merge patch 7/7 into this one, so:
  - do not use intermediate BR_PATH variable, just export PATH with th
    right content
  - now, HOST_PATH and TARGET_PATH are just $PATH
- remove unrelevant comments (Arnout)

changes v3 -> v4:
- rebase
- rename BR2_PATH -> BR_PATH

changes v2 -> v3:
- rebase

changes v1 -> v2:
- rebase
---
 Makefile            | 4 ++++
 package/Makefile.in | 7 ++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index ae868da..97f9983 100644
--- a/Makefile
+++ b/Makefile
@@ -356,6 +356,10 @@ export STAGING_DIR
 export HOST_DIR
 export BINARIES_DIR
 export BASE_DIR
+# * Export the PATH including the Buildroot host bindirs with immediate
+#   assignation to avoid recursive variable referencing issues triggered by make.
+# * Quotes are needed for spaces and all in the original PATH content.
+export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
 
 ################################################################################
 #
diff --git a/package/Makefile.in b/package/Makefile.in
index 454f614..0ec237d 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -144,8 +144,8 @@ 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="$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
+# PATH already contains Buildroot host bindirs
+TARGET_PATH = $(PATH)
 
 # Define TARGET_xx variables for all common binutils/gcc
 TARGET_AR       = $(TARGET_CROSS)ar
@@ -200,7 +200,8 @@ 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)
+# PATH already contains Buildroot host bindirs
+HOST_PATH = $(PATH)
 
 # hostcc version as an integer - E.G. 4.3.2 => 432
 HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
-- 
1.9.0

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

* [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH
  2014-03-01 19:59 [Buildroot] [PATCH v5 0/4] PATH cleanup Samuel Martin
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment Samuel Martin
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs Samuel Martin
@ 2014-03-01 19:59 ` Samuel Martin
  2014-03-04 17:07   ` Arnout Vandecappelle
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH Samuel Martin
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2014-03-01 19:59 UTC (permalink / raw)
  To: buildroot

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

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

---
changes v4 -> v5:
- rebase
- extend PATH instead of BR_PATH

changes v3 -> v4:
- rebase
- fix typo in commit log (ThomasP)
- rename BR2_PATH -> BR_PATH

changes v2 -> v3:
- rebase

changes v1 -> v2:
- rebase
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 97f9983..35755dc 100644
--- a/Makefile
+++ b/Makefile
@@ -359,7 +359,7 @@ export BASE_DIR
 # * Export the PATH including the Buildroot host bindirs with immediate
 #   assignation to avoid recursive variable referencing issues triggered by make.
 # * Quotes are needed for spaces and all in the original PATH content.
-export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
+export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
 
 ################################################################################
 #
-- 
1.9.0

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

* [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH
  2014-03-01 19:59 [Buildroot] [PATCH v5 0/4] PATH cleanup Samuel Martin
                   ` (2 preceding siblings ...)
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH Samuel Martin
@ 2014-03-01 19:59 ` Samuel Martin
  2014-03-04 17:19   ` Arnout Vandecappelle
  3 siblings, 1 reply; 11+ messages in thread
From: Samuel Martin @ 2014-03-01 19:59 UTC (permalink / raw)
  To: buildroot

Thanks to the 2 previous patches, the PATH now contains all locations
in which host-packages may install programs.

This patch removes the occurrences TARGET_PATH and HOST_PATH from the
*.mk files.

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

---
changes v4 -> v5:
- rebase
- remove all occurence of {TARGET,HOST}_PATH
- fixup a couple of PATH definitions

changes v3 -> v4:
- rebase
- fix typo in commit log (ThomasP)
- rename BR2_PATH -> BR_PATH
- make substitution of new occurenrces (gpm.mk)

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

changes v1 -> v2:
- make substitution of new occurenrces (pkg-python.mk and sdl.mk)
---
 fs/ext2/ext2.mk          |  2 +-
 package/Makefile.in      | 14 ++++----------
 package/gpm/gpm.mk       |  2 +-
 package/libhid/libhid.mk |  2 +-
 package/pkg-python.mk    |  8 ++------
 package/sdl/sdl.mk       |  2 +-
 6 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 0af955f..ab43483 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -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) $@
+	$(EXT2_ENV) fs/ext2/genext2fs.sh -d $(TARGET_DIR) $(EXT2_OPTS) $@
 endef
 
 rootfs-ext2-symlink:
diff --git a/package/Makefile.in b/package/Makefile.in
index 0ec237d..3e755c5 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -144,9 +144,6 @@ else
 TARGET_CROSS=$(HOST_DIR)/usr/bin/$(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_PREFIX))-
 endif
 
-# PATH already contains Buildroot host bindirs
-TARGET_PATH = $(PATH)
-
 # Define TARGET_xx variables for all common binutils/gcc
 TARGET_AR       = $(TARGET_CROSS)ar
 TARGET_AS       = $(TARGET_CROSS)as
@@ -200,8 +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
-# PATH already contains Buildroot host bindirs
-HOST_PATH = $(PATH)
 
 # hostcc version as an integer - E.G. 4.3.2 => 432
 HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
@@ -210,7 +205,7 @@ HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
 HOST_PERL_ARCHNAME := $(shell perl -MConfig -e "print Config->{archname}")
 export PERL5LIB := $(HOST_DIR)/usr/lib/perl5/$(HOST_PERL_ARCHNAME):$(HOST_DIR)/usr/lib/perl5
 
-TARGET_CONFIGURE_OPTS=PATH=$(TARGET_PATH) \
+TARGET_CONFIGURE_OPTS= \
 		AR="$(TARGET_AR)" \
 		AS="$(TARGET_AS)" \
 		LD="$(TARGET_LD)" \
@@ -247,10 +242,9 @@ TARGET_CONFIGURE_OPTS=PATH=$(TARGET_PATH) \
 		PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 		STAGING_DIR="$(STAGING_DIR)"
 
-TARGET_MAKE_ENV=PATH=$(TARGET_PATH)
-
+TARGET_MAKE_ENV=
 
-HOST_CONFIGURE_OPTS=PATH=$(HOST_PATH) \
+HOST_CONFIGURE_OPTS= \
 		AR="$(HOSTAR)" \
 		AS="$(HOSTAS)" \
 		LD="$(HOSTLD)" \
@@ -272,7 +266,7 @@ HOST_CONFIGURE_OPTS=PATH=$(HOST_PATH) \
 		PKG_CONFIG_LIBDIR="$(HOST_DIR)/usr/lib/pkgconfig:$(HOST_DIR)/usr/share/pkgconfig" \
 		LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib:$(LD_LIBRARY_PATH)"
 
-HOST_MAKE_ENV=PATH=$(HOST_PATH) \
+HOST_MAKE_ENV= \
 		LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib:$(LD_LIBRARY_PATH)" \
 		PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 		PKG_CONFIG_SYSROOT_DIR="/" \
diff --git a/package/gpm/gpm.mk b/package/gpm/gpm.mk
index 85fdad6..a47fde0 100644
--- a/package/gpm/gpm.mk
+++ b/package/gpm/gpm.mk
@@ -27,7 +27,7 @@ GPM_CONF_OPT = --without-curses
 # configure is missing but gpm seems not compatible with our autoreconf
 # mechanism so we have to do it manually instead of using GPM_AUTORECONF = YES
 define GPM_RUN_AUTOGEN
-	cd $(@D) && PATH=$(HOST_PATH) ./autogen.sh
+	cd $(@D) && ./autogen.sh
 endef
 GPM_PRE_CONFIGURE_HOOKS += GPM_RUN_AUTOGEN
 
diff --git a/package/libhid/libhid.mk b/package/libhid/libhid.mk
index 1b9b895..e6aab90 100644
--- a/package/libhid/libhid.mk
+++ b/package/libhid/libhid.mk
@@ -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:$(PATH)
 LIBHID_CONF_OPT = \
 	--disable-swig \
 	--disable-werror \
diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index 512ef66..7705a86 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -22,7 +22,6 @@
 
 # Target distutils-based packages
 PKG_PYTHON_DISTUTILS_ENV = \
-	PATH="$(TARGET_PATH)" \
 	CC="$(TARGET_CC)" \
 	CFLAGS="$(TARGET_CFLAGS)" \
 	LDFLAGS="$(TARGET_LDFLAGS)" \
@@ -39,15 +38,13 @@ PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
 	--prefix=$(TARGET_DIR)/usr
 
 # Host distutils-based packages
-HOST_PKG_PYTHON_DISTUTILS_ENV = \
-	PATH="$(HOST_PATH)"
+HOST_PKG_PYTHON_DISTUTILS_ENV =
 
 HOST_PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
 	--prefix=$(HOST_DIR)/usr
 
 # Target setuptools-based packages
 PKG_PYTHON_SETUPTOOLS_ENV = \
-	PATH="$(TARGET_PATH)" \
 	PYTHONPATH="$(if $(BR2_PACKAGE_PYTHON3),$(PYTHON3_PATH),$(PYTHON_PATH))" \
 	_python_sysroot=$(STAGING_DIR) \
 	_python_prefix=/usr \
@@ -60,8 +57,7 @@ PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
 	--root=/
 
 # Host setuptools-based packages
-HOST_PKG_PYTHON_SETUPTOOLS_ENV = \
-	PATH="$(HOST_PATH)"
+HOST_PKG_PYTHON_SETUPTOOLS_ENV =
 
 HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
 	--prefix=$(HOST_DIR)/usr
diff --git a/package/sdl/sdl.mk b/package/sdl/sdl.mk
index d5fb331..053a3c6 100644
--- a/package/sdl/sdl.mk
+++ b/package/sdl/sdl.mk
@@ -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 SDL_AUTORECONF = YES
 define SDL_RUN_AUTOGEN
-	cd $(@D) && PATH=$(HOST_PATH) ./autogen.sh
+	cd $(@D) && ./autogen.sh
 endef
 
 SDL_PRE_CONFIGURE_HOOKS += SDL_RUN_AUTOGEN
-- 
1.9.0

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

* [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment Samuel Martin
@ 2014-03-01 22:47   ` Thomas Petazzoni
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Petazzoni @ 2014-03-01 22:47 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Sat,  1 Mar 2014 20:59:08 +0100, Samuel Martin wrote:
> TARGET_MAKE_ENV already contains the PATH definition among other useful
> variables.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Applied, thanks.

For the other patches, I'll wait for some other people to do some
review.

Thanks!

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

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

* [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs Samuel Martin
@ 2014-03-04 17:06   ` Arnout Vandecappelle
  2014-03-04 21:52     ` Thomas Petazzoni
  0 siblings, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2014-03-04 17:06 UTC (permalink / raw)
  To: buildroot

On 01/03/14 20:59, Samuel Martin wrote:
> This patch exports the PATH variable containing the factorized content
> of TARGET_PATH and HOST_PATH (because they were fairly similar).
> 
> TARGET_PATH and HOST_PATH are now set using only the PATH variable.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

 Looks good to me.

 Except... I'm having second thoughts about the principle of this patch.
When debugging failing builds, I often copy&paste the build command line
if I want to drill down to the details of the error. But if PATH is
exported (and TARGET_PATH is removed, in 4/4), it is no longer visible on
the command line... So I have to remember to set it myself...

 A workaround would be to add a 'make shell' target, that drops you into
an interactive shell. I don't really like it very much, but it would
solve other issues with exported variables.

 Regards,
 Arnout


> 
> ---
> changes v4 -> v5:
> - make my call: export PATH
>   - merge patch 7/7 into this one, so:
>   - do not use intermediate BR_PATH variable, just export PATH with th
>     right content
>   - now, HOST_PATH and TARGET_PATH are just $PATH
> - remove unrelevant comments (Arnout)
> 
> changes v3 -> v4:
> - rebase
> - rename BR2_PATH -> BR_PATH
> 
> changes v2 -> v3:
> - rebase
> 
> changes v1 -> v2:
> - rebase
> ---
>  Makefile            | 4 ++++
>  package/Makefile.in | 7 ++++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index ae868da..97f9983 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -356,6 +356,10 @@ export STAGING_DIR
>  export HOST_DIR
>  export BINARIES_DIR
>  export BASE_DIR
> +# * Export the PATH including the Buildroot host bindirs with immediate
> +#   assignation to avoid recursive variable referencing issues triggered by make.
> +# * Quotes are needed for spaces and all in the original PATH content.
> +export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
>  
>  ################################################################################
>  #
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 454f614..0ec237d 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -144,8 +144,8 @@ 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="$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin/:$(PATH)"
> +# PATH already contains Buildroot host bindirs
> +TARGET_PATH = $(PATH)
>  
>  # Define TARGET_xx variables for all common binutils/gcc
>  TARGET_AR       = $(TARGET_CROSS)ar
> @@ -200,7 +200,8 @@ 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)
> +# PATH already contains Buildroot host bindirs
> +HOST_PATH = $(PATH)
>  
>  # hostcc version as an integer - E.G. 4.3.2 => 432
>  HOSTCC_VERSION:=$(shell $(HOSTCC_NOCCACHE) --version | \
> 


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

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

* [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH Samuel Martin
@ 2014-03-04 17:07   ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2014-03-04 17:07 UTC (permalink / raw)
  To: buildroot

On 01/03/14 20:59, Samuel Martin wrote:
> Extend BR_PATH because a few host-packages install programs in this

 s/BR_//

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

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

> 
> ---
> changes v4 -> v5:
> - rebase
> - extend PATH instead of BR_PATH
> 
> changes v3 -> v4:
> - rebase
> - fix typo in commit log (ThomasP)
> - rename BR2_PATH -> BR_PATH
> 
> changes v2 -> v3:
> - rebase
> 
> changes v1 -> v2:
> - rebase
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 97f9983..35755dc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -359,7 +359,7 @@ export BASE_DIR
>  # * Export the PATH including the Buildroot host bindirs with immediate
>  #   assignation to avoid recursive variable referencing issues triggered by make.
>  # * Quotes are needed for spaces and all in the original PATH content.
> -export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
> +export PATH := "$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(HOST_DIR)/usr/bin:$(HOST_DIR)/usr/sbin:$(PATH)"
>  
>  ################################################################################
>  #
> 


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

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

* [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH
  2014-03-01 19:59 ` [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH Samuel Martin
@ 2014-03-04 17:19   ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2014-03-04 17:19 UTC (permalink / raw)
  To: buildroot

On 01/03/14 20:59, Samuel Martin wrote:
> Thanks to the 2 previous patches, the PATH now contains all locations
> in which host-packages may install programs.
> 
> This patch removes the occurrences TARGET_PATH and HOST_PATH from the
> *.mk files.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v4 -> v5:
> - rebase
> - remove all occurence of {TARGET,HOST}_PATH
> - fixup a couple of PATH definitions
> 
> changes v3 -> v4:
> - rebase
> - fix typo in commit log (ThomasP)
> - rename BR2_PATH -> BR_PATH
> - make substitution of new occurenrces (gpm.mk)
> 
> changes v2 -> v3:
> - rebase
> - no one-line commit log (ThomasP)
> 
> changes v1 -> v2:
> - make substitution of new occurenrces (pkg-python.mk and sdl.mk)
> ---
>  fs/ext2/ext2.mk          |  2 +-
>  package/Makefile.in      | 14 ++++----------
>  package/gpm/gpm.mk       |  2 +-
>  package/libhid/libhid.mk |  2 +-
>  package/pkg-python.mk    |  8 ++------
>  package/sdl/sdl.mk       |  2 +-

 Have you tested that ext2, gpm, libhid and sdl still work?

>  6 files changed, 10 insertions(+), 20 deletions(-)
> 

[snip]
> diff --git a/package/pkg-python.mk b/package/pkg-python.mk
> index 512ef66..7705a86 100644
> --- a/package/pkg-python.mk
> +++ b/package/pkg-python.mk
> @@ -22,7 +22,6 @@
>  
>  # Target distutils-based packages
>  PKG_PYTHON_DISTUTILS_ENV = \
> -	PATH="$(TARGET_PATH)" \
>  	CC="$(TARGET_CC)" \
>  	CFLAGS="$(TARGET_CFLAGS)" \
>  	LDFLAGS="$(TARGET_LDFLAGS)" \
> @@ -39,15 +38,13 @@ PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
>  	--prefix=$(TARGET_DIR)/usr
>  
>  # Host distutils-based packages
> -HOST_PKG_PYTHON_DISTUTILS_ENV = \
> -	PATH="$(HOST_PATH)"
> +HOST_PKG_PYTHON_DISTUTILS_ENV =

 Since this is now empty, the variable can be removed. Also its use in
python-m2crypto can be removed, and in $(2)_BASE_ENV. And since
$(2)_BASE_ENV becomes empty, it can be removed as well.

 But perhaps you better do that in a separate patch, since some people
may disagree.


 Regards,
 Arnout


>  
>  HOST_PKG_PYTHON_DISTUTILS_INSTALL_OPT = \
>  	--prefix=$(HOST_DIR)/usr
>  
>  # Target setuptools-based packages
>  PKG_PYTHON_SETUPTOOLS_ENV = \
> -	PATH="$(TARGET_PATH)" \
>  	PYTHONPATH="$(if $(BR2_PACKAGE_PYTHON3),$(PYTHON3_PATH),$(PYTHON_PATH))" \
>  	_python_sysroot=$(STAGING_DIR) \
>  	_python_prefix=/usr \
> @@ -60,8 +57,7 @@ PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
>  	--root=/
>  
>  # Host setuptools-based packages
> -HOST_PKG_PYTHON_SETUPTOOLS_ENV = \
> -	PATH="$(HOST_PATH)"
> +HOST_PKG_PYTHON_SETUPTOOLS_ENV =
>  
>  HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPT = \
>  	--prefix=$(HOST_DIR)/usr
> diff --git a/package/sdl/sdl.mk b/package/sdl/sdl.mk
> index d5fb331..053a3c6 100644
> --- a/package/sdl/sdl.mk
> +++ b/package/sdl/sdl.mk
> @@ -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 SDL_AUTORECONF = YES
>  define SDL_RUN_AUTOGEN
> -	cd $(@D) && PATH=$(HOST_PATH) ./autogen.sh
> +	cd $(@D) && ./autogen.sh
>  endef
>  
>  SDL_PRE_CONFIGURE_HOOKS += SDL_RUN_AUTOGEN
> 


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

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

* [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs
  2014-03-04 17:06   ` Arnout Vandecappelle
@ 2014-03-04 21:52     ` Thomas Petazzoni
  2014-03-04 23:11       ` Samuel Martin
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2014-03-04 21:52 UTC (permalink / raw)
  To: buildroot

Dear Arnout Vandecappelle,

On Tue, 04 Mar 2014 18:06:41 +0100, Arnout Vandecappelle wrote:
> On 01/03/14 20:59, Samuel Martin wrote:
> > This patch exports the PATH variable containing the factorized content
> > of TARGET_PATH and HOST_PATH (because they were fairly similar).
> > 
> > TARGET_PATH and HOST_PATH are now set using only the PATH variable.
> > 
> > Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
>  Looks good to me.
> 
>  Except... I'm having second thoughts about the principle of this patch.
> When debugging failing builds, I often copy&paste the build command line
> if I want to drill down to the details of the error. But if PATH is
> exported (and TARGET_PATH is removed, in 4/4), it is no longer visible on
> the command line... So I have to remember to set it myself...

True. I also do that very often.

I personally dislike exporting the PATH globally, as it means that
we're "magically" passing some stuff into the commands we are running,
which are not explicit when looking at the commands being executed.

>  A workaround would be to add a 'make shell' target, that drops you into
> an interactive shell. I don't really like it very much, but it would
> solve other issues with exported variables.

Not a big fan either :/

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

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

* [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs
  2014-03-04 21:52     ` Thomas Petazzoni
@ 2014-03-04 23:11       ` Samuel Martin
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Martin @ 2014-03-04 23:11 UTC (permalink / raw)
  To: buildroot

Arnout, Thomas, all,

On Tue, Mar 4, 2014 at 10:52 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Arnout Vandecappelle,
>
> On Tue, 04 Mar 2014 18:06:41 +0100, Arnout Vandecappelle wrote:
>> On 01/03/14 20:59, Samuel Martin wrote:
>> > This patch exports the PATH variable containing the factorized content
>> > of TARGET_PATH and HOST_PATH (because they were fairly similar).
>> >
>> > TARGET_PATH and HOST_PATH are now set using only the PATH variable.
>> >
>> > Signed-off-by: Samuel Martin <s.martin49@gmail.com>
>>
>>  Looks good to me.
>>
>>  Except... I'm having second thoughts about the principle of this patch.
>> When debugging failing builds, I often copy&paste the build command line
>> if I want to drill down to the details of the error. But if PATH is
>> exported (and TARGET_PATH is removed, in 4/4), it is no longer visible on
>> the command line... So I have to remember to set it myself...
>
> True. I also do that very often.

... and I use that too!

>
> I personally dislike exporting the PATH globally, as it means that
> we're "magically" passing some stuff into the commands we are running,
> which are not explicit when looking at the commands being executed.

The main argument in favor of exporting the PATH is having it
correctly set when executing the post-{build,image} scripts; things
that could set when calling these scripts (the same way we set it in
the environment when build the packages).

I think I made the wrong call ;-)
So, I'll go for another round.

>
>>  A workaround would be to add a 'make shell' target, that drops you into
>> an interactive shell. I don't really like it very much, but it would
>> solve other issues with exported variables.
>
> Not a big fan either :/

Regards,

-- 
Samuel

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

end of thread, other threads:[~2014-03-04 23:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-01 19:59 [Buildroot] [PATCH v5 0/4] PATH cleanup Samuel Martin
2014-03-01 19:59 ` [Buildroot] [PATCH v5 1/4] bustle: use TARGET_MAKE_ENV instead of setting PATH in the make environment Samuel Martin
2014-03-01 22:47   ` Thomas Petazzoni
2014-03-01 19:59 ` [Buildroot] [PATCH v5 2/4] Makefile: export PATH including the Buildroot host bindirs Samuel Martin
2014-03-04 17:06   ` Arnout Vandecappelle
2014-03-04 21:52     ` Thomas Petazzoni
2014-03-04 23:11       ` Samuel Martin
2014-03-01 19:59 ` [Buildroot] [PATCH v5 3/4] Makefile: add $(HOST_DIR)/sbin to the PATH Samuel Martin
2014-03-04 17:07   ` Arnout Vandecappelle
2014-03-01 19:59 ` [Buildroot] [PATCH v5 4/4] *.mk: remove all occurences of TARGET_PATH and HOST_PATH Samuel Martin
2014-03-04 17:19   ` 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.