[Buildroot] [PATCH 0/3] BR2_EXTERNAL, O: misc fixes and docs

[Buildroot] [PATCH 0/3] BR2_EXTERNAL, O: misc fixes and docs

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Hello All!

Here is a small series that fixes use of relative paths in BR2_EXTERNAL
and documents the limitations in the manual for BR2_EXTERNAL and O.

This is a split-up of my previous patch, which can be dropped.

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (3):
      Makefile: use absolute paths to BR2_EXTERNAL
      manual: $(BR2_EXTERNAL)/{Config.in,external.mk} are mandatory
      manual: add explanations on limitation about using O=...

 Makefile                             |  9 +++++++++
 docs/manual/common-usage.txt         | 13 ++++++++++---
 docs/manual/customize-outside-br.txt | 13 ++++++++-----
 3 files changed, 27 insertions(+), 8 deletions(-)

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Yann E. MORIN]

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 makes BR2_EXTERNAL canonical (ie. makes it an absolute path),
and checks the directory exists.

The manual is updated to discourage using relative paths.

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                             |  9 +++++++++
 docs/manual/customize-outside-br.txt | 12 +++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9dfb1e0..64fa138 100644
--- a/Makefile
+++ b/Makefile
@@ -118,6 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
   override BR2_EXTERNAL = support/dummy-external
   $(shell rm -f $(BR2_EXTERNAL_FILE))
 else
+  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
+  ifeq ($(_BR2_EXTERNAL),)
+    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
+  BR2_EXTERNAL := $(_BR2_EXTERNAL)
   $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
 endif
 
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

[Buildroot] [PATCH 2/3] manual: $(BR2_EXTERNAL)/{Config.in, external.mk} are mandatory

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

The manual is a bit vague about whether Config.in and external.mk
are mandatory or optional.

Make it explicit in the manual that they are mandatory.

Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 docs/manual/customize-outside-br.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
index 53d88d7..efebc6a 100644
--- a/docs/manual/customize-outside-br.txt
+++ b/docs/manual/customize-outside-br.txt
@@ -85,6 +85,7 @@ Or disable the usage of external definitions:
    logic. Buildroot automatically includes +BR2_EXTERNAL/Config.in+ to
    make it appear in the top-level configuration menu, and includes
    +BR2_EXTERNAL/external.mk+ with the rest of the makefile logic.
+   Providing those two files is mandatory, but they can be empty.
 +
 The main usage of this is to store package recipes. The recommended
    way to do this is to write a +BR2_EXTERNAL/Config.in+ that looks
-- 
1.8.1.2

[Buildroot] [PATCH 3/3] manual: add explanations on limitation about using O=...

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Using a relative path for O=... has limitations, since it is interpreted
relative to the Buildroot tree, and thus may lead to unexpected results.

For example, running this:
    make -C buildroot O=my-O

will not vreate my-O in the current working directory, but as a
sub-directory of the Buildroot tree, here in buildroot/my-O

Explain this in the manual (as is similarly done for BR2_EXTERNAL).
Also add a note that $(O) will be created if missing.

Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 docs/manual/common-usage.txt | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

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

[Buildroot] [PATCH 3/3] manual: add explanations on limitation about using O=...

[Samuel Martin]

2014/1/22 Yann E. MORIN <yann.morin.1998@free.fr>

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Using a relative path for O=... has limitations, since it is interpreted
> relative to the Buildroot tree, and thus may lead to unexpected results.
>
> For example, running this:
>     make -C buildroot O=my-O
>
> will not vreate my-O in the current working directory, but as a
> sub-directory of the Buildroot tree, here in buildroot/my-O
>
> Explain this in the manual (as is similarly done for BR2_EXTERNAL).
> Also add a note that $(O) will be created if missing.
>
> Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
Acked-by: Samuel Martin <s.martin49@gmail.com>

Regards,

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

[Buildroot] [PATCH 2/3] manual: $(BR2_EXTERNAL)/{Config.in, external.mk} are mandatory

[Samuel Martin]

2014/1/22 Yann E. MORIN <yann.morin.1998@free.fr>

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> The manual is a bit vague about whether Config.in and external.mk
> are mandatory or optional.
>
> Make it explicit in the manual that they are mandatory.
>
> Reported-by: J?r?my Rosen <jeremy.rosen@openwide.fr>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
Acked-by: Samuel Martin <s.martin49@gmail.com>

Regards,

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Samuel Martin]

2014/1/22 Yann E. MORIN <yann.morin.1998@free.fr>

> 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 makes BR2_EXTERNAL canonical (ie. makes it an absolute path),
> and checks the directory exists.
>
> The manual is updated to discourage using relative paths.
>
> 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>
>
Acked-by: Samuel Martin <s.martin49@gmail.com>

