Patchwork [v3,1/6] Makefile: introduce BR2_PATH

login
register
mail settings
Submitter Samuel Martin
Date Jan. 30, 2014, 8:38 p.m.
Message ID <1391114333-28001-2-git-send-email-s.martin49@gmail.com>
Download mbox | patch
Permalink /patch/315446/
State Superseded
Headers show

Comments

Samuel Martin - Jan. 30, 2014, 8:38 p.m.
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(-)
Thomas Petazzoni - Jan. 31, 2014, 8:25 a.m.
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
Jeremy Rosen - Jan. 31, 2014, 8:41 a.m.
> 
> 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...
Thomas Petazzoni - Jan. 31, 2014, 9:08 a.m.
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
Samuel Martin - Jan. 31, 2014, 10:13 a.m.
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,

Patch

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