[Buildroot] [PATCH v3 0/6] PATH cleanup

Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [Buildroot] [PATCH v3 0/6] PATH cleanup
@ 2014-01-30 20:38 Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH Samuel Martin
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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

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

* [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-31  8:25   ` Thomas Petazzoni
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH Samuel Martin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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 <s.martin49@gmail.com>

---
changes v2 -> v3:
- rebase

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

diff --git a/Makefile b/Makefile
index dc57cf4..0d638ce 100644
--- a/Makefile
+++ b/Makefile
@@ -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))
 
diff --git a/package/Makefile.in b/package/Makefile.in
index 2e433fd..a940f54 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -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 | \
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH Samuel Martin
@ 2014-01-31  8:25   ` Thomas Petazzoni
  2014-01-31  8:41     ` Jeremy Rosen
  2014-01-31 10:13     ` Samuel Martin
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2014-01-31  8:25 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Thu, 30 Jan 2014 21:38:48 +0100, Samuel Martin wrote:
> 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH
  2014-01-31  8:25   ` Thomas Petazzoni
@ 2014-01-31  8:41     ` Jeremy Rosen
  2014-01-31  9:08       ` Thomas Petazzoni
  2014-01-31 10:13     ` Samuel Martin
  1 sibling, 1 reply; 15+ messages in thread
From: Jeremy Rosen @ 2014-01-31  8:41 UTC (permalink / raw)
  To: buildroot


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

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

* [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH
  2014-01-31  8:41     ` Jeremy Rosen
@ 2014-01-31  9:08       ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2014-01-31  9:08 UTC (permalink / raw)
  To: buildroot

Dear Jeremy Rosen,

On Fri, 31 Jan 2014 09:41:29 +0100 (CET), Jeremy Rosen wrote:

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

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

* [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH
  2014-01-31  8:25   ` Thomas Petazzoni
  2014-01-31  8:41     ` Jeremy Rosen
@ 2014-01-31 10:13     ` Samuel Martin
  1 sibling, 0 replies; 15+ messages in thread
From: Samuel Martin @ 2014-01-31 10:13 UTC (permalink / raw)
  To: buildroot

Hello Thomas, all,

On Fri, Jan 31, 2014 at 9:25 AM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Dear Samuel Martin,
>
> On Thu, 30 Jan 2014 21:38:48 +0100, Samuel Martin wrote:
> > 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
> 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,

-- 
Samuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140131/a6e25b5b/attachment.html>

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

* [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-31  8:25   ` Thomas Petazzoni
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH Samuel Martin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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

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

---
changes v2 -> v3:
- rebase

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

diff --git a/Makefile b/Makefile
index 0d638ce..5e8cfd8 100644
--- a/Makefile
+++ b/Makefile
@@ -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))
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH Samuel Martin
@ 2014-01-31  8:25   ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2014-01-31  8:25 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Thu, 30 Jan 2014 21:38:49 +0100, Samuel Martin wrote:
> Extend BR2_PATH because few host-packages install programs in this
> location.

s/few/a few/

The meaning is completely different otherwise :)

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

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

* [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-31  8:31   ` Thomas Petazzoni
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 4/6] infra: remove unused {TARGET, HOST}_PATH definition Samuel Martin
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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 <s.martin49@gmail.com>

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

diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 0af955f..7b96dda 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) $@
+	PATH=$(BR2_PATH) $(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 a940f54..bf6efe1 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -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="/" \
diff --git a/package/bustle/bustle.mk b/package/bustle/bustle.mk
index 2330c08..c488dfa 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) \
+	PATH=$(BR2_PATH) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
 		PCAP_FLAGS='-lpcap' -C $(@D) dist/build/bustle-pcap
 endef
 
diff --git a/package/libhid/libhid.mk b/package/libhid/libhid.mk
index 97aab5d..ba0ad6b 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:$(BR2_PATH)
 LIBHID_CONF_OPT = \
 	--disable-swig \
 	--disable-werror \
diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index 79e6bcf..357ed8d 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -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 = \
diff --git a/package/sdl/sdl.mk b/package/sdl/sdl.mk
index ba5fbca..4789ea2 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 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
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH Samuel Martin
@ 2014-01-31  8:31   ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2014-01-31  8:31 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Thu, 30 Jan 2014 21:38:50 +0100, Samuel Martin wrote:
> Thanks to the 2 previous patches of the series, BR2_PATH conatins

conatins -> contains

> diff --git a/package/bustle/bustle.mk b/package/bustle/bustle.mk
> index 2330c08..c488dfa 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) \
> +	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.

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

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