Regards,


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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Jeremy Rosen]

Yann, all

----- 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 makes BR2_EXTERNAL canonical (ie. makes it an absolute
> path),
> and checks the directory exists.
> 
> The manual is updated to discourage using relative paths.
> 
> 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                             |  9 +++++++++
>  docs/manual/customize-outside-br.txt | 12 +++++++-----
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9dfb1e0..64fa138 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -118,6 +118,15 @@ ifeq ($(BR2_EXTERNAL),)
>    override BR2_EXTERNAL = support/dummy-external
>    $(shell rm -f $(BR2_EXTERNAL_FILE))
>  else
> +  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> +  ifeq ($(_BR2_EXTERNAL),)
> +    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
> +  BR2_EXTERNAL := $(_BR2_EXTERNAL)

you need an override keyword here.

once this is added, the patch works correctly.


>    $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) >
>    $(BR2_EXTERNAL_FILE))
>  endif
>  
> 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
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Wed, 22 Jan 2014 21:59:34 +0100, Yann E. MORIN wrote:

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

In this patch and in patch 3/3, I don't quite agree with the strong
suggestion to use absolute paths. I personally find the usage of
absolute paths ugly, and prefer to have in the manual an explanation on
how the paths are interpreted, so that the users can understand how to
properly use either relative paths or absolute paths as they prefer.

To me, a tool that strongly suggests to use absolute paths is broken.

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Arnout Vandecappelle]

On 26/01/14 14:53, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Wed, 22 Jan 2014 21:59:34 +0100, Yann E. MORIN wrote:
>
>> +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.
>
> In this patch and in patch 3/3, I don't quite agree with the strong
> suggestion to use absolute paths. I personally find the usage of
> absolute paths ugly, and prefer to have in the manual an explanation on
> how the paths are interpreted, so that the users can understand how to
> properly use either relative paths or absolute paths as they prefer.
>
> To me, a tool that strongly suggests to use absolute paths is broken.

+1

  I have never used anything except relative paths for O= (and I use O= 
all the time :-).

  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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Arnout Vandecappelle]

On 22/01/14 21:59, Yann E. MORIN wrote:
> +  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> +  ifeq ($(_BR2_EXTERNAL),)
> +    ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),/)
> +      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> +    else
> +      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))

  I don't think the limited explanation "relatively to $(TOPDIR)" 
warrants the additional complexity of patsubsting stuff.

> +    endif
> +  endif
> +  BR2_EXTERNAL := $(_BR2_EXTERNAL)

  AFAICS there is no need to have a separate _BR2_EXTERNAL, since we'll 
anyway error out if it is empty.

  So, I'd propose:

override BR2_EXTERNAL := $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
ifeq ($(BR2_EXTERNAL),)
	$(error BR2_EXTERNAL directory '$(BR2_EXTERNAL)' does not exist)
endif


  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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Yann E. MORIN]

On 2014-01-26 14:53 +0100, Thomas Petazzoni spake thusly:
> Dear Yann E. MORIN,
> 
> On Wed, 22 Jan 2014 21:59:34 +0100, Yann E. MORIN wrote:
> 
> > +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.
> 
> In this patch and in patch 3/3, I don't quite agree with the strong
> suggestion to use absolute paths. I personally find the usage of
> absolute paths ugly, and prefer to have in the manual an explanation on
> how the paths are interpreted, so that the users can understand how to
> properly use either relative paths or absolute paths as they prefer.
> 
> To me, a tool that strongly suggests to use absolute paths is broken.

I am a bit unsure how to handle this situation.

On the one hand, I agree with you that not supporting relative paths
would be bad. We should work seemlessly with relative paths.

On the other hand, the way we handle relative paths is counter-intuitive,
since they are interpreted relative to the Buildroot top-dir, not to the
current directory [*], which is baiscally the way virtually all other
programs handles relative paths, which makes it weird to a new-comer (or
even to long-timers, I have to admit).

Also, I personnally don't much care what we do or do not usggest either
way. I'm just trying to avoid the easy pitfalls.

[*] Note that this is not our fault, but make's, since that the way make
behaves. I checked the dicumentation, and there is no way to get back
the CWD make was run from.

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Yann E. MORIN]

On 2014-01-27 18:18 +0100, Arnout Vandecappelle spake thusly:
> On 26/01/14 14:53, Thomas Petazzoni wrote:
> >Dear Yann E. MORIN,
> >
> >On Wed, 22 Jan 2014 21:59:34 +0100, Yann E. MORIN wrote:
> >
> >>+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.
> >
> >In this patch and in patch 3/3, I don't quite agree with the strong
> >suggestion to use absolute paths. I personally find the usage of
> >absolute paths ugly, and prefer to have in the manual an explanation on
> >how the paths are interpreted, so that the users can understand how to
> >properly use either relative paths or absolute paths as they prefer.
> >
> >To me, a tool that strongly suggests to use absolute paths is broken.
> 
> +1
> 
>  I have never used anything except relative paths for O= (and I use O= all
> the time :-).

