| Message ID | 1391114333-28001-2-git-send-email-s.martin49@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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
> > 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...
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
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,
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 | \
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(-)