All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
@ 2016-10-19 21:11 Thomas Petazzoni
  2016-11-02 22:18 ` Yann E. MORIN
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2016-10-19 21:11 UTC (permalink / raw)
  To: buildroot

commit: https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master

When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
resolved differently, depending on each package build-system (whether it
uses the given paths or get the absolute canonical ones).

Using absolute canonical paths will help achieving reproducible builds and
will make easier tracking down host machine paths leaking into the host,
target or staging trees.
So, this change ensures the build takes place with the CURDIR and O
variables are set to their absolute canonical paths.

In order to recall the toplevel makefile with absolute canonical paths
for $(CURDIR) and $(O), we need to:
1- Compute the absolute canonical paths for $(CURDIR) and $(O) that will
   be passed to the sub-make. This is achieved using the 'realpath' make
   primitive. However, some care must be taken when manipulating O:
   - the out-of-tree makefile wrapper happens a trailing "/.", we need
     to strip this part away to not break the comparison driving the
     sub-make call;
   - the user can leave a trailing '/' to $(O);
   - according to [1,2], realpath returns an empty string in case of
     non-existing entry. So, to avoid passing an empty O= variable to
     sub-make, it is necessary to define the output directory and create
     it prior to call realpath on it (because on the first invocation,
     $(O) usually does not yet exists), hence the trick doing the mkdir
     right before calling realpath.
2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
   when call recalling the top-level makefile with umask and paths
   correctly set.
3- Lastly, update the condition for setting the CONFIG_DIR and
   NEED_WRAPPER variables.

Note:
* This change takes care of the makefile wrapper installed in $(O) to
  avoid unneeded make recursion.

[1] https://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html
[2] http://man7.org/linux/man-pages/man3/realpath.3.html

Reported-by: Matthew Weber <matt@thewebers.ws>
Cc: Matthew Weber <matt@thewebers.ws>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 717b2db..3ba1490 100644
--- a/Makefile
+++ b/Makefile
@@ -33,7 +33,7 @@ SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
 # or avoid confusing packages that can use the O=<dir> syntax for out-of-tree
 # build by preventing it from being forwarded to sub-make calls.
 ifneq ("$(origin O)", "command line")
-O := output
+O := $(CURDIR)/output
 else
 # Other packages might also support Linux-style out of tree builds
 # with the O=<dir> syntax (E.G. BusyBox does). As make automatically
@@ -50,27 +50,46 @@ endif
 
 # Check if the current Buildroot execution meets all the pre-requisites.
 # If they are not met, Buildroot will actually do its job in a sub-make meeting
-# its pre-requisites, which is:
+# its pre-requisites, which are:
 #  1- Permissive enough umask:
 #       Wrong or too restrictive umask will prevent Buildroot and packages from
 #       creating files and directories.
+#  2- Absolute canonical CWD (i.e. $(CURDIR)):
+#       Otherwise, some packages will use CWD as-is, others will compute its
+#       absolute canonical path. This makes harder tracking and fixing host
+#       machine path leaks.
+#  3- Absolute canonical output location (i.e. $(O)):
+#       For the same reason as the one for CWD.
+
+# Remove the trailing '/.' from $(O) as it can be added by the makefile wrapper
+# installed in the $(O) directory.
+# Also remove the trailing '/' the user can set when on the command line.
+override O := $(patsubst %/,%,$(patsubst %.,%,$(O)))
+# Make sure $(O) actually exists before calling realpath on it; this is to
+# avoid empty CANONICAL_O in case on non-existing entry.
+CANONICAL_O := $(shell mkdir -p $(O) >/dev/null 2>&1)$(realpath $(O))
+
+CANONICAL_CURDIR = $(realpath $(CURDIR))
 
 REQ_UMASK = 0022
 
-# we need to pass O= everywhere we call back into the toplevel makefile
-EXTRAMAKEARGS = O=$(O)
+# Make sure O= is passed (with its absolute canonical path) everywhere the
+# toplevel makefile is called back.
+EXTRAMAKEARGS := O=$(CANONICAL_O)
 
 # Check Buildroot execution pre-requisites here.
-ifneq ($(shell umask),$(REQ_UMASK))
+ifneq ($(shell umask):$(CURDIR):$(O),$(REQ_UMASK):$(CANONICAL_CURDIR):$(CANONICAL_O))
 .PHONY: _all $(MAKECMDGOALS)
 
 $(MAKECMDGOALS): _all
 	@:
 
 _all:
-	@umask $(REQ_UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
+	@umask $(REQ_UMASK) && \
+		$(MAKE) -C $(CANONICAL_CURDIR) --no-print-directory \
+			$(MAKECMDGOALS) $(EXTRAMAKEARGS)
 
-else # umask
+else # umask / $(CURDIR) / $(O)
 
 # This is our default rule, so must come first
 all:
@@ -140,8 +159,9 @@ endif
 include support/misc/utils.mk
 
 # Set variables related to in-tree or out-of-tree build.
-ifeq ($(O),output)
-CONFIG_DIR := $(TOPDIR)
+# Here, both $(O) and $(CURDIR) are absolute canonical paths.
+ifeq ($(O),$(CURDIR)/output)
+CONFIG_DIR := $(CURDIR)
 NEED_WRAPPER =
 else
 CONFIG_DIR := $(O)
@@ -1038,4 +1058,4 @@ include docs/manual/manual.mk
 
 .PHONY: $(noconfig_targets)
 
-endif #umask
+endif #umask / $(CURDIR) / $(O)

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-10-19 21:11 [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths Thomas Petazzoni
@ 2016-11-02 22:18 ` Yann E. MORIN
  2016-11-02 23:27   ` Samuel Martin
  0 siblings, 1 reply; 7+ messages in thread
From: Yann E. MORIN @ 2016-11-02 22:18 UTC (permalink / raw)
  To: buildroot

Samuel, All,

On 2016-10-19 23:11 +0200, Thomas Petazzoni spake thusly:
> commit: https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143
> branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
> 
> When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
> resolved differently, depending on each package build-system (whether it
> uses the given paths or get the absolute canonical ones).
> 
> Using absolute canonical paths will help achieving reproducible builds and
> will make easier tracking down host machine paths leaking into the host,
> target or staging trees.
> So, this change ensures the build takes place with the CURDIR and O
> variables are set to their absolute canonical paths.
> 
> In order to recall the toplevel makefile with absolute canonical paths
> for $(CURDIR) and $(O), we need to:
> 1- Compute the absolute canonical paths for $(CURDIR) and $(O) that will
>    be passed to the sub-make. This is achieved using the 'realpath' make
>    primitive. However, some care must be taken when manipulating O:
>    - the out-of-tree makefile wrapper happens a trailing "/.", we need
>      to strip this part away to not break the comparison driving the
>      sub-make call;
>    - the user can leave a trailing '/' to $(O);
>    - according to [1,2], realpath returns an empty string in case of
>      non-existing entry. So, to avoid passing an empty O= variable to
>      sub-make, it is necessary to define the output directory and create
>      it prior to call realpath on it (because on the first invocation,
>      $(O) usually does not yet exists), hence the trick doing the mkdir
>      right before calling realpath.
> 2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
>    when call recalling the top-level makefile with umask and paths
>    correctly set.
> 3- Lastly, update the condition for setting the CONFIG_DIR and
>    NEED_WRAPPER variables.
> 
> Note:
> * This change takes care of the makefile wrapper installed in $(O) to
>   avoid unneeded make recursion.

This commit breaks a few use-cases that were previously working.

for example, Maxime reports that:

    make BR2_PRIMARY_SITE=blabla source

no longer works.

Also, the autobuilders now no longer respect their 'njobs' configuration
option, for the same reason; they (basivally) do:

    make BR2_JLEVEL=kwargs["--njobs"]

but this is nol onger working either.

Care to have a look at this, please?

Regards,
Yann E. MORIN.

> [1] https://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html
> [2] http://man7.org/linux/man-pages/man3/realpath.3.html
> 
> Reported-by: Matthew Weber <matt@thewebers.ws>
> Cc: Matthew Weber <matt@thewebers.ws>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Tested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Makefile | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 717b2db..3ba1490 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -33,7 +33,7 @@ SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
>  # or avoid confusing packages that can use the O=<dir> syntax for out-of-tree
>  # build by preventing it from being forwarded to sub-make calls.
>  ifneq ("$(origin O)", "command line")
> -O := output
> +O := $(CURDIR)/output
>  else
>  # Other packages might also support Linux-style out of tree builds
>  # with the O=<dir> syntax (E.G. BusyBox does). As make automatically
> @@ -50,27 +50,46 @@ endif
>  
>  # Check if the current Buildroot execution meets all the pre-requisites.
>  # If they are not met, Buildroot will actually do its job in a sub-make meeting
> -# its pre-requisites, which is:
> +# its pre-requisites, which are:
>  #  1- Permissive enough umask:
>  #       Wrong or too restrictive umask will prevent Buildroot and packages from
>  #       creating files and directories.
> +#  2- Absolute canonical CWD (i.e. $(CURDIR)):
> +#       Otherwise, some packages will use CWD as-is, others will compute its
> +#       absolute canonical path. This makes harder tracking and fixing host
> +#       machine path leaks.
> +#  3- Absolute canonical output location (i.e. $(O)):
> +#       For the same reason as the one for CWD.
> +
> +# Remove the trailing '/.' from $(O) as it can be added by the makefile wrapper
> +# installed in the $(O) directory.
> +# Also remove the trailing '/' the user can set when on the command line.
> +override O := $(patsubst %/,%,$(patsubst %.,%,$(O)))
> +# Make sure $(O) actually exists before calling realpath on it; this is to
> +# avoid empty CANONICAL_O in case on non-existing entry.
> +CANONICAL_O := $(shell mkdir -p $(O) >/dev/null 2>&1)$(realpath $(O))
> +
> +CANONICAL_CURDIR = $(realpath $(CURDIR))
>  
>  REQ_UMASK = 0022
>  
> -# we need to pass O= everywhere we call back into the toplevel makefile
> -EXTRAMAKEARGS = O=$(O)
> +# Make sure O= is passed (with its absolute canonical path) everywhere the
> +# toplevel makefile is called back.
> +EXTRAMAKEARGS := O=$(CANONICAL_O)
>  
>  # Check Buildroot execution pre-requisites here.
> -ifneq ($(shell umask),$(REQ_UMASK))
> +ifneq ($(shell umask):$(CURDIR):$(O),$(REQ_UMASK):$(CANONICAL_CURDIR):$(CANONICAL_O))
>  .PHONY: _all $(MAKECMDGOALS)
>  
>  $(MAKECMDGOALS): _all
>  	@:
>  
>  _all:
> -	@umask $(REQ_UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
> +	@umask $(REQ_UMASK) && \
> +		$(MAKE) -C $(CANONICAL_CURDIR) --no-print-directory \
> +			$(MAKECMDGOALS) $(EXTRAMAKEARGS)
>  
> -else # umask
> +else # umask / $(CURDIR) / $(O)
>  
>  # This is our default rule, so must come first
>  all:
> @@ -140,8 +159,9 @@ endif
>  include support/misc/utils.mk
>  
>  # Set variables related to in-tree or out-of-tree build.
> -ifeq ($(O),output)
> -CONFIG_DIR := $(TOPDIR)
> +# Here, both $(O) and $(CURDIR) are absolute canonical paths.
> +ifeq ($(O),$(CURDIR)/output)
> +CONFIG_DIR := $(CURDIR)
>  NEED_WRAPPER =
>  else
>  CONFIG_DIR := $(O)
> @@ -1038,4 +1058,4 @@ include docs/manual/manual.mk
>  
>  .PHONY: $(noconfig_targets)
>  
> -endif #umask
> +endif #umask / $(CURDIR) / $(O)
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-11-02 22:18 ` Yann E. MORIN
@ 2016-11-02 23:27   ` Samuel Martin
  2016-11-03  1:58     ` Arnout Vandecappelle
  0 siblings, 1 reply; 7+ messages in thread