* [Buildroot] [PATCH v3 4/6] infra: remove unused {TARGET, HOST}_PATH definition
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
                   ` (2 preceding siblings ...)
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 5/6] pkg-cmake.mk: add PATH in the configure command environment Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 6/6] Makefile: export PATH Samuel Martin
  5 siblings, 0 replies; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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

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

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

changes v1 -> v2:
- rebase
---
 package/Makefile.in | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index bf6efe1..a313745 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
 
-# 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 | \
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 5/6] pkg-cmake.mk: add PATH in the configure command environment
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
                   ` (3 preceding siblings ...)
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 4/6] infra: remove unused {TARGET, HOST}_PATH definition Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 6/6] Makefile: export PATH Samuel Martin
  5 siblings, 0 replies; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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 <s.martin49@gmail.com>

---
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 <s.martin49@gmail.com>
---
 package/pkg-cmake.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 13be557..16d9920 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -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)" \
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 6/6] Makefile: export PATH
  2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
                   ` (4 preceding siblings ...)
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 5/6] pkg-cmake.mk: add PATH in the configure command environment Samuel Martin
@ 2014-01-30 20:38 ` Samuel Martin
  2014-01-31  8:33   ` Thomas Petazzoni
  5 siblings, 1 reply; 15+ messages in thread
From: Samuel Martin @ 2014-01-30 20:38 UTC (permalink / raw)
  To: buildroot

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 <s.martin49@gmail.com>

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

changes v1 -> v2:
- rebase
- extend commit message
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 5e8cfd8..fa33bc0 100644
--- a/Makefile
+++ b/Makefile
@@ -333,6 +333,7 @@ export STAGING_DIR
 export HOST_DIR
 export BINARIES_DIR
 export BASE_DIR
+export PATH:=$(BR2_PATH)
 
 ################################################################################
 #
-- 
1.8.5.3

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

* [Buildroot] [PATCH v3 6/6] Makefile: export PATH
  2014-01-30 20:38 ` [Buildroot] [PATCH v3 6/6] Makefile: export PATH Samuel Martin
@ 2014-01-31  8:33   ` Thomas Petazzoni
  2014-02-04  9:05     ` Arnout Vandecappelle
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2014-01-31  8:33 UTC (permalink / raw)
  To: buildroot

Dear Samuel Martin,

On Thu, 30 Jan 2014 21:38:53 +0100, Samuel Martin wrote:
> 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [Buildroot] [PATCH v3 6/6] Makefile: export PATH
  2014-01-31  8:33   ` Thomas Petazzoni
@ 2014-02-04  9:05     ` Arnout Vandecappelle
  0 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2014-02-04  9:05 UTC (permalink / raw)
  To: buildroot

On 31/01/14 09:33, Thomas Petazzoni wrote:
> Dear Samuel Martin,
> 
> On Thu, 30 Jan 2014 21:38:53 +0100, Samuel Martin wrote:
>> 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).

 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

-- 
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] 15+ messages in thread

end of thread, other threads:[~2014-02-04  9:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-30 20:38 [Buildroot] [PATCH v3 0/6] PATH cleanup Samuel Martin
2014-01-30 20:38 ` [Buildroot] [PATCH v3 1/6] Makefile: introduce BR2_PATH Samuel Martin
2014-01-31  8:25   ` Thomas Petazzoni
2014-01-31  8:41     ` Jeremy Rosen
2014-01-31  9:08       ` Thomas Petazzoni
2014-01-31 10:13     ` Samuel Martin
2014-01-30 20:38 ` [Buildroot] [PATCH v3 2/6] Makefile: add $(HOST_DIR)/sbin to BR2_PATH Samuel Martin
2014-01-31  8:25   ` Thomas Petazzoni
2014-01-30 20:38 ` [Buildroot] [PATCH v3 3/6] *.mk: replace (TARGET|HOST)_PATH by BR2_PATH Samuel Martin
2014-01-31  8:31   ` Thomas Petazzoni
2014-01-30 20:38 ` [Buildroot] [PATCH v3 4/6] infra: remove unused {TARGET, HOST}_PATH definition Samuel Martin
2014-01-30 20:38 ` [Buildroot] [PATCH v3 5/6] pkg-cmake.mk: add PATH in the configure command environment Samuel Martin
2014-01-30 20:38 ` [Buildroot] [PATCH v3 6/6] Makefile: export PATH Samuel Martin
2014-01-31  8:33   ` Thomas Petazzoni
2014-02-04  9:05     ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox