Re: Regression caused by Commit: 5bd7ad2b225b ("Preserve the real value of -jN in MAKEFLAGS using jobserver.")

[NeilBrown]

On Thu, Sep 28 2017, NeilBrown wrote:

> Hi,
> One of my Makefiles contains
>   MAKEFLAGS += -j
>
> Prior to the above commit, this would cause make to run as if I had run
>    make -j
>
> Since that commit, it appears to be ignored.  If I run
>   make -j
> explicitly I get parallel makes, but setting MAKEFLAGS in the Makefile
> doesn't achieve that result.
>
> This usage is still described as supported by make.texi:
>
> The @code{MAKEFLAGS} variable can also be useful if you want to have
> certain options, such as @samp{-k} (@pxref{Options Summary, ,Summary of
> Options}), set each time you run @code{make}.  You simply put a value for
> @code{MAKEFLAGS} in your environment.  You can also set @code{MAKEFLAGS} in
> a makefile, to specify additional flags that should also be in effect for
> that makefile.  (Note that you cannot use @code{MFLAGS} this way.  That
> variable is set only for compatibility; @code{make} does not interpret a
> value you set for it in any way.)


I dug into this a bit.
The regression is caused because the 'j' flag now causes arg_job_slots
to be set, rather than setting job_slots directly.
arg_job_slots is conditionally copied into job_slots after the call to

    decode_env_switches (STRING_SIZE_TUPLE ("MAKEFLAGS"));

at line 1453, but that is called again at line 1993 and the value set
then (which is when the value from "MAKEFLAGS += -j" in a Makefile takes
effect) is lost.

A possible fix is:

diff --git a/main.c b/main.c
index 226b26b96862..46e28379d687 100644
--- a/main.c
+++ b/main.c
@@ -1995,6 +1995,10 @@ main (int argc, char **argv, char **envp)
     decode_env_switches (STRING_SIZE_TUPLE ("MFLAGS"));
 #endif
 
+    if (!jobserver_auth)
+           /* MAKEFLAGS might have changed arg_job_slots */
+           job_slots = arg_job_slots;
+
     /* Reset in case the switches changed our mind.  */
     syncing = (output_sync == OUTPUT_SYNC_LINE
                || output_sync == OUTPUT_SYNC_TARGET);

but that feels like a bit of a hack.  I don't know the code well enough
to be sure.

Thanks,
NeilBrown

signature.asc
Description: PGP signature