| Message ID | 99185cfa61404dfa019a5e2c292b928791e85af5.1712848167.git.quic_mathbern@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: fix use of -j without an argument | expand |
Hi Matheus, On 11/4/24 17:09, Matheus Tavares Bernardino wrote: > Our Makefile massages the given make arguments to invoke ninja > accordingly. One key difference is that ninja will parallelize by > default, whereas make only does so with -j<n> or -j. The make man page > says that "if the -j option is given without an argument, make will not > limit the number of jobs that can run simultaneously". We use to support > that by replacing -j with "" (empty string) when calling ninja, so that > it would do its auto-parallelization based on the number of CPU cores. > > This was accidentally broken at d1ce2cc95b (Makefile: preserve > --jobserver-auth argument when calling ninja, 2024-04-02), > causing `make -j` to fail: > > $ make -j V=1 > /usr/bin/ninja -v -j -d keepdepfile all | cat > make -C contrib/plugins/ V="1" TARGET_DIR="contrib/plugins/" all > ninja: fatal: invalid -j parameter > make: *** [Makefile:161: run-ninja] Error > > Let's fix that and indent the touched code for better readability. > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > --- > Makefile | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 183756018f..d299c14dab 100644 > --- a/Makefile > +++ b/Makefile > @@ -142,8 +142,12 @@ MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS)))) > MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS)))) > MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq) > NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ > - $(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ > - -d keepdepfile > + $(if $(filter -j, $(MAKEFLAGS)) \ > + ,, \ > + $(or \ > + $(filter -l% -j%, $(MAKEFLAGS)), \ > + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ > + ) -d keepdepfile > ninja-cmd-goals = $(or $(MAKECMDGOALS), all) > ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g)) > Apparently Martin sent the same patch (although not as nicely indented) and Paolo queued it: https://lore.kernel.org/qemu-devel/20240402081738.1051560-1-martin@geanix.com/
Hi, Philippe On Thu, 11 Apr 2024 17:29:58 +0200 =?UTF-8?Q?Philippe_Mathieu-Daud=C3=A9?= <philmd@linaro.org> wrote: > > Hi Matheus, > > On 11/4/24 17:09, Matheus Tavares Bernardino wrote: > > Our Makefile massages the given make arguments to invoke ninja > > accordingly. One key difference is that ninja will parallelize by > > default, whereas make only does so with -j<n> or -j. The make man page > > says that "if the -j option is given without an argument, make will not > > limit the number of jobs that can run simultaneously". We use to support > > that by replacing -j with "" (empty string) when calling ninja, so that > > it would do its auto-parallelization based on the number of CPU cores. > > > > This was accidentally broken at d1ce2cc95b (Makefile: preserve > > --jobserver-auth argument when calling ninja, 2024-04-02), > > causing `make -j` to fail: > > > > $ make -j V=1 > > /usr/bin/ninja -v -j -d keepdepfile all | cat > > make -C contrib/plugins/ V="1" TARGET_DIR="contrib/plugins/" all > > ninja: fatal: invalid -j parameter > > make: *** [Makefile:161: run-ninja] Error > > > > Let's fix that and indent the touched code for better readability. > > > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > > --- > > Makefile | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 183756018f..d299c14dab 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -142,8 +142,12 @@ MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS)))) > > MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS)))) > > MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq) > > NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ > > - $(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ > > - -d keepdepfile > > + $(if $(filter -j, $(MAKEFLAGS)) \ > > + ,, \ > > + $(or \ > > + $(filter -l% -j%, $(MAKEFLAGS)), \ > > + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ > > + ) -d keepdepfile > > ninja-cmd-goals = $(or $(MAKECMDGOALS), all) > > ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g)) > > > > Apparently Martin sent the same patch (although not as nicely > indented) and Paolo queued it: > https://lore.kernel.org/qemu-devel/20240402081738.1051560-1-martin@geanix.com/ Actually, this patch is a follow-up to that one, fixing a feature that was accidentally broken.
On 11/4/24 17:38, Matheus Tavares Bernardino wrote: > Hi, Philippe > > On Thu, 11 Apr 2024 17:29:58 +0200 =?UTF-8?Q?Philippe_Mathieu-Daud=C3=A9?= <philmd@linaro.org> wrote: >> >> Hi Matheus, >> >> On 11/4/24 17:09, Matheus Tavares Bernardino wrote: >>> Our Makefile massages the given make arguments to invoke ninja >>> accordingly. One key difference is that ninja will parallelize by >>> default, whereas make only does so with -j<n> or -j. The make man page >>> says that "if the -j option is given without an argument, make will not >>> limit the number of jobs that can run simultaneously". We use to support >>> that by replacing -j with "" (empty string) when calling ninja, so that >>> it would do its auto-parallelization based on the number of CPU cores. >>> >>> This was accidentally broken at d1ce2cc95b (Makefile: preserve >>> --jobserver-auth argument when calling ninja, 2024-04-02), >>> causing `make -j` to fail: >>> >>> $ make -j V=1 >>> /usr/bin/ninja -v -j -d keepdepfile all | cat >>> make -C contrib/plugins/ V="1" TARGET_DIR="contrib/plugins/" all >>> ninja: fatal: invalid -j parameter >>> make: *** [Makefile:161: run-ninja] Error >>> >>> Let's fix that and indent the touched code for better readability. >>> >>> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> >>> --- >>> Makefile | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/Makefile b/Makefile >>> index 183756018f..d299c14dab 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -142,8 +142,12 @@ MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS)))) >>> MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS)))) >>> MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq) >>> NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ >>> - $(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ >>> - -d keepdepfile >>> + $(if $(filter -j, $(MAKEFLAGS)) \ >>> + ,, \ >>> + $(or \ >>> + $(filter -l% -j%, $(MAKEFLAGS)), \ >>> + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ >>> + ) -d keepdepfile >>> ninja-cmd-goals = $(or $(MAKECMDGOALS), all) >>> ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g)) >>> >> >> Apparently Martin sent the same patch (although not as nicely >> indented) and Paolo queued it: >> https://lore.kernel.org/qemu-devel/20240402081738.1051560-1-martin@geanix.com/ > > Actually, this patch is a follow-up to that one, fixing a feature that was > accidentally broken. Oops sorry I missed that, I was not expecting this patch to be merged in 9.0 :/
On Thu, Apr 11, 2024 at 5:46 PM Matheus Tavares Bernardino <quic_mathbern@quicinc.com> wrote: > + $(if $(filter -j, $(MAKEFLAGS)) \ > + ,, \ > + $(or \ > + $(filter -l% -j%, $(MAKEFLAGS)), \ > + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ > + ) -d keepdepfile This is more easily written as $(filter-out -j, $(or ...)). I've sent a v2. Paolo > ninja-cmd-goals = $(or $(MAKECMDGOALS), all) > ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g)) > > -- > 2.37.2 >
diff --git a/Makefile b/Makefile index 183756018f..d299c14dab 100644 --- a/Makefile +++ b/Makefile @@ -142,8 +142,12 @@ MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS)))) MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS)))) MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq) NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ - $(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ - -d keepdepfile + $(if $(filter -j, $(MAKEFLAGS)) \ + ,, \ + $(or \ + $(filter -l% -j%, $(MAKEFLAGS)), \ + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \ + ) -d keepdepfile ninja-cmd-goals = $(or $(MAKECMDGOALS), all) ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g))
Our Makefile massages the given make arguments to invoke ninja accordingly. One key difference is that ninja will parallelize by default, whereas make only does so with -j<n> or -j. The make man page says that "if the -j option is given without an argument, make will not limit the number of jobs that can run simultaneously". We use to support that by replacing -j with "" (empty string) when calling ninja, so that it would do its auto-parallelization based on the number of CPU cores. This was accidentally broken at d1ce2cc95b (Makefile: preserve --jobserver-auth argument when calling ninja, 2024-04-02), causing `make -j` to fail: $ make -j V=1 /usr/bin/ninja -v -j -d keepdepfile all | cat make -C contrib/plugins/ V="1" TARGET_DIR="contrib/plugins/" all ninja: fatal: invalid -j parameter make: *** [Makefile:161: run-ninja] Error Let's fix that and indent the touched code for better readability. Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> --- Makefile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)