From: Samuel Martin @ 2016-11-02 23:27 UTC (permalink / raw)
  To: buildroot

Yann, Arnout, all,

On Wed, Nov 2, 2016 at 11:18 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Samuel, All,
>
> On 2016-10-19 23:11 +0200, Thomas Petazzoni spake thusly:
>> commit: https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143
>> branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
>>
>> When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
>> resolved differently, depending on each package build-system (whether it
>> uses the given paths or get the absolute canonical ones).
>>
>> Using absolute canonical paths will help achieving reproducible builds and
>> will make easier tracking down host machine paths leaking into the host,
>> target or staging trees.
>> So, this change ensures the build takes place with the CURDIR and O
>> variables are set to their absolute canonical paths.
>>
>> In order to recall the toplevel makefile with absolute canonical paths
>> for $(CURDIR) and $(O), we need to:
>> 1- Compute the absolute canonical paths for $(CURDIR) and $(O) that will
>>    be passed to the sub-make. This is achieved using the 'realpath' make
>>    primitive. However, some care must be taken when manipulating O:
>>    - the out-of-tree makefile wrapper happens a trailing "/.", we need
>>      to strip this part away to not break the comparison driving the
>>      sub-make call;
>>    - the user can leave a trailing '/' to $(O);
>>    - according to [1,2], realpath returns an empty string in case of
>>      non-existing entry. So, to avoid passing an empty O= variable to
>>      sub-make, it is necessary to define the output directory and create
>>      it prior to call realpath on it (because on the first invocation,
>>      $(O) usually does not yet exists), hence the trick doing the mkdir
>>      right before calling realpath.
>> 2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
>>    when call recalling the top-level makefile with umask and paths
>>    correctly set.
>> 3- Lastly, update the condition for setting the CONFIG_DIR and
>>    NEED_WRAPPER variables.
>>
>> Note:
>> * This change takes care of the makefile wrapper installed in $(O) to
>>   avoid unneeded make recursion.
>
> This commit breaks a few use-cases that were previously working.
>
> for example, Maxime reports that:
>
>     make BR2_PRIMARY_SITE=blabla source
>
> no longer works.
>
> Also, the autobuilders now no longer respect their 'njobs' configuration
> option, for the same reason; they (basivally) do:
>
>     make BR2_JLEVEL=kwargs["--njobs"]
>
> but this is nol onger working either.
>
> Care to have a look at this, please?

