diff mbox series

[PATCHv2,1/2] tools/build: Fix -s detection code in tools/build/Makefile.build

Message ID 20231008212251.236023-2-jolsa@kernel.org (mailing list archive)
State Not Applicable
Headers show
Series tools/build: Fix -s detection code for new make | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jiri Olsa Oct. 8, 2023, 9:22 p.m. UTC
As Dmitry described in [1] changelog the current way of detecting
-s option is broken for new make.

Changing the tools/build -s option detection the same way as it was
fixed for root Makefile in [1].

[1] 4bf73588165b ("kbuild: Port silent mode detection to future gnu make.")

Cc: Dmitry Goncharov <dgoncharov@users.sf.net>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/build/Makefile.build | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Namhyung Kim Oct. 13, 2023, 3:57 a.m. UTC | #1
Hi Jiri,

On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> As Dmitry described in [1] changelog the current way of detecting
> -s option is broken for new make.

I'm not sure what -s option does for perf (at least).
It doesn't seem much different whether I give it or not.
Am I missing something?

Thanks,
Namhyung

>
> Changing the tools/build -s option detection the same way as it was
> fixed for root Makefile in [1].
>
> [1] 4bf73588165b ("kbuild: Port silent mode detection to future gnu make.")
>
> Cc: Dmitry Goncharov <dgoncharov@users.sf.net>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/build/Makefile.build | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> index fac42486a8cf..5fb3fb3d97e0 100644
> --- a/tools/build/Makefile.build
> +++ b/tools/build/Makefile.build
> @@ -20,7 +20,15 @@ else
>    Q=@
>  endif
>
> -ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
> +# If the user is running make -s (silent mode), suppress echoing of commands
> +# make-4.0 (and later) keep single letter options in the 1st word of MAKEFLAGS.
> +ifeq ($(filter 3.%,$(MAKE_VERSION)),)
> +short-opts := $(firstword -$(MAKEFLAGS))
> +else
> +short-opts := $(filter-out --%,$(MAKEFLAGS))
> +endif
> +
> +ifneq ($(findstring s,$(short-opts)),)
>    quiet=silent_
>  endif
>
> --
> 2.41.0
>
Jiri Olsa Oct. 13, 2023, 6:37 a.m. UTC | #2
On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > As Dmitry described in [1] changelog the current way of detecting
> > -s option is broken for new make.
> 
> I'm not sure what -s option does for perf (at least).
> It doesn't seem much different whether I give it or not.
> Am I missing something?

what's your make version? the wrong output is visible when running
with make version > 4.4 .. basicaly the -s is wrongly detected and
you either get no output at all from some builds or overly verbose
output

it's mentioned in the [1] commit changelog, I can put it to the
changelog in new version

jirka

> 
> Thanks,
> Namhyung
> 
> >
> > Changing the tools/build -s option detection the same way as it was
> > fixed for root Makefile in [1].
> >
> > [1] 4bf73588165b ("kbuild: Port silent mode detection to future gnu make.")
> >
> > Cc: Dmitry Goncharov <dgoncharov@users.sf.net>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/build/Makefile.build | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> > index fac42486a8cf..5fb3fb3d97e0 100644
> > --- a/tools/build/Makefile.build
> > +++ b/tools/build/Makefile.build
> > @@ -20,7 +20,15 @@ else
> >    Q=@
> >  endif
> >
> > -ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
> > +# If the user is running make -s (silent mode), suppress echoing of commands
> > +# make-4.0 (and later) keep single letter options in the 1st word of MAKEFLAGS.
> > +ifeq ($(filter 3.%,$(MAKE_VERSION)),)
> > +short-opts := $(firstword -$(MAKEFLAGS))
> > +else
> > +short-opts := $(filter-out --%,$(MAKEFLAGS))
> > +endif
> > +
> > +ifneq ($(findstring s,$(short-opts)),)
> >    quiet=silent_
> >  endif
> >
> > --
> > 2.41.0
> >
Namhyung Kim Oct. 17, 2023, 1:36 a.m. UTC | #3
On Thu, Oct 12, 2023 at 11:37 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> > Hi Jiri,
> >
> > On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > As Dmitry described in [1] changelog the current way of detecting
> > > -s option is broken for new make.
> >
> > I'm not sure what -s option does for perf (at least).
> > It doesn't seem much different whether I give it or not.
> > Am I missing something?
>
> what's your make version? the wrong output is visible when running
> with make version > 4.4 .. basicaly the -s is wrongly detected and
> you either get no output at all from some builds or overly verbose
> output
>
> it's mentioned in the [1] commit changelog, I can put it to the
> changelog in new version

