Re: Port silent mode detection to future gnu make.

Re: Port silent mode detection to future gnu make.

[Masahiro Yamada]

On Sun, Nov 27, 2022 at 12:43 PM Dmitry Goncharov
<dgoncharov@users.sf.net> wrote:
>
>
>
> Port silent mode detection to future gnu make.
>
> Makefile uses the following piece of make code to detect if option -s is
> specified on the command line.
>
> ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
>
>
> This code is executed by make at parse time and assumes that MAKEFLAGS does
> not contain command line variable definitions.
> Currently there is a change to make under discussion here
> https://savannah.gnu.org/bugs/?63347.
> This proposed patch modifies make such that at parse time MAKEFLAGS
> contains command line variable definitions.
> Currently if the user defines a=s on the command line, then at build only
> time MAKEFLAGS contains " -- a=s".
> With the patch proposed in sv63347, MAKEFLAGS will contain this " -- a=s"
> at parse time and build time.
>
>
> Once this change is applied this code will confuse a command line variable
> definition which contains letter 's' with option -s.
>
> E.g.
> $ # old make
> $ make net/wireless/ocb.o a=s
>   CALL    scripts/checksyscalls.sh
>   DESCEND objtool
> $ # this a new make which defines makeflags at parse time
> $ ~/src/gmake/make/l64/make net/wireless/ocb.o a=s
> $
>
> We can see here that letter 's' from 'a=s' was confused with -s.
>
> This patch checks for presence of -s using a method recommended by the make
> manual here
> https://www.gnu.org/software/make/manual/make.html#Testing-Flags.
>
> This suggested method will work with the old and new make.
>
> Signed-off-by: Dmitry Goncharov <dgoncharov@users.sf.net>
>
> Reported-by: Jan Palus <jpalus+gnu@fastmail.com>




I like MAKEFLAGS defined on-the-fly in the parse stage,
so I appreciate your sv63347_fix.diff.


I have no objection to changing the kernel Makefile, but I am
not willing to do it based on the proposal that has not even
queued up in the git tree.

Please come back after your patch is applied
(and please have the patch include the commit hash
causing the behaviour change)


BTW, the GNU Make manual suggests $(word 1) instead of $(firstword).
Is it for the purpose of backward compatibility for older
Make versions?

The kernel build only supports Make>=3.82, and I personally prefer
$(firstword).








>
> ---
>
> diff --git a/Makefile b/Makefile
> index 6f846b1f2618..1ab3d6f098fb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,7 +94,7 @@ endif
>  # If the user is running make -s (silent mode), suppress echoing of
>  # commands
>
> -ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
> +ifneq ($(findstring s,$(word 1,-$(MAKEFLAGS))),)
>    quiet=silent_
>    KBUILD_VERBOSE = 0
>  endif
>
>
>
> regards, Dmitry



--
Best Regards
Masahiro Yamada

Re: Port silent mode detection to future gnu make.

[Dmitry Goncharov]

On Sun, Nov 27, 2022 at 1:02 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> Please come back after your patch is applied
> (and please have the patch include the commit hash
> causing the behaviour change)

Will do

> BTW, the GNU Make manual suggests $(word 1) instead of $(firstword).
> Is it for the purpose of backward compatibility for older
> Make versions?

Paul chose $(word 1) for this example. i am not sure if there was any
specific reason. $(firstword) would do the same.


> The kernel build only supports Make>=3.82, and I personally prefer
> $(firstword).

3.82 had firstword. Will replace $(word 1) with $(firstword).

regards, Dmitry

Re: Port silent mode detection to future gnu make.

[Dmitry Goncharov]

Good morning.

On Sun, Nov 27, 2022 at 1:02 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> Please come back after your patch is applied
> (and please have the patch include the commit hash
> causing the behaviour change)



The change is available in git in sha dc2d963989b96161472b2cd38cef5d1f4851ea34.

regards, Dmitry

diff --git a/Makefile b/Makefile
index 6f846b1f2618..c5d5558e9806 100644
--- a/Makefile
+++ b/Makefile
@@ -94,7 +94,7 @@ endif
 # If the user is running make -s (silent mode), suppress echoing of
 # commands

-ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
+ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),)
   quiet=silent_
   KBUILD_VERBOSE = 0
 endif

Re: Port silent mode detection to future gnu make.

[Masahiro Yamada]

On Tue, Nov 29, 2022 at 12:00 PM Dmitry Goncharov
<dgoncharov@users.sf.net> wrote:
>
> Good morning.
>
> On Sun, Nov 27, 2022 at 1:02 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > Please come back after your patch is applied
> > (and please have the patch include the commit hash
> > causing the behaviour change)
>
>
>
> The change is available in git in sha dc2d963989b96161472b2cd38cef5d1f4851ea34.
>
> regards, Dmitry
>
> diff --git a/Makefile b/Makefile
> index 6f846b1f2618..c5d5558e9806 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -94,7 +94,7 @@ endif
>  # If the user is running make -s (silent mode), suppress echoing of
>  # commands
>
> -ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
> +ifneq ($(findstring s,$(firstword -$(MAKEFLAGS))),)
>    quiet=silent_
>    KBUILD_VERBOSE = 0
>  endif



Yup, I saw that.



Paul said with GNU Make 4.4,
Kbuild is already having an issue in the -s detection.


$ export MAKEFLAGS=-I/usr/local/mk

 or

$ export MAKEFLAGS=-Orecurse



Commit 77881d228103 ("Ensure that MAKEFLAGS is set when invoking $(shell ...)")
is the commit that caused a change.


Please send v2 with $(firstword) and updated commit log.



Also, add this tag:

Link: https://lists.gnu.org/archive/html/bug-make/2022-11/msg00190.html



-- 
Best Regards
Masahiro Yamada

Re: Port silent mode detection to future gnu make.

[Dmitry Goncharov]

On Tue, Nov 29, 2022 at 1:22 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> Kbuild is already having an issue in the -s detection.
> $ export MAKEFLAGS=-I/usr/local/mk
>  or
> $ export MAKEFLAGS=-Orecurse

i am not concerned about these use cases. This code in question fails
to handle the use case of MAKEFALGS in env and apparently this does
not bother anyone, otherwise this code would already be fixed.  My
concern is that the change, that we introduced to make, won't cause a
regression to those of us who specify -s, once the users migrate to
the new make.
However, the fix will help the MAKEFLAGS-in-env use case, as well.


> Commit 77881d228103 ("Ensure that MAKEFLAGS is set when invoking $(shell ...)")
> is the commit that caused a change.

The whole sequence of events is the following

1. commit 98da874c43035a490cdca81331724f233a3d0c9a [SV 10593] Export
variables to $(shell ...) commands
This allowed make variables to be exported to $(shell) at parse time.

2. Then a user opened https://savannah.gnu.org/bugs/?63347 and
(correctly) argued that the behavior is inconsistent. The
inconsistency was caused by MAKEFLAGS lacking command line variable
definitions until build time.

3.  commit dc2d963989b96161472b2cd38cef5d1f4851ea34 [SV 63347] Always
add command line variable assignments to MAKEFLAGS
This adds command line variable definitions to MAKEFLAGS at parse time.




> Please send v2 with $(firstword) and updated commit log.
> Also, add this tag:
> Link: https://lists.gnu.org/archive/html/bug-make/2022-11/msg00190.html

Done.

regards, Dmitry