Could you explain in details a test case you got (what you look at
exactly to get this, or maybe you just git-bisect-ed?)?

I think it may be cause by this patch [1] not being apply.
Would you mind give it a try and report back?
I gave it a quick try using this diff [2], and running 'make
foo-reconfigure' w/ and w/o various variables on the command line.

Arnout, do you have an idea about what happens here?... this is a bit
beyond my make-fu :-/

Regards,

[1] http://patchwork.ozlabs.org/patch/682684/
[2] http://code.bulix.org/8k4x9g-107063/?raw


-- 
Samuel

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-11-02 23:27   ` Samuel Martin
@ 2016-11-03  1:58     ` Arnout Vandecappelle
  2016-11-03 15:05       ` Maxime Hadjinlian
  0 siblings, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2016-11-03  1:58 UTC (permalink / raw)
  To: buildroot



On 03-11-16 00:27, Samuel Martin wrote:
> Yann, Arnout, all,
> 
> On Wed, Nov 2, 2016 at 11:18 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> Samuel, All,
>>
>> On 2016-10-19 23:11 +0200, Thomas Petazzoni spake thusly:
>>> commit: https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143
>>> branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
>>>
>>> When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
>>> resolved differently, depending on each package build-system (whether it
>>> uses the given paths or get the absolute canonical ones).
>>>
>>> Using absolute canonical paths will help achieving reproducible builds and
>>> will make easier tracking down host machine paths leaking into the host,
>>> target or staging trees.
>>> So, this change ensures the build takes place with the CURDIR and O
>>> variables are set to their absolute canonical paths.
>>>
>>> In order to recall the toplevel makefile with absolute canonical paths
>>> for $(CURDIR) and $(O), we need to:
>>> 1- Compute the absolute canonical paths for $(CURDIR) and $(O) that will
>>>    be passed to the sub-make. This is achieved using the 'realpath' make
>>>    primitive. However, some care must be taken when manipulating O:
>>>    - the out-of-tree makefile wrapper happens a trailing "/.", we need
>>>      to strip this part away to not break the comparison driving the
>>>      sub-make call;
>>>    - the user can leave a trailing '/' to $(O);
>>>    - according to [1,2], realpath returns an empty string in case of
>>>      non-existing entry. So, to avoid passing an empty O= variable to
>>>      sub-make, it is necessary to define the output directory and create
>>>      it prior to call realpath on it (because on the first invocation,
>>>      $(O) usually does not yet exists), hence the trick doing the mkdir
>>>      right before calling realpath.
>>> 2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
>>>    when call recalling the top-level makefile with umask and paths
>>>    correctly set.
>>> 3- Lastly, update the condition for setting the CONFIG_DIR and
>>>    NEED_WRAPPER variables.
>>>
>>> Note:
>>> * This change takes care of the makefile wrapper installed in $(O) to
>>>   avoid unneeded make recursion.
>>
>> This commit breaks a few use-cases that were previously working.
>>
>> for example, Maxime reports that:
>>
>>     make BR2_PRIMARY_SITE=blabla source
>>
>> no longer works.
>>
>> Also, the autobuilders now no longer respect their 'njobs' configuration
>> option, for the same reason; they (basivally) do:
>>
>>     make BR2_JLEVEL=kwargs["--njobs"]
>>
>> but this is nol onger working either.
>>
>> Care to have a look at this, please?
> 
> Could you explain in details a test case you got (what you look at
> exactly to get this, or maybe you just git-bisect-ed?)?
> 
> I think it may be cause by this patch [1] not being apply.
> Would you mind give it a try and report back?
> I gave it a quick try using this diff [2], and running 'make
> foo-reconfigure' w/ and w/o various variables on the command line.
> 
> Arnout, do you have an idea about what happens here?... this is a bit
> beyond my make-fu :-/

 Yay make-fu!

http://patchwork.ozlabs.org/patch/690649/


 I shouldn't have dismissed your patch [1] so easily...

 Regards,
 Arnout

> 
> Regards,
> 
> [1] http://patchwork.ozlabs.org/patch/682684/
> [2] http://code.bulix.org/8k4x9g-107063/?raw
> 
> 

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-11-03  1:58     ` Arnout Vandecappelle
@ 2016-11-03 15:05       ` Maxime Hadjinlian
  2016-11-03 15:19         ` Arnout Vandecappelle
  0 siblings, 1 reply; 7+ messages in thread