IIUC it's about detecting `make -s` properly and not being confused
by `make a=s` or something.  I'm not objecting on it but I don't see
what `make -s` does actually.

Anyway, my make version is 4.3.

Thanks,
Namhyung
Jiri Olsa Oct. 17, 2023, 8:43 a.m. UTC | #4
On Mon, Oct 16, 2023 at 06:36:10PM -0700, Namhyung Kim wrote:
> On Thu, Oct 12, 2023 at 11:37 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> > > Hi Jiri,
> > >
> > > On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > As Dmitry described in [1] changelog the current way of detecting
> > > > -s option is broken for new make.
> > >
> > > I'm not sure what -s option does for perf (at least).
> > > It doesn't seem much different whether I give it or not.
> > > Am I missing something?
> >
> > what's your make version? the wrong output is visible when running
> > with make version > 4.4 .. basicaly the -s is wrongly detected and
> > you either get no output at all from some builds or overly verbose
> > output
> >
> > it's mentioned in the [1] commit changelog, I can put it to the
> > changelog in new version
> 
> IIUC it's about detecting `make -s` properly and not being confused
> by `make a=s` or something.  I'm not objecting on it but I don't see
> what `make -s` does actually.

so the tools/build/Makefile.build and tools/scripts/Makefile.include detect
make -s option, which puts make into silent mode, so both makefiles switch
off the output by setting quiet=silent_ or silent=1

the problem is that the detection of make -s option changed in make > 4.4
and current code could be tricked to switch to silent mode just by having
's' persent on the command line, like with 'a=s'

jirka

> 
> Anyway, my make version is 4.3.
> 
> Thanks,
> Namhyung
Namhyung Kim Oct. 17, 2023, 8:16 p.m. UTC | #5
On Tue, Oct 17, 2023 at 1:43 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Oct 16, 2023 at 06:36:10PM -0700, Namhyung Kim wrote:
> > On Thu, Oct 12, 2023 at 11:37 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> > > > Hi Jiri,
> > > >
> > > > On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > >
> > > > > As Dmitry described in [1] changelog the current way of detecting
> > > > > -s option is broken for new make.
> > > >
> > > > I'm not sure what -s option does for perf (at least).
> > > > It doesn't seem much different whether I give it or not.
> > > > Am I missing something?
> > >
> > > what's your make version? the wrong output is visible when running
> > > with make version > 4.4 .. basicaly the -s is wrongly detected and
> > > you either get no output at all from some builds or overly verbose
> > > output
> > >
> > > it's mentioned in the [1] commit changelog, I can put it to the
> > > changelog in new version
> >
> > IIUC it's about detecting `make -s` properly and not being confused
> > by `make a=s` or something.  I'm not objecting on it but I don't see
> > what `make -s` does actually.
>
> so the tools/build/Makefile.build and tools/scripts/Makefile.include detect
> make -s option, which puts make into silent mode, so both makefiles switch
> off the output by setting quiet=silent_ or silent=1
>
> the problem is that the detection of make -s option changed in make > 4.4
> and current code could be tricked to switch to silent mode just by having
> 's' persent on the command line, like with 'a=s'

I think our talk is circulating :-).  Anyway I'm ok with the change, so

Acked-by: Namhyung Kim <namhyung@kernel.org>

Which tree do you want to route it?

