Makefile: don't depend on the umask
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.]
--- Makefile | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
Makefile +15 -0
@@ -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"
Guido, Arnout,
Quoted 9 lines · Thomas Petazzoni
Makefile +1
-0@@ -24,6 +24,19 @@ # You shouldn't need to mess with anything beyond this point... #-------------------------------------------------------------- +# Trick for always running with a fixed umask
Makefile line 37
Quoted 10 lines
Makefile +10
-0+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
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/1 Makefile: optimize umask setting
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).
--- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Makefile +5 -5
@@ -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
Signature · 1 line
2.1.4
PATCH 1/1 Makefile: optimize umask setting
Quoted 18 lines · 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
Makefilein 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 <[email protected]>--- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)Makefile +0
-0
Makefile line 30
Quoted 5 lines
Makefile +1
-1@@ -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 :-)
Makefile line 36
Quoted 10 lines
Makefile +4
-4-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
Quoted 4 lines
Makefile +0
-0else # umask
Signature · 6 lines
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
PATCH 1/1 Makefile: fix performance regression introduced by "Makefile: don't depend on the umask"
In reply to Thomas PetazzoniCommit 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.
--- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Makefile +4 -4
@@ -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
Signature · 1 line
2.1.4
PATCH 1/1 Makefile: fix performance regression introduced by "Makefile: don't depend on the umask"
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.
---
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(-)
Makefile +4 -4
@@ -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
Signature · 1 line
2.1.4