* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
@ 2014-01-21 21:04 Yann E. MORIN
2014-01-22 9:00 ` Jeremy Rosen
0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2014-01-21 21:04 UTC (permalink / raw)
To: buildroot
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
Using a relative path for BR2_EXTERNAL, and using an external defconfig,
such as in (from a Buildroot top-dir):
make O=.. BR2_EXTERNAL=.. foo_defconfig
is broken. It is unclear why the %_defconfig rule recurses in that case,
but using relative paths is inherently broken.
This patch first makes BR2_EXTERNAL canonical (ie. makes it an absolute
path). Then we check this path exists and contains a valid BR2_EXTERNAL
tree, by checking external.mk and Config.in exist.
The manual is updated to discourage using relative paths.
Also, a note is added to the O documentation that relative paths are
discouraged as well.
Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Romain Naour <romain.naour@openwide.fr>
---
Makefile | 10 +++++++++-
docs/manual/common-usage.txt | 13 ++++++++++---
docs/manual/customize-outside-br.txt | 12 +++++++-----
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 9dfb1e0..5753f81 100644
--- a/Makefile
+++ b/Makefile
@@ -118,7 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
override BR2_EXTERNAL = support/dummy-external
$(shell rm -f $(BR2_EXTERNAL_FILE))
else
- $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+ _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
+ ifeq ($(and $(_BR2_EXTERNAL),$(wildcard $(_BR2_EXTERNAL)/external.mk),$(wildcard $(_BR2_EXTERNAL)/Config.in)),)
+ ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),)
+ $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
+ else
+ $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))
+ endif
+ endif
+ $(shell echo BR2_EXTERNAL ?= $(_BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
endif
# Need that early, before we scan packages
diff --git a/docs/manual/common-usage.txt b/docs/manual/common-usage.txt
index 1d15c05..ca44b10 100644
--- a/docs/manual/common-usage.txt
+++ b/docs/manual/common-usage.txt
@@ -40,7 +40,8 @@ Or:
$ cd /tmp/build; make O=$PWD -C path/to/buildroot
--------------------
-All the output files will be located under +/tmp/build+.
+All the output files will be located under +/tmp/build+. If the +O+
+path does not exist, Buildroot will create it.
When using out-of-tree builds, the Buildroot +.config+ and temporary
files are also stored in the output directory. This means that you can
@@ -48,13 +49,19 @@ safely run multiple builds in parallel using the same source tree as
long as they use unique output directories.
For ease of use, Buildroot generates a Makefile wrapper in the output
-directory - so after the first run, you no longer need to pass +O=..+
-and +-C ..+, simply run (in the output directory):
+directory - so after the first run, you no longer need to pass +O=<...>+
+and +-C <...>+, simply run (in the output directory):
--------------------
$ make <target>
--------------------
+The +O+ path can be either an absolute or a relative path, but if it's
+passed as a relative path, it is important to note that it is interpreted
+relative to the main Buildroot source directory, not the current working
+directory. Using relative paths can lead to various broken setups, and
+thus is highly discouraged in favour of using absolute paths.
+
[[env-vars]]
Environment variables
diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
index 19257e6..53d88d7 100644
--- a/docs/manual/customize-outside-br.txt
+++ b/docs/manual/customize-outside-br.txt
@@ -32,16 +32,18 @@ removed by passing an empty value.
The +BR2_EXTERNAL+ path can be either an absolute or a relative path,
but if it's passed as a relative path, it is important to note that it
-is interpreted relatively to the main Buildroot source directory, not
-the Buildroot output directory.
+is interpreted relative to the main Buildroot source directory, not
+the Buildroot output directory. Using relative paths can lead to various
+broken setups, and thus is highly discouraged in favour of using
+absolute paths.
Some examples:
-----
- buildroot/ $ make BR2_EXTERNAL=../foobar menuconfig
+ buildroot/ $ make BR2_EXTERNAL=/path/to/foobar menuconfig
-----
-Starting from now on, external definitions from the +../foobar+
+Starting from now on, external definitions from the +/path/to/foobar+
directory will be used:
-----
@@ -52,7 +54,7 @@ directory will be used:
We can switch to another external definitions directory at any time:
-----
- buildroot/ $ make BR2_EXTERNAL=../barfoo xconfig
+ buildroot/ $ make BR2_EXTERNAL=/where/we/have/barfoo xconfig
-----
Or disable the usage of external definitions:
--
1.8.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
2014-01-21 21:04 [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it Yann E. MORIN
@ 2014-01-22 9:00 ` Jeremy Rosen
2014-01-22 10:21 ` yann.morin.1998 at free.fr
2014-01-22 18:55 ` Yann E. MORIN
0 siblings, 2 replies; 6+ messages in thread
From: Jeremy Rosen @ 2014-01-22 9:00 UTC (permalink / raw)
To: buildroot
ok, I tried your patch and I have a couple of problems
I'm still trying my use-case (I'm only half convinced it's uesfull at
this point, but it's still supposed to work, which is a good enough
reason to try to understand what's going on)
with the tmp/ directory containing only the buildroot subdirectory
i get:
rosen at pcrosen:~/tmp/buildroot$ LANG=C make O=.. BR2_EXTERNAL=.. raspberrypi_defconfig
Makefile:126: *** BR2_EXTERNAL='..' does not exist, relatively to /home/rosen/tmp/buildroot. Stop.
Which is a confusing error message, the problem is that I don't have
a Config.in and external.mk file in tmp /(which are mandatory in
BR2_EXTERNAL though that's not stated in the doc afaics)
once that is fixed, that command line still creates the infinite
recursion on calling defconfig. So that doesn't solve that particular issue...
----- Mail original -----
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Using a relative path for BR2_EXTERNAL, and using an external
> defconfig,
> such as in (from a Buildroot top-dir):
> make O=.. BR2_EXTERNAL=.. foo_defconfig
>
> is broken. It is unclear why the %_defconfig rule recurses in that
> case,
> but using relative paths is inherently broken.
>
> This patch first makes BR2_EXTERNAL canonical (ie. makes it an
> absolute
> path). Then we check this path exists and contains a valid
> BR2_EXTERNAL
> tree, by checking external.mk and Config.in exist.
>
> The manual is updated to discourage using relative paths.
>
> Also, a note is added to the O documentation that relative paths are
> discouraged as well.
>
> Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> Cc: Romain Naour <romain.naour@openwide.fr>
> ---
> Makefile | 10 +++++++++-
> docs/manual/common-usage.txt | 13 ++++++++++---
> docs/manual/customize-outside-br.txt | 12 +++++++-----
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9dfb1e0..5753f81 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -118,7 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
> override BR2_EXTERNAL = support/dummy-external
> $(shell rm -f $(BR2_EXTERNAL_FILE))
> else
> - $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) >
> $(BR2_EXTERNAL_FILE))
> + _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> + ifeq ($(and $(_BR2_EXTERNAL),$(wildcard
> $(_BR2_EXTERNAL)/external.mk),$(wildcard
> $(_BR2_EXTERNAL)/Config.in)),)
> + ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),)
> + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> + else
> + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist,
> relatively to $(TOPDIR))
> + endif
> + endif
> + $(shell echo BR2_EXTERNAL ?= $(_BR2_EXTERNAL) >
> $(BR2_EXTERNAL_FILE))
> endif
>
> # Need that early, before we scan packages
> diff --git a/docs/manual/common-usage.txt
> b/docs/manual/common-usage.txt
> index 1d15c05..ca44b10 100644
> --- a/docs/manual/common-usage.txt
> +++ b/docs/manual/common-usage.txt
> @@ -40,7 +40,8 @@ Or:
> $ cd /tmp/build; make O=$PWD -C path/to/buildroot
> --------------------
>
> -All the output files will be located under +/tmp/build+.
> +All the output files will be located under +/tmp/build+. If the +O+
> +path does not exist, Buildroot will create it.
>
> When using out-of-tree builds, the Buildroot +.config+ and temporary
> files are also stored in the output directory. This means that you
> can
> @@ -48,13 +49,19 @@ safely run multiple builds in parallel using the
> same source tree as
> long as they use unique output directories.
>
> For ease of use, Buildroot generates a Makefile wrapper in the
> output
> -directory - so after the first run, you no longer need to pass
> +O=..+
> -and +-C ..+, simply run (in the output directory):
> +directory - so after the first run, you no longer need to pass
> +O=<...>+
> +and +-C <...>+, simply run (in the output directory):
>
> --------------------
> $ make <target>
> --------------------
>
> +The +O+ path can be either an absolute or a relative path, but if
> it's
> +passed as a relative path, it is important to note that it is
> interpreted
> +relative to the main Buildroot source directory, not the current
> working
> +directory. Using relative paths can lead to various broken setups,
> and
> +thus is highly discouraged in favour of using absolute paths.
> +
> [[env-vars]]
>
> Environment variables
> diff --git a/docs/manual/customize-outside-br.txt
> b/docs/manual/customize-outside-br.txt
> index 19257e6..53d88d7 100644
> --- a/docs/manual/customize-outside-br.txt
> +++ b/docs/manual/customize-outside-br.txt
> @@ -32,16 +32,18 @@ removed by passing an empty value.
>
> The +BR2_EXTERNAL+ path can be either an absolute or a relative
> path,
> but if it's passed as a relative path, it is important to note that
> it
> -is interpreted relatively to the main Buildroot source directory,
> not
> -the Buildroot output directory.
> +is interpreted relative to the main Buildroot source directory, not
> +the Buildroot output directory. Using relative paths can lead to
> various
> +broken setups, and thus is highly discouraged in favour of using
> +absolute paths.
>
> Some examples:
>
> -----
> - buildroot/ $ make BR2_EXTERNAL=../foobar menuconfig
> + buildroot/ $ make BR2_EXTERNAL=/path/to/foobar menuconfig
> -----
>
> -Starting from now on, external definitions from the +../foobar+
> +Starting from now on, external definitions from the
> +/path/to/foobar+
> directory will be used:
>
> -----
> @@ -52,7 +54,7 @@ directory will be used:
> We can switch to another external definitions directory at any time:
>
> -----
> - buildroot/ $ make BR2_EXTERNAL=../barfoo xconfig
> + buildroot/ $ make BR2_EXTERNAL=/where/we/have/barfoo xconfig
> -----
>
> Or disable the usage of external definitions:
> --
> 1.8.1.2
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
2014-01-22 9:00 ` Jeremy Rosen
@ 2014-01-22 10:21 ` yann.morin.1998 at free.fr
2014-01-22 12:43 ` Jeremy Rosen
2014-01-22 18:55 ` Yann E. MORIN
1 sibling, 1 reply; 6+ messages in thread
From: yann.morin.1998 at free.fr @ 2014-01-22 10:21 UTC (permalink / raw)
To: buildroot
J?r?my, All,
----- Original Message -----
From: "Jeremy Rosen" <jeremy.rosen@openwide.fr>
To: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: "Peter Korsgaard" <jacmet@uclibc.org>, "Romain Naour" <romain.naour@openwide.fr>, buildroot at busybox.net
Sent: Wednesday, January 22, 2014 10:00:40 AM
Subject: Re: [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
ok, I tried your patch and I have a couple of problems
[--SNIP--]
> diff --git a/Makefile b/Makefile
> index 9dfb1e0..5753f81 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -118,7 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
> override BR2_EXTERNAL = support/dummy-external
> $(shell rm -f $(BR2_EXTERNAL_FILE))
> else
> - $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
> + _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> + ifeq ($(and $(_BR2_EXTERNAL),$(wildcard $(_BR2_EXTERNAL)/external.mk),$(wildcard $(_BR2_EXTERNAL)/Config.in)),)
> + ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),)
> + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> + else
> + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))
> + endif
> + endif
Doh, I forgot to commit with this line:
override BR2_EXTERNAL := $(_BR2_EXTERNAL)
> + $(shell echo BR2_EXTERNAL ?= $(_BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
> endif
Care to test again, please?
Regards,
Yann E. MORIN.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
2014-01-22 10:21 ` yann.morin.1998 at free.fr
@ 2014-01-22 12:43 ` Jeremy Rosen
2014-01-22 21:10 ` Yann E. MORIN
0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Rosen @ 2014-01-22 12:43 UTC (permalink / raw)
To: buildroot
Ok, thx Yann
my remark about the error message still not being clean still stands
and the doc is not explicit about Config.in and external.mk being
required
if you don't want to add these to your patch I can post a separate
one, but for the technical part :
Tested-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
Cordialement
J?r?my Rosen
+33 (0)1 42 68 28 04
fight key loggers : write some perl using vim
Open Wide Ingenierie
23, rue Daviel
75012 Paris - France
www.openwide.fr
----- Mail original -----
> J?r?my, All,
>
> ----- Original Message -----
> From: "Jeremy Rosen" <jeremy.rosen@openwide.fr>
> To: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: "Peter Korsgaard" <jacmet@uclibc.org>, "Romain Naour"
> <romain.naour@openwide.fr>, buildroot at busybox.net
> Sent: Wednesday, January 22, 2014 10:00:40 AM
> Subject: Re: [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL
> dir before using it
>
> ok, I tried your patch and I have a couple of problems
>
> [--SNIP--]
> > diff --git a/Makefile b/Makefile
> > index 9dfb1e0..5753f81 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -118,7 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
> > override BR2_EXTERNAL = support/dummy-external
> > $(shell rm -f $(BR2_EXTERNAL_FILE))
> > else
> > - $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) >
> > $(BR2_EXTERNAL_FILE))
> > + _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 &&
> > pwd)
> > + ifeq ($(and $(_BR2_EXTERNAL),$(wildcard
> > $(_BR2_EXTERNAL)/external.mk),$(wildcard
> > $(_BR2_EXTERNAL)/Config.in)),)
> > + ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),)
> > + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> > + else
> > + $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist,
> > relatively to $(TOPDIR))
> > + endif
> > + endif
>
> Doh, I forgot to commit with this line:
> override BR2_EXTERNAL := $(_BR2_EXTERNAL)
>
> > + $(shell echo BR2_EXTERNAL ?= $(_BR2_EXTERNAL) >
> > $(BR2_EXTERNAL_FILE))
> > endif
>
> Care to test again, please?
>
> Regards,
> Yann E. MORIN.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
2014-01-22 9:00 ` Jeremy Rosen
2014-01-22 10:21 ` yann.morin.1998 at free.fr
@ 2014-01-22 18:55 ` Yann E. MORIN
1 sibling, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2014-01-22 18:55 UTC (permalink / raw)
To: buildroot
J?r?my, All,
On 2014-01-22 10:00 +0100, Jeremy Rosen spake thusly:
> ok, I tried your patch and I have a couple of problems
[--SNIP--]
> rosen at pcrosen:~/tmp/buildroot$ LANG=C make O=.. BR2_EXTERNAL=.. raspberrypi_defconfig
> Makefile:126: *** BR2_EXTERNAL='..' does not exist, relatively to /home/rosen/tmp/buildroot. Stop.
OK, that one's fixed.
> Which is a confusing error message, the problem is that I don't have
> a Config.in and external.mk file in tmp /(which are mandatory in
> BR2_EXTERNAL though that's not stated in the doc afaics)
Ah, right. They do be mandatory. I'll update the doc accordingly, so it
is explicit one has to provide both in an br2-external tree.
Thanks for the review and testing! :-)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it
2014-01-22 12:43 ` Jeremy Rosen
@ 2014-01-22 21:10 ` Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2014-01-22 21:10 UTC (permalink / raw)
To: buildroot
J?r?my, All,
On 2014-01-22 13:43 +0100, Jeremy Rosen spake thusly:
> my remark about the error message still not being clean still stands
>
> and the doc is not explicit about Config.in and external.mk being
> required
Hopefully, my new series will address those two points.
> if you don't want to add these to your patch I can post a separate
> one, but for the technical part :
>
> Tested-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
Thank you!
I haven't included your Tested-by in the new series, though, since I've
modified it more than a bit. Care to have a quick look?
Thanks for the feedback! :-)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-22 21:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-21 21:04 [Buildroot] [PATCH] BR2_EXTERNAL: check it is a valid BR2_EXTERNAL dir before using it Yann E. MORIN
2014-01-22 9:00 ` Jeremy Rosen
2014-01-22 10:21 ` yann.morin.1998 at free.fr
2014-01-22 12:43 ` Jeremy Rosen
2014-01-22 21:10 ` Yann E. MORIN
2014-01-22 18:55 ` Yann E. MORIN
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox