Message ID | 20181114105557.12599-5-thomas.petazzoni@bootlin.com |
---|---|
State | Superseded |
Headers | show |
Series | Per-package host/target directory support | expand |
Thomas, All, On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly: > In a follow-up commit, we will make the .NOTPARALLEL statement > conditional on a Config.in option, so we need to move it further down. Ultimately, I don't see why we do need a config option to turn top-level parallel build on/off. It will ultimately be the user's choice to do so when calling 'make -jN' no? However, what we do need for now, is a config option that enabled per-package directories (PPD, formerly known as PPS, per-package staging). When that is turned off, we must not allow top-level parallel build. I was thinking that we would not need that option either, because we would enable PPD when we detect that the user is doing top-level parllel build, by inspecting $(MAKEFLAGS) to see if they contained -j. However, make is our fiend here, because it does not include -j in the MAKFLAGS variable, but the one that is exported: $ cat Makefile $(info var MAKEFLAGS='$(MAKEFLAGS)') all: @echo "env MAKEFLAGS='$${MAKEFLAGS}'" $ make -j1 var MAKEFLAGS='' env MAKEFLAGS='' $ make -j3 var MAKEFLAGS='' env MAKEFLAGS=' -j --jobserver-fds=3,4' $ make -j var MAKEFLAGS='' env MAKEFLAGS=' -j' And there is no other variable in make that specifies that either... So yes, this means we have no way from the Makefile to detect whether we're parallel or not. Sigh. :-( So we do need that option for PPD. > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN. > --- > Makefile | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/Makefile b/Makefile > index e675ac26aa..24c803872d 100644 > --- a/Makefile > +++ b/Makefile > @@ -105,22 +105,6 @@ ifneq ($(firstword $(sort $(RUNNING_MAKE_VERSION) $(MIN_MAKE_VERSION))),$(MIN_MA > $(error You have make '$(RUNNING_MAKE_VERSION)' installed. GNU make >= $(MIN_MAKE_VERSION) is required) > endif > > -# Parallel execution of this Makefile is disabled because it changes > -# the packages building order, that can be a problem for two reasons: > -# - If a package has an unspecified optional dependency and that > -# dependency is present when the package is built, it is used, > -# otherwise it isn't (but compilation happily proceeds) so the end > -# result will differ if the order is swapped due to parallel > -# building. > -# - Also changing the building order can be a problem if two packages > -# manipulate the same file in the target directory. > -# > -# Taking into account the above considerations, if you still want to execute > -# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and > -# use the -j<jobs> option when building, e.g: > -# make -j$((`getconf _NPROCESSORS_ONLN`+1)) > -.NOTPARALLEL: > - > # absolute path > TOPDIR := $(CURDIR) > CONFIG_CONFIG_IN = Config.in > @@ -246,6 +230,22 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),) > -include $(BR2_CONFIG) > endif > > +# Parallel execution of this Makefile is disabled because it changes > +# the packages building order, that can be a problem for two reasons: > +# - If a package has an unspecified optional dependency and that > +# dependency is present when the package is built, it is used, > +# otherwise it isn't (but compilation happily proceeds) so the end > +# result will differ if the order is swapped due to parallel > +# building. > +# - Also changing the building order can be a problem if two packages > +# manipulate the same file in the target directory. > +# > +# Taking into account the above considerations, if you still want to execute > +# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and > +# use the -j<jobs> option when building, e.g: > +# make -j$((`getconf _NPROCESSORS_ONLN`+1)) > +.NOTPARALLEL: > + > # timezone and locale may affect build output > ifeq ($(BR2_REPRODUCIBLE),y) > export TZ = UTC > -- > 2.19.1 >
Hello, On Thu, 15 Nov 2018 22:37:58 +0100, Yann E. MORIN wrote: > Ultimately, I don't see why we do need a config option to turn top-level > parallel build on/off. It will ultimately be the user's choice to do so > when calling 'make -jN' no? > > However, what we do need for now, is a config option that enabled > per-package directories (PPD, formerly known as PPS, per-package > staging). When that is turned off, we must not allow top-level > parallel build. > > I was thinking that we would not need that option either, because we > would enable PPD when we detect that the user is doing top-level parllel > build, by inspecting $(MAKEFLAGS) to see if they contained -j. > > However, make is our fiend here, because it does not include -j in the > MAKFLAGS variable, but the one that is exported: > > $ cat Makefile > $(info var MAKEFLAGS='$(MAKEFLAGS)') > all: > @echo "env MAKEFLAGS='$${MAKEFLAGS}'" > > $ make -j1 > var MAKEFLAGS='' > env MAKEFLAGS='' > > $ make -j3 > var MAKEFLAGS='' > env MAKEFLAGS=' -j --jobserver-fds=3,4' > > $ make -j > var MAKEFLAGS='' > env MAKEFLAGS=' -j' > > And there is no other variable in make that specifies that either... > > So yes, this means we have no way from the Makefile to detect whether > we're parallel or not. Sigh. :-( > > So we do need that option for PPD. > > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Thanks for this research and the review. I think there is another reason to have an option, at least for the moment: I'm pretty sure a number of people not aware of top-level parallel build issues use Buildroot with "make -jN" today, as they have learned that using make -jN will "speed up their build" (I mean in general, not specifically for Buildroot). If we were to use make -jN as the criteria to decide if PPD should be used or not, then suddenly all such users would start using PPD, which is still way too experimental to expose it without the user's explicit consent. My plan is rather the following: (1) Have it under an explicit option for now. (2) Once we consider it stable, drop the option entirely, and always use PPD, even if the user doesn't use top-level parallel build. There's a slight additional cost, but there's also a huge benefit in terms of dependency checking/isolation. Note that we might stay on step (1) for a little while, I think it will take time to really make it fully usable. Best regards, Thomas
diff --git a/Makefile b/Makefile index e675ac26aa..24c803872d 100644 --- a/Makefile +++ b/Makefile @@ -105,22 +105,6 @@ ifneq ($(firstword $(sort $(RUNNING_MAKE_VERSION) $(MIN_MAKE_VERSION))),$(MIN_MA $(error You have make '$(RUNNING_MAKE_VERSION)' installed. GNU make >= $(MIN_MAKE_VERSION) is required) endif -# Parallel execution of this Makefile is disabled because it changes -# the packages building order, that can be a problem for two reasons: -# - If a package has an unspecified optional dependency and that -# dependency is present when the package is built, it is used, -# otherwise it isn't (but compilation happily proceeds) so the end -# result will differ if the order is swapped due to parallel -# building. -# - Also changing the building order can be a problem if two packages -# manipulate the same file in the target directory. -# -# Taking into account the above considerations, if you still want to execute -# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and -# use the -j<jobs> option when building, e.g: -# make -j$((`getconf _NPROCESSORS_ONLN`+1)) -.NOTPARALLEL: - # absolute path TOPDIR := $(CURDIR) CONFIG_CONFIG_IN = Config.in @@ -246,6 +230,22 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),) -include $(BR2_CONFIG) endif +# Parallel execution of this Makefile is disabled because it changes +# the packages building order, that can be a problem for two reasons: +# - If a package has an unspecified optional dependency and that +# dependency is present when the package is built, it is used, +# otherwise it isn't (but compilation happily proceeds) so the end +# result will differ if the order is swapped due to parallel +# building. +# - Also changing the building order can be a problem if two packages +# manipulate the same file in the target directory. +# +# Taking into account the above considerations, if you still want to execute +# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and +# use the -j<jobs> option when building, e.g: +# make -j$((`getconf _NPROCESSORS_ONLN`+1)) +.NOTPARALLEL: + # timezone and locale may affect build output ifeq ($(BR2_REPRODUCIBLE),y) export TZ = UTC
In a follow-up commit, we will make the .NOTPARALLEL statement conditional on a Config.in option, so we need to move it further down. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- Makefile | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)