[PATCH] kbuild: Fix output of make kernelrelease

[PATCH] kbuild: Fix output of make kernelrelease

[Steven Rostedt]

Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
more simply" changed the output of "make kernelrelease" such that the
kernel release version was not the last line printed. This broke various
tools that would find the kernel release with "make kernelrelease | tail -1".
One of those tools that broke was ktest.pl which resides in the kernel.

If the target of the make is "kernelrelease" do not print the
"Leaving directory ..." message at the end as that will break tools that
expect the kernelrelease version to be the last line outputted.

Fixes: 7ff525712acf "kbuild: fake the "Entering directory ..." message more simply"
Cc: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Peter Foley <pefoley2@pefoley.com>
Cc: Michal Marek <mmarek@suse.cz>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Makefile b/Makefile
index 05d67af376c5..83fc5f0398a0 100644
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,14 @@ NAME = Shuffling Zombie Juror
 # (this increases performance and avoids hard-to-debug behaviour);
 MAKEFLAGS += -rR
 
+#
+# The target "kernelrelease" requires the last line to be the release
+# of the kernel, not a "Leaving directory ..." message.
+#   ktest.pl and other tools require this.
+ifeq ("$(MAKECMDGOALS)", "kernelrelease")
+MAKEFLAGS += --no-print-directory
+endif
+
 # Avoid funny character set dependencies
 unexport LC_ALL
 LC_COLLATE=C
-- 
2.0.1

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Michal Marek]

Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
> 
> Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
> more simply" changed the output of "make kernelrelease" such that the
> kernel release version was not the last line printed. This broke various
> tools that would find the kernel release with "make kernelrelease | tail -1".

The cleaner and recommended (see recent make help) way is to use make -s:

$ make O=build -s kernelrelease
3.18.0-rc1+

no further processing is needed.


> One of those tools that broke was ktest.pl which resides in the kernel.

Can you please apply this patch?

Thanks,
Michal

From c660b235e25eee053337e0e6c952e87f39839c63 Mon Sep 17 00:00:00 2001
From: Michal Marek <mmarek@suse.cz>
Date: Wed, 22 Oct 2014 21:25:39 +0200
Subject: [PATCH] ktest: Use make -s kernelrelease

The previous tail -1 broke with commit 7ff525712acf ("kbuild: fake the
"Entering directory ..." message more simply")

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 tools/testing/ktest/ktest.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index bf13981..60fe020 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -2005,7 +2005,7 @@ sub get_version {
     # get the release name
     return if ($have_version);
     doprint "$make kernelrelease ... ";
-    $version = `$make kernelrelease | tail -1`;
+    $version = `$make -s kernelrelease`;
     chomp($version);
     doprint "$version\n";
     $have_version = 1;
-- 
1.9.2

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Steven Rostedt]

On Wed, 22 Oct 2014 21:44:08 +0200
Michal Marek <mmarek@suse.cz> wrote:

> Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
> > 
> > Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
> > more simply" changed the output of "make kernelrelease" such that the
> > kernel release version was not the last line printed. This broke various
> > tools that would find the kernel release with "make kernelrelease | tail -1".
> 
> The cleaner and recommended (see recent make help) way is to use make -s:
> 
> $ make O=build -s kernelrelease
> 3.18.0-rc1+

Hmm, I see the help index was recently updated to include that.

Does this work with older kernels too? That's very important.

-- Steve

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Steven Rostedt]

On Wed, 22 Oct 2014 21:44:08 +0200
Michal Marek <mmarek@suse.cz> wrote:

> Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
> > 
> > Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
> > more simply" changed the output of "make kernelrelease" such that the
> > kernel release version was not the last line printed. This broke various
> > tools that would find the kernel release with "make kernelrelease | tail -1".
> 
> The cleaner and recommended (see recent make help) way is to use make -s:
> 
> $ make O=build -s kernelrelease
> 3.18.0-rc1+
> 
> no further processing is needed.


I don't mind changing my script with your patch. But this does break
other scripts of mine that I need to hunt down and change. My fear is
why do we need the '-s' when there may be other methods to preserve the
old functionality.

If this is breaking my scripts, I wonder if it is also breaking other
scripts out there too? As you stated, it wasn't until recently that the
help message recommended the '-s'.

-- Steve


