[Buildroot] [git commit] Makefile: don't depend on the umask

[Buildroot] [git commit] Makefile: don't depend on the umask

[Thomas Petazzoni]

commit: http://git.buildroot.net/buildroot/commit/?id=bee5745ccc20be6dbba243b1f8af0d5c522923e8
branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master

Some packages and BR itself create files and directories on the target
with cp/mkdir/etc which depend on the umask at the time of building.

To fix this, use a trick inside the Makefile which wraps all rules when
the umask is not 0022. This sets the umask at the top level, and then
the building process continues as usual.

[Thomas: add --no-print-directory, as suggested by Arnout.]

Signed-off-by: Guido Mart??nez <guido@vanguardiasur.com.ar>
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 |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index daf692e..55b1d45 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,19 @@
 # You shouldn't need to mess with anything beyond this point...
 #--------------------------------------------------------------
 
+# Trick for always running with a fixed umask
+UMASK=0022
+ifneq ($(shell umask),$(UMASK))
+.PHONY: all $(MAKECMDGOALS)
+
+all:
+	@umask $(UMASK) && $(MAKE) --no-print-directory
+
+$(MAKECMDGOALS):
+	@umask $(UMASK) && $(MAKE) --no-print-directory $@
+
+else # umask
+
 # This is our default rule, so must come first
 all:
 
@@ -937,3 +950,5 @@ include docs/manual/manual.mk
 -include $(BR2_EXTERNAL)/docs/*/*.mk
 
 .PHONY: $(noconfig_targets)
+
+endif #umask

[Buildroot] Major regression introduced by "Makefile: don't depend on the umask"

[Thomas Petazzoni]

Guido, Arnout,

On Mon, 13 Jul 2015 12:57:58 +0200, Thomas Petazzoni wrote:

> diff --git a/Makefile b/Makefile
> index daf692e..55b1d45 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -24,6 +24,19 @@
>  # You shouldn't need to mess with anything beyond this point...
>  #--------------------------------------------------------------
>  
> +# Trick for always running with a fixed umask
> +UMASK=0022
> +ifneq ($(shell umask),$(UMASK))
> +.PHONY: all $(MAKECMDGOALS)
> +
> +all:
> +	@umask $(UMASK) && $(MAKE) --no-print-directory
> +
> +$(MAKECMDGOALS):
> +	@umask $(UMASK) && $(MAKE) --no-print-directory $@
> +

I believe this unfortunately doesn't work properly: if you do a make
invocation with multiple targets, it will run on sub-make invocation
for each of them, causing all Buildroot makefiles to be parsed for each
target instead of only once.

You can do the test yourself. As a preparation, do something like:

$ make libesmtp-patch fbset-patch findutils-patch fping-patch gamin-patch gesftpserver-patch gmpc-patch iftop-patch jasper-patch less-patch libcap-ng-patch libdvdnav-patch libdvdread-patch libcuefile-patch musepack-patch libreplaygain-patch libsamplerate-patch libsndfile-patch linux-fusion-patch lm-sensors-patch mplayer-patch host-nasm-patch ngircd-patch proftpd-patch sawman-patch sdl_gfx-patch sdl_image-patch sdl_mixer-patch sdl_net-patch sdl_sound-patch sdl_ttf-patch sdl-patch shared-mime-info-patch spice-protocol-patch spice-patch statserial-patch taglib-patch udisks-patch time-patch tcping-patch

Once this is done, we will run this command once again. It should do
nothing since everything has already be done and therefore be fairly
quick. But it's not if your system has a umask different from 022,
since the mechanism invoking the sub-make will invoke one sub-make for
each target:

$ time make libesmtp-patch fbset-patch findutils-patch fping-patch gamin-patch gesftpserver-patch gmpc-patch iftop-patch jasper-patch less-patch libcap-ng-patch libdvdnav-patch libdvdread-patch libcuefile-patch musepack-patch libreplaygain-patch libsamplerate-patch libsndfile-patch linux-fusion-patch lm-sensors-patch mplayer-patch host-nasm-patch ngircd-patch proftpd-patch sawman-patch sdl_gfx-patch sdl_image-patch sdl_mixer-patch sdl_net-patch sdl_sound-patch sdl_ttf-patch sdl-patch shared-mime-info-patch spice-protocol-patch spice-patch statserial-patch taglib-patch udisks-patch time-patch tcping-patch

(during this command, the CPU goes crazy to 100%)

real	1m50.089s
user	1m41.480s
sys	0m3.580s

Now, forcing the umask to 022 before running the command, with bypasses
the new mechanism:

$ umask 022
$ time make libesmtp-patch fbset-patch findutils-patch fping-patch gamin-patch gesftpserver-patch gmpc-patch iftop-patch jasper-patch less-patch libcap-ng-patch libdvdnav-patch libdvdread-patch libcuefile-patch musepack-patch libreplaygain-patch libsamplerate-patch libsndfile-patch linux-fusion-patch lm-sensors-patch mplayer-patch host-nasm-patch ngircd-patch proftpd-patch sawman-patch sdl_gfx-patch sdl_image-patch sdl_mixer-patch sdl_net-patch sdl_sound-patch sdl_ttf-patch sdl-patch shared-mime-info-patch spice-protocol-patch spice-patch statserial-patch taglib-patch udisks-patch time-patch tcping-patch
make: Nothing to be done for 'fbset-patch'.
make: Nothing to be done for 'findutils-patch'.
make: Nothing to be done for 'fping-patch'.
make: Nothing to be done for 'gamin-patch'.
make: Nothing to be done for 'gesftpserver-patch'.
make: Nothing to be done for 'gmpc-patch'.
make: Nothing to be done for 'iftop-patch'.
make: Nothing to be done for 'jasper-patch'.
make: Nothing to be done for 'less-patch'.
make: Nothing to be done for 'libcap-ng-patch'.
make: Nothing to be done for 'libdvdnav-patch'.
make: Nothing to be done for 'libdvdread-patch'.
make: Nothing to be done for 'libcuefile-patch'.
make: Nothing to be done for 'musepack-patch'.
make: Nothing to be done for 'libreplaygain-patch'.
make: Nothing to be done for 'libsamplerate-patch'.
make: Nothing to be done for 'libsndfile-patch'.
make: Nothing to be done for 'linux-fusion-patch'.
make: Nothing to be done for 'lm-sensors-patch'.
make: Nothing to be done for 'mplayer-patch'.
make: Nothing to be done for 'host-nasm-patch'.
make: Nothing to be done for 'ngircd-patch'.
make: Nothing to be done for 'proftpd-patch'.
make: Nothing to be done for 'sawman-patch'.
make: Nothing to be done for 'sdl_gfx-patch'.
make: Nothing to be done for 'sdl_image-patch'.
make: Nothing to be done for 'sdl_mixer-patch'.
make: Nothing to be done for 'sdl_net-patch'.
make: Nothing to be done for 'sdl_sound-patch'.
make: Nothing to be done for 'sdl_ttf-patch'.
make: Nothing to be done for 'sdl-patch'.
make: Nothing to be done for 'shared-mime-info-patch'.
make: Nothing to be done for 'spice-protocol-patch'.
make: Nothing to be done for 'spice-patch'.
make: Nothing to be done for 'statserial-patch'.
make: Nothing to be done for 'taglib-patch'.
make: Nothing to be done for 'udisks-patch'.
make: Nothing to be done for 'time-patch'.
make: Nothing to be done for 'tcping-patch'.

real	0m2.645s
user	0m2.424s
sys	0m0.108s

So what was taking 2.6 seconds is now taking 1 minute and 50 seconds.
Not really a great improvement, as you can imagine :-/

Could you look into this problem?

Best regards,

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

[Buildroot] [PATCH] Makefile: optimize umask setting

[Guido Martínez]

When using make to build multiple targets ("make a b ..."), the makefile
would get called individually for each target, making the build a lot
slower since every Makefile in BR's tree was parsed each time.

Fix this by making every invoked target depend on an "_all" rule, and
doing the real work there, only one time. This is similar to the logic
used in the generated makefile (support/scripts/mkmakefile).

Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 531ac5d..f50f767 100644
--- a/Makefile
+++ b/Makefile
@@ -27,13 +27,13 @@
 # Trick for always running with a fixed umask
 UMASK=0022
 ifneq ($(shell umask),$(UMASK))
-.PHONY: all $(MAKECMDGOALS)
+.PHONY: _all
 
-all:
-	@umask $(UMASK) && $(MAKE) --no-print-directory
+$(MAKECMDGOALS): _all
+	@:
 
-$(MAKECMDGOALS):
-	@umask $(UMASK) && $(MAKE) --no-print-directory $@
+_all:
+	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
 
 else # umask
 
-- 
2.1.4

[Buildroot] [PATCH] Makefile: fix performance regression introduced by "Makefile: don't depend on the umask"

[Arnout Vandecappelle]

Commit bee5745c introduced an extra level of 'make' when the umask is
different from 0022. However, when several targets were specified on
the command line, a new make instance would be called for each target.
This introduces a huge performance overhead when many targets are
specified on the command line.

To fix this, use the same approach as used in the mkmakefile script:
an addition target on which the MAKECMDGOALS depend, so that this
target is run only once.

Note that the mkmakefile script contains a special exception for
Makefile, because the Makefile in the output directory is generated.
Since the top-level Makefile is not generated, this exception is not
needed here.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Guido Mart?nez <guido@vanguardiasur.com.a>
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c7d6bee..b4ef2fa 100644
--- a/Makefile
+++ b/Makefile
@@ -29,11 +29,11 @@ UMASK=0022
 ifneq ($(shell umask),$(UMASK))
 .PHONY: all $(MAKECMDGOALS)
 
-all:
-	@umask $(UMASK) && $(MAKE) --no-print-directory
+_all:
+	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKEARGS) $(MAKECMDGOALS)
 
-$(MAKECMDGOALS):
-	@umask $(UMASK) && $(MAKE) --no-print-directory $@
+$(MAKECMDGOALS): _all
+	@:
 
 else # umask
 
-- 
2.1.4

[Buildroot] [PATCH] Makefile: optimize umask setting

[Arnout Vandecappelle]

On 07/16/15 23:55, Guido Mart?nez wrote:
> When using make to build multiple targets ("make a b ..."), the makefile
> would get called individually for each target, making the build a lot
> slower since every Makefile in BR's tree was parsed each time.
> 
> Fix this by making every invoked target depend on an "_all" rule, and
> doing the real work there, only one time. This is similar to the logic
> used in the generated makefile (support/scripts/mkmakefile).
> 
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
>  Makefile | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 531ac5d..f50f767 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -27,13 +27,13 @@
>  # Trick for always running with a fixed umask
>  UMASK=0022
>  ifneq ($(shell umask),$(UMASK))
> -.PHONY: all $(MAKECMDGOALS)
> +.PHONY: _all

 $(MAKECMDGOALS) should still be .PHONY (the non-phony targets are only
non-phony on the next level of make invocation).

 But in my patch, I forgot to replace all by _all :-)

>  
> -all:
> -	@umask $(UMASK) && $(MAKE) --no-print-directory
> +$(MAKECMDGOALS): _all
> +	@:
>  
> -$(MAKECMDGOALS):
> -	@umask $(UMASK) && $(MAKE) --no-print-directory $@
> +_all:
> +	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)

 That's indeed a more logical order.

 mkmakefile also adds $(MAKEARGS) - should we keep that?

 Regards,
 Arnout

>  
>  else # umask
>  
> 


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

[Buildroot] [PATCH] Makefile: fix performance regression introduced by "Makefile: don't depend on the umask"

[Arnout Vandecappelle]

Commit bee5745c introduced an extra level of 'make' when the umask is
different from 0022. However, when several targets were specified on
the command line (i.e., 'make a b c'), a new make instance would be
called for each target, which each times parses all the makefiles. This
introduces a huge performance overhead when many targets are specified
on the command line.

To fix this, use the same approach as used in the mkmakefile script:
an additional _all target on which the MAKECMDGOALS depend, so that this
target is run only once.

Note that the mkmakefile script contains a special exception for
Makefile, because the Makefile in the output directory is generated.
Since the top-level Makefile is not generated, this exception is not
needed here.

While we're at it, also fix the whitespace in the UMASK assignment.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Guido Mart?nez <guido@vanguardiasur.com.r>
---
v2:
 - replace _all by all in .PHONY
 - remove the useless MAKEFLAGS (it is not defined, the variable is
   called MAKEFLAGS but that's already included in MAKE)
 - add UMASK whitespace fix
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c7d6bee..b4ef2fa 100644
--- a/Makefile
+++ b/Makefile
@@ -29,11 +29,11 @@ UMASK=0022
 ifneq ($(shell umask),$(UMASK))
 .PHONY: all $(MAKECMDGOALS)
 
-all:
-	@umask $(UMASK) && $(MAKE) --no-print-directory
+_all:
+	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKEARGS) $(MAKECMDGOALS)
 
-$(MAKECMDGOALS):
-	@umask $(UMASK) && $(MAKE) --no-print-directory $@
+$(MAKECMDGOALS): _all
+	@:
 
 else # umask
 
-- 
2.1.4