From: Maxime Hadjinlian @ 2016-11-03 15:05 UTC (permalink / raw)
  To: buildroot

Hi everyone,

On Thu, Nov 3, 2016 at 2:58 AM, Arnout Vandecappelle <arnout@mind.be> wrote:

>
>
> On 03-11-16 00:27, Samuel Martin wrote:
> > Yann, Arnout, all,
> >
> > On Wed, Nov 2, 2016 at 11:18 PM, Yann E. MORIN <yann.morin.1998@free.fr>
> wrote:
> >> Samuel, All,
> >>
> >> On 2016-10-19 23:11 +0200, Thomas Petazzoni spake thusly:
> >>> commit: https://git.buildroot.net/buildroot/commit/?id=
> 173135df5b69dfd5ae6fe6cf2de8833c6f74c143
> >>> branch: https://git.buildroot.net/buildroot/commit/?id=refs/
> heads/master
> >>>
> >>> When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
> >>> resolved differently, depending on each package build-system (whether
> it
> >>> uses the given paths or get the absolute canonical ones).
> >>>
> >>> Using absolute canonical paths will help achieving reproducible builds
> and
> >>> will make easier tracking down host machine paths leaking into the
> host,
> >>> target or staging trees.
> >>> So, this change ensures the build takes place with the CURDIR and O
> >>> variables are set to their absolute canonical paths.
> >>>
> >>> In order to recall the toplevel makefile with absolute canonical paths
> >>> for $(CURDIR) and $(O), we need to:
> >>> 1- Compute the absolute canonical paths for $(CURDIR) and $(O) that
> will
> >>>    be passed to the sub-make. This is achieved using the 'realpath'
> make
> >>>    primitive. However, some care must be taken when manipulating O:
> >>>    - the out-of-tree makefile wrapper happens a trailing "/.", we need
> >>>      to strip this part away to not break the comparison driving the
> >>>      sub-make call;
> >>>    - the user can leave a trailing '/' to $(O);
> >>>    - according to [1,2], realpath returns an empty string in case of
> >>>      non-existing entry. So, to avoid passing an empty O= variable to
> >>>      sub-make, it is necessary to define the output directory and
> create
> >>>      it prior to call realpath on it (because on the first invocation,
> >>>      $(O) usually does not yet exists), hence the trick doing the mkdir
> >>>      right before calling realpath.
> >>> 2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
> >>>    when call recalling the top-level makefile with umask and paths
> >>>    correctly set.
> >>> 3- Lastly, update the condition for setting the CONFIG_DIR and
> >>>    NEED_WRAPPER variables.
> >>>
> >>> Note:
> >>> * This change takes care of the makefile wrapper installed in $(O) to
> >>>   avoid unneeded make recursion.
> >>
> >> This commit breaks a few use-cases that were previously working.
> >>
> >> for example, Maxime reports that:
> >>
> >>     make BR2_PRIMARY_SITE=blabla source
>
>>
> >> no longer works.
> >>
> >> Also, the autobuilders now no longer respect their 'njobs' configuration
> >> option, for the same reason; they (basivally) do:
> >>
> >>     make BR2_JLEVEL=kwargs["--njobs"]
> >>
> >> but this is nol onger working either.
> >>
> >> Care to have a look at this, please?
> >
> > Could you explain in details a test case you got (what you look at
> > exactly to get this, or maybe you just git-bisect-ed?)?
>
So here's my test case; I use an external and here's how my CI bot does it:

$ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
BR2_EXTERNAL=../myexternal tzdata-extract

It will go and download by using the BR2_PRIMARY_SITE defined in my
defconfig, everything's fine.
Now if I do that:

$ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
BR2_EXTERNAL=../myexternal BR2_PRIMARY_SITE="blabla" tzdata-extract
It is still gonna use the value defined in my defconfig instead of the one
on the command line which should take precedence.

It doesn't matter if you pass the variable to make or put it in the ENV.

>
> > I think it may be cause by this patch [1] not being apply.
> > Would you mind give it a try and report back?
>
It clearly fixes my issue and I assume the one encountered by the
autobuilders.

> > I gave it a quick try using this diff [2], and running 'make
> > foo-reconfigure' w/ and w/o various variables on the command line.
> >
> > Arnout, do you have an idea about what happens here?... this is a bit
> > beyond my make-fu :-/
>
>  Yay make-fu!
>
> http://patchwork.ozlabs.org/patch/690649/
>
>
>  I shouldn't have dismissed your patch [1] so easily...
>

>  Regards,
>  Arnout
>
> >
> > Regards,
> >
> > [1] http://patchwork.ozlabs.org/patch/682684/
> > [2] http://code.bulix.org/8k4x9g-107063/?raw
> >
> >
>
> --
> 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20161103/41d0075c/attachment.html>

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-11-03 15:05       ` Maxime Hadjinlian
@ 2016-11-03 15:19         ` Arnout Vandecappelle
  2016-11-03 17:56           ` Maxime Hadjinlian
  0 siblings, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2016-11-03 15:19 UTC (permalink / raw)
  To: buildroot



On 03-11-16 16:05, Maxime Hadjinlian wrote:
> Hi everyone,
> 
> On Thu, Nov 3, 2016 at 2:58 AM, Arnout Vandecappelle <arnout@mind.be
> <mailto:arnout@mind.be>> wrote:
> 
> 
> 
>     On 03-11-16 00:27, Samuel Martin wrote:
>     > Yann, Arnout, all,
>     >
>     > On Wed, Nov 2, 2016 at 11:18 PM, Yann E. MORIN <yann.morin.1998@free.fr
>     <mailto:yann.morin.1998@free.fr>> wrote:
>     >> Samuel, All,
>     >>
>     >> On 2016-10-19 23:11 +0200, Thomas Petazzoni spake thusly:
>     >>> commit:
>     https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143
>     <https://git.buildroot.net/buildroot/commit/?id=173135df5b69dfd5ae6fe6cf2de8833c6f74c143>
>     >>> branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
>     <https://git.buildroot.net/buildroot/commit/?id=refs/heads/master>
>     >>>
>     >>> When $(CURDIR) and/or $(O) contain symlinks in their paths, they can be
>     >>> resolved differently, depending on each package build-system (whether it
>     >>> uses the given paths or get the absolute canonical ones).
>     >>>
>     >>> Using absolute canonical paths will help achieving reproducible builds and
>     >>> will make easier tracking down host machine paths leaking into the host,
>     >>> target or staging trees.
>     >>> So, this change ensures the build takes place with the CURDIR and O
>     >>> variables are set to their absolute canonical paths.
>     >>>
>     >>> In order to recall the toplevel makefile with absolute canonical paths
>     >>> for $(CURDIR) and $(O), we need to:
>     >>> 1- Compute the absolute canonical paths for $(CURDIR) and $(O) that will
>     >>>    be passed to the sub-make. This is achieved using the 'realpath' make
>     >>>    primitive. However, some care must be taken when manipulating O:
>     >>>    - the out-of-tree makefile wrapper happens a trailing "/.", we need
>     >>>      to strip this part away to not break the comparison driving the
>     >>>      sub-make call;
>     >>>    - the user can leave a trailing '/' to $(O);
>     >>>    - according to [1,2], realpath returns an empty string in case of
>     >>>      non-existing entry. So, to avoid passing an empty O= variable to
>     >>>      sub-make, it is necessary to define the output directory and create
>     >>>      it prior to call realpath on it (because on the first invocation,
>     >>>      $(O) usually does not yet exists), hence the trick doing the mkdir
>     >>>      right before calling realpath.
>     >>> 2- Update EXTRAMAKEARGS with the absolute canonical $(O) and use it
>     >>>    when call recalling the top-level makefile with umask and paths
>     >>>    correctly set.
>     >>> 3- Lastly, update the condition for setting the CONFIG_DIR and
>     >>>    NEED_WRAPPER variables.
>     >>>
>     >>> Note:
>     >>> * This change takes care of the makefile wrapper installed in $(O) to
>     >>>   avoid unneeded make recursion.
>     >>
>     >> This commit breaks a few use-cases that were previously working.
>     >>
>     >> for example, Maxime reports that:
>     >>
>     >>     make BR2_PRIMARY_SITE=blabla source
> 
>     >>
>     >> no longer works.
>     >>
>     >> Also, the autobuilders now no longer respect their 'njobs' configuration
>     >> option, for the same reason; they (basivally) do:
>     >>
>     >>     make BR2_JLEVEL=kwargs["--njobs"]
>     >>
>     >> but this is nol onger working either.
>     >>
>     >> Care to have a look at this, please?
>     >
>     > Could you explain in details a test case you got (what you look at
>     > exactly to get this, or maybe you just git-bisect-ed?)?
> 
> So here's my test case; I use an external and here's how my CI bot does it:
> 
> $ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
> BR2_EXTERNAL=../myexternal tzdata-extract
> 
> It will go and download by using the BR2_PRIMARY_SITE defined in my defconfig,
> everything's fine.
> Now if I do that:
> 
> $ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
> BR2_EXTERNAL=../myexternal BR2_PRIMARY_SITE="blabla" tzdata-extract
> It is still gonna use the value defined in my defconfig instead of the one on
> the command line which should take precedence.
> 
> It doesn't matter if you pass the variable to make or put it in the ENV.

 So, have you tested http://patchwork.ozlabs.org/patch/690649/ ?

 If yes, can you add your Tested-by?

 Regards,
 Arnout

> 
>     >
>     > I think it may be cause by this patch [1] not being apply.
>     > Would you mind give it a try and report back?
> 
> It clearly fixes my issue and I assume the one encountered by the autobuilders. 
> 
>     > I gave it a quick try using this diff [2], and running 'make
>     > foo-reconfigure' w/ and w/o various variables on the command line.
>     >
>     > Arnout, do you have an idea about what happens here?... this is a bit
>     > beyond my make-fu :-/
> 
>      Yay make-fu!
> 
>     http://patchwork.ozlabs.org/patch/690649/
>     <http://patchwork.ozlabs.org/patch/690649/>
> 
> 
>      I shouldn't have dismissed your patch [1] so easily...
> 
> 
>      Regards,
>      Arnout
> 
>     >
>     > Regards,
>     >
>     > [1] http://patchwork.ozlabs.org/patch/682684/
>     <http://patchwork.ozlabs.org/patch/682684/>
>     > [2] http://code.bulix.org/8k4x9g-107063/?raw
>     <http://code.bulix.org/8k4x9g-107063/?raw>
>     >
>     >
> 
>     --
>     Arnout Vandecappelle                          arnout at mind be
>     Senior Embedded Software Architect            +32-16-286500
>     <tel:%2B32-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
>     <http://www.linkedin.com/in/arnoutvandecappelle>
>     GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
> 
> 

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths
  2016-11-03 15:19         ` Arnout Vandecappelle