> 
> 
> > One of those tools that broke was ktest.pl which resides in the kernel.
> 
> Can you please apply this patch?
> 
> Thanks,
> Michal
> 
> >From c660b235e25eee053337e0e6c952e87f39839c63 Mon Sep 17 00:00:00 2001
> From: Michal Marek <mmarek@suse.cz>
> Date: Wed, 22 Oct 2014 21:25:39 +0200
> Subject: [PATCH] ktest: Use make -s kernelrelease
> 
> The previous tail -1 broke with commit 7ff525712acf ("kbuild: fake the
> "Entering directory ..." message more simply")
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Michal Marek <mmarek@suse.cz>
> ---
>  tools/testing/ktest/ktest.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
> index bf13981..60fe020 100755
> --- a/tools/testing/ktest/ktest.pl
> +++ b/tools/testing/ktest/ktest.pl
> @@ -2005,7 +2005,7 @@ sub get_version {
>      # get the release name
>      return if ($have_version);
>      doprint "$make kernelrelease ... ";
> -    $version = `$make kernelrelease | tail -1`;
> +    $version = `$make -s kernelrelease`;
>      chomp($version);
>      doprint "$version\n";
>      $have_version = 1;

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Michal Marek]

On 2014-10-22 22:35, Steven Rostedt wrote:
> On Wed, 22 Oct 2014 21:44:08 +0200
> Michal Marek <mmarek@suse.cz> wrote:
> 
>> Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
>>>
>>> Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
>>> more simply" changed the output of "make kernelrelease" such that the
>>> kernel release version was not the last line printed. This broke various
>>> tools that would find the kernel release with "make kernelrelease | tail -1".
>>
>> The cleaner and recommended (see recent make help) way is to use make -s:
>>
>> $ make O=build -s kernelrelease
>> 3.18.0-rc1+
> 
> Hmm, I see the help index was recently updated to include that.
> 
> Does this work with older kernels too? That's very important.

Yes. Except for a few bugs in the history, make -s has always been silent.

Michal

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Michal Marek]

On 2014-10-23 03:11, Steven Rostedt wrote:
> On Wed, 22 Oct 2014 21:44:08 +0200
> Michal Marek <mmarek@suse.cz> wrote:
> 
>> Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
>>>
>>> Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
>>> more simply" changed the output of "make kernelrelease" such that the
>>> kernel release version was not the last line printed. This broke various
>>> tools that would find the kernel release with "make kernelrelease | tail -1".
>>
>> The cleaner and recommended (see recent make help) way is to use make -s:
>>
>> $ make O=build -s kernelrelease
>> 3.18.0-rc1+
>>
>> no further processing is needed.
> 
> 
> I don't mind changing my script with your patch. But this does break
> other scripts of mine that I need to hunt down and change. My fear is
> why do we need the '-s' when there may be other methods to preserve the
> old functionality.

If you have scripts that only do x=$(make kernelrelease), then you
should definitely fix them. You are right that the make kernelrelease |
tail -1 trick had always been working until 7ff525712acf, so I can apply
your patch. But please fix ktest.pl so that it's consistent with what
the help text says. Also, the Makefile patch should cover kernelversion
and image_name as well.

Michal

Re: [PATCH] kbuild: Fix output of make kernelrelease

[Steven Rostedt]

On Thu, 23 Oct 2014 10:57:57 +0200
Michal Marek <mmarek@suse.cz> wrote:

> On 2014-10-23 03:11, Steven Rostedt wrote:
> > On Wed, 22 Oct 2014 21:44:08 +0200
> > Michal Marek <mmarek@suse.cz> wrote:
> > 
> >> Dne 22.10.2014 v 16:19 Steven Rostedt napsal(a):
> >>>
> >>> Commit 7ff525712acf "kbuild: fake the "Entering directory ..." message
> >>> more simply" changed the output of "make kernelrelease" such that the
> >>> kernel release version was not the last line printed. This broke various
> >>> tools that would find the kernel release with "make kernelrelease | tail -1".
> >>
> >> The cleaner and recommended (see recent make help) way is to use make -s:
> >>
> >> $ make O=build -s kernelrelease
> >> 3.18.0-rc1+
> >>
> >> no further processing is needed.
> > 
> > 
> > I don't mind changing my script with your patch. But this does break
> > other scripts of mine that I need to hunt down and change. My fear is
> > why do we need the '-s' when there may be other methods to preserve the
> > old functionality.
> 
> If you have scripts that only do x=$(make kernelrelease), then you
> should definitely fix them. You are right that the make kernelrelease |
> tail -1 trick had always been working until 7ff525712acf, so I can apply
> your patch.

Thanks, because that's what my scripts indeed do ;-)

> But please fix ktest.pl so that it's consistent with what
> the help text says.

Yeah, regardless I was planning on applying your patch as it needs to
handle bisects, and without it, ktest will fail to bisect after
7ff525712acf.

> Also, the Makefile patch should cover kernelversion
> and image_name as well.

OK, I'll make the update.

Thanks!

-- Steve