Thanks,
Namhyung
Jiri Olsa Oct. 18, 2023, 7:21 a.m. UTC | #6
On Tue, Oct 17, 2023 at 01:16:28PM -0700, Namhyung Kim wrote:
> On Tue, Oct 17, 2023 at 1:43 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Oct 16, 2023 at 06:36:10PM -0700, Namhyung Kim wrote:
> > > On Thu, Oct 12, 2023 at 11:37 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> > > > > Hi Jiri,
> > > > >
> > > > > On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > >
> > > > > > As Dmitry described in [1] changelog the current way of detecting
> > > > > > -s option is broken for new make.
> > > > >
> > > > > I'm not sure what -s option does for perf (at least).
> > > > > It doesn't seem much different whether I give it or not.
> > > > > Am I missing something?
> > > >
> > > > what's your make version? the wrong output is visible when running
> > > > with make version > 4.4 .. basicaly the -s is wrongly detected and
> > > > you either get no output at all from some builds or overly verbose
> > > > output
> > > >
> > > > it's mentioned in the [1] commit changelog, I can put it to the
> > > > changelog in new version
> > >
> > > IIUC it's about detecting `make -s` properly and not being confused
> > > by `make a=s` or something.  I'm not objecting on it but I don't see
> > > what `make -s` does actually.
> >
> > so the tools/build/Makefile.build and tools/scripts/Makefile.include detect
> > make -s option, which puts make into silent mode, so both makefiles switch
> > off the output by setting quiet=silent_ or silent=1
> >
> > the problem is that the detection of make -s option changed in make > 4.4
> > and current code could be tricked to switch to silent mode just by having
> > 's' persent on the command line, like with 'a=s'
> 
> I think our talk is circulating :-).  Anyway I'm ok with the change, so

:) ok, thanks

> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Which tree do you want to route it?

I think perf tree is the best one to route it

thanks,
jirka
Namhyung Kim Oct. 18, 2023, 10:29 p.m. UTC | #7
On Wed, Oct 18, 2023 at 12:21 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Oct 17, 2023 at 01:16:28PM -0700, Namhyung Kim wrote:
> > On Tue, Oct 17, 2023 at 1:43 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Mon, Oct 16, 2023 at 06:36:10PM -0700, Namhyung Kim wrote:
> > > > On Thu, Oct 12, 2023 at 11:37 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > >
> > > > > On Thu, Oct 12, 2023 at 08:57:33PM -0700, Namhyung Kim wrote:
> > > > > > Hi Jiri,
> > > > > >
> > > > > > On Sun, Oct 8, 2023 at 2:23 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > > > > > >
> > > > > > > As Dmitry described in [1] changelog the current way of detecting
> > > > > > > -s option is broken for new make.
> > > > > >
> > > > > > I'm not sure what -s option does for perf (at least).
> > > > > > It doesn't seem much different whether I give it or not.
> > > > > > Am I missing something?
> > > > >
> > > > > what's your make version? the wrong output is visible when running
> > > > > with make version > 4.4 .. basicaly the -s is wrongly detected and
> > > > > you either get no output at all from some builds or overly verbose
> > > > > output
> > > > >
> > > > > it's mentioned in the [1] commit changelog, I can put it to the
> > > > > changelog in new version
> > > >
> > > > IIUC it's about detecting `make -s` properly and not being confused
> > > > by `make a=s` or something.  I'm not objecting on it but I don't see
> > > > what `make -s` does actually.
> > >
> > > so the tools/build/Makefile.build and tools/scripts/Makefile.include detect
> > > make -s option, which puts make into silent mode, so both makefiles switch
> > > off the output by setting quiet=silent_ or silent=1
> > >
> > > the problem is that the detection of make -s option changed in make > 4.4
> > > and current code could be tricked to switch to silent mode just by having
> > > 's' persent on the command line, like with 'a=s'
> >
> > I think our talk is circulating :-).  Anyway I'm ok with the change, so
>
> :) ok, thanks
>
> >
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Which tree do you want to route it?
>
> I think perf tree is the best one to route it

Ok, I'll add it.

Thanks,
Namhyung
diff mbox series

Patch

diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
index fac42486a8cf..5fb3fb3d97e0 100644
--- a/tools/build/Makefile.build
+++ b/tools/build/Makefile.build
@@ -20,7 +20,15 @@  else
   Q=@
 endif
 
-ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
+# If the user is running make -s (silent mode), suppress echoing of commands
+# make-4.0 (and later) keep single letter options in the 1st word of MAKEFLAGS.
+ifeq ($(filter 3.%,$(MAKE_VERSION)),)
+short-opts := $(firstword -$(MAKEFLAGS))
+else
+short-opts := $(filter-out --%,$(MAKEFLAGS))
+endif
+
+ifneq ($(findstring s,$(short-opts)),)
   quiet=silent_
 endif