@ 2016-11-03 17:56           ` Maxime Hadjinlian
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Hadjinlian @ 2016-11-03 17:56 UTC (permalink / raw)
  To: buildroot

On Thu, Nov 3, 2016 at 4:19 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
> >
> > So here's my test case; I use an external and here's how my CI bot does it:
> >
> > $ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
> > BR2_EXTERNAL=../myexternal tzdata-extract
> >
> > It will go and download by using the BR2_PRIMARY_SITE defined in my defconfig,
> > everything's fine.
> > Now if I do that:
> >
> > $ rm buildroot/dl/tzdata* ; make O=../output -C buildroot
> > BR2_EXTERNAL=../myexternal BR2_PRIMARY_SITE="blabla" tzdata-extract
> > It is still gonna use the value defined in my defconfig instead of the one on
> > the command line which should take precedence.
> >
> > It doesn't matter if you pass the variable to make or put it in the ENV.
>
>  So, have you tested http://patchwork.ozlabs.org/patch/690649/ ?

No, I have tested http://patchwork.ozlabs.org/patch/682684/ as pointed
to by Samuel.

Let me test this one, I'll report back soon.
>
>
>  If yes, can you add your Tested-by?
>
>  Regards,
>  Arnout
>

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

end of thread, other threads:[~2016-11-03 17:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-19 21:11 [Buildroot] [git commit] core: re-enter make if $(CURDIR) or $(O) are not canonical paths Thomas Petazzoni
2016-11-02 22:18 ` Yann E. MORIN
2016-11-02 23:27   ` Samuel Martin
2016-11-03  1:58     ` Arnout Vandecappelle
2016-11-03 15:05       ` Maxime Hadjinlian
2016-11-03 15:19         ` Arnout Vandecappelle
2016-11-03 17:56           ` Maxime Hadjinlian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.