It does work because Buildroot create $(O). At worst, it will not end up
where you hoped it to end up in, but the build will not fail because of
a mis-configured $(O) (typo, not relative to $(TOPDIR)...)

(which, by the way, is a fundamentaly broken behaviour, IMHO.)

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Yann E. MORIN]

Arnoud, All,

On 2014-01-27 18:26 +0100, Arnout Vandecappelle spake thusly:
> On 22/01/14 21:59, Yann E. MORIN wrote:
> >+  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> >+  ifeq ($(_BR2_EXTERNAL),)
> >+    ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),/)
> >+      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> >+    else
> >+      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))
> 
>  I don't think the limited explanation "relatively to $(TOPDIR)" warrants
> the additional complexity of patsubsting stuff.

Well, we want to tell the user why w edid not find his BR2_EXTERNAL.
And since it can be tricky to understand that a relative path is not
found, we do want to highlight that fact in the error message.

> >+    endif
> >+  endif
> >+  BR2_EXTERNAL := $(_BR2_EXTERNAL)
> 
>  AFAICS there is no need to have a separate _BR2_EXTERNAL, since we'll
> anyway error out if it is empty.

I used an intermediate, since I want to display the user-supplied path
in the error message, not the one we found (or did not find).

>  So, I'd propose:
> 
> override BR2_EXTERNAL := $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> ifeq ($(BR2_EXTERNAL),)
> 	$(error BR2_EXTERNAL directory '$(BR2_EXTERNAL)' does not exist)
> endif

Uh? In this case $(BR2_EXTERNAL) would be empty in the error message, no?

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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Arnout Vandecappelle]

On 02/02/14 11:23, Yann E. MORIN wrote:
> Arnoud, All,
>
> On 2014-01-27 18:26 +0100, Arnout Vandecappelle spake thusly:
>> On 22/01/14 21:59, Yann E. MORIN wrote:
>>> +  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
>>> +  ifeq ($(_BR2_EXTERNAL),)
>>> +    ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),/)
>>> +      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
>>> +    else
>>> +      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))
>>
>>   I don't think the limited explanation "relatively to $(TOPDIR)" warrants
>> the additional complexity of patsubsting stuff.
>
> Well, we want to tell the user why w edid not find his BR2_EXTERNAL.
> And since it can be tricky to understand that a relative path is not
> found, we do want to highlight that fact in the error message.

  For me it would be sufficient to make it "relative to the buildroot 
directory" for both the absolute and relative case.

>
>>> +    endif
>>> +  endif
>>> +  BR2_EXTERNAL := $(_BR2_EXTERNAL)
>>
>>   AFAICS there is no need to have a separate _BR2_EXTERNAL, since we'll
>> anyway error out if it is empty.
>
> I used an intermediate, since I want to display the user-supplied path
> in the error message, not the one we found (or did not find).
>
>>   So, I'd propose:
>>
>> override BR2_EXTERNAL := $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
>> ifeq ($(BR2_EXTERNAL),)
>> 	$(error BR2_EXTERNAL directory '$(BR2_EXTERNAL)' does not exist)
>> endif
>
> Uh? In this case $(BR2_EXTERNAL) would be empty in the error message, no?

  Silly me, of course!

  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

[Buildroot] [PATCH 1/3] Makefile: use absolute paths to BR2_EXTERNAL

[Yann E. MORIN]

Arnoud, All,

On 2014-02-02 12:01 +0100, Arnout Vandecappelle spake thusly:
> On 02/02/14 11:23, Yann E. MORIN wrote:
> >Arnoud, All,
> >
> >On 2014-01-27 18:26 +0100, Arnout Vandecappelle spake thusly:
> >>On 22/01/14 21:59, Yann E. MORIN wrote:
> >>>+  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
> >>>+  ifeq ($(_BR2_EXTERNAL),)
> >>>+    ifeq ($(patsubst /%,/,$(BR2_EXTERNAL)),/)
> >>>+      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist)
> >>>+    else
> >>>+      $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relatively to $(TOPDIR))
> >>
> >>  I don't think the limited explanation "relatively to $(TOPDIR)" warrants
> >>the additional complexity of patsubsting stuff.
> >
> >Well, we want to tell the user why w edid not find his BR2_EXTERNAL.
> >And since it can be tricky to understand that a relative path is not
> >found, we do want to highlight that fact in the error message.
> 
>  For me it would be sufficient to make it "relative to the buildroot
> directory" for both the absolute and relative case.

OK, I get it. OK for me.

Thanks!

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