linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: Fail the build early when no lz4 present
@ 2018-10-10 22:23 Borislav Petkov
  2018-10-10 22:34 ` Randy Dunlap
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Borislav Petkov @ 2018-10-10 22:23 UTC (permalink / raw)
  To: LKML; +Cc: Masahiro Yamada, Michal Marek, linux-kbuild

From: Borislav Petkov <bp@suse.de>

When building randconfigs, the build fails at kernel compression stage
due to missing lz4 on the system but CONFIG_KERNEL_LZ4 has been selected
by randconfig. The result looks somethins like this:

    (cat arch/x86/boot/compressed/vmlinux.bin arch/x86/boot/compressed/vmlinux.relocs | lz4c -l -c1 stdin stdout && printf \334\141\301\001) > arch/x86/boot/compressed/vmlinux.bin.lz4 || (rm -f arch/x86/boot/compressed/vmlinux.bin.lz4 ; false)
  /bin/sh: 1: lz4c: not found
  make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
  make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
  make: *** [arch/x86/Makefile:290: bzImage] Error 2

Fail the build much earlier by checking for lz4c presence before doing
anything else.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: linux-kbuild@vger.kernel.org
---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index a0c32de80482..f91de649234b 100644
--- a/Makefile
+++ b/Makefile
@@ -788,6 +788,12 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
 LDFLAGS_vmlinux += --gc-sections
 endif
 
+ifdef CONFIG_KERNEL_LZ4
+ifeq ($(CONFIG_SHELL which lz4c),)
+$(error "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled")
+endif
+endif
+
 # arch Makefile may override CC so keep this after arch Makefile is included
 NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 
-- 
2.19.0.271.gfe8321ec057f


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-10 22:23 [PATCH] kbuild: Fail the build early when no lz4 present Borislav Petkov
@ 2018-10-10 22:34 ` Randy Dunlap
  2018-10-11  4:26 ` Masahiro Yamada
  2018-10-11 20:33 ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2018-10-10 22:34 UTC (permalink / raw)
  To: Borislav Petkov, LKML; +Cc: Masahiro Yamada, Michal Marek, linux-kbuild

On 10/10/18 3:23 PM, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> When building randconfigs, the build fails at kernel compression stage
> due to missing lz4 on the system but CONFIG_KERNEL_LZ4 has been selected
> by randconfig. The result looks somethins like this:
> 
>     (cat arch/x86/boot/compressed/vmlinux.bin arch/x86/boot/compressed/vmlinux.relocs | lz4c -l -c1 stdin stdout && printf \334\141\301\001) > arch/x86/boot/compressed/vmlinux.bin.lz4 || (rm -f arch/x86/boot/compressed/vmlinux.bin.lz4 ; false)
>   /bin/sh: 1: lz4c: not found
>   make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
>   make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
>   make: *** [arch/x86/Makefile:290: bzImage] Error 2
> 
> Fail the build much earlier by checking for lz4c presence before doing
> anything else.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: linux-kbuild@vger.kernel.org
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index a0c32de80482..f91de649234b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -788,6 +788,12 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
>  LDFLAGS_vmlinux += --gc-sections
>  endif
>  
> +ifdef CONFIG_KERNEL_LZ4
> +ifeq ($(CONFIG_SHELL which lz4c),)

Hi,
I have been informed in the past the POSIX spells "which" as "command -v" and
hence requested to do s/which/command -v/

> +$(error "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled")
> +endif
> +endif
> +
>  # arch Makefile may override CC so keep this after arch Makefile is included
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>  
> 

cheers,
-- 
~Randy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-10 22:23 [PATCH] kbuild: Fail the build early when no lz4 present Borislav Petkov
  2018-10-10 22:34 ` Randy Dunlap
@ 2018-10-11  4:26 ` Masahiro Yamada
  2018-10-11  8:52   ` Borislav Petkov
  2018-10-11 20:33 ` kbuild test robot
  2 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2018-10-11  4:26 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linux Kernel Mailing List, Michal Marek, Linux Kbuild mailing list

Hi Borislav,

On Thu, Oct 11, 2018 at 7:23 AM Borislav Petkov <bp@alien8.de> wrote:
>
> From: Borislav Petkov <bp@suse.de>
>
> When building randconfigs, the build fails at kernel compression stage
> due to missing lz4 on the system but CONFIG_KERNEL_LZ4 has been selected
> by randconfig. The result looks somethins like this:
>
>     (cat arch/x86/boot/compressed/vmlinux.bin arch/x86/boot/compressed/vmlinux.relocs | lz4c -l -c1 stdin stdout && printf \334\141\301\001) > arch/x86/boot/compressed/vmlinux.bin.lz4 || (rm -f arch/x86/boot/compressed/vmlinux.bin.lz4 ; false)
>   /bin/sh: 1: lz4c: not found


So, the cause of the failure is clear enough
from the build log.



It is weird to check only lz4c.
If CONFIG_KERNEL_LZO is enabled, but lzop is not installed,
I see this log

  LZO     arch/x86/boot/compressed/vmlinux.bin.lzo
/bin/sh: 1: lzop: not found

It is still clear what to do, though.




>   make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
>   make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
>   make: *** [arch/x86/Makefile:290: bzImage] Error 2
>
> Fail the build much earlier by checking for lz4c presence before doing
> anything else.


Is it necessary to check this earlier?

If you get this error, you just need to install the tool.
Then, you can re-run the incremental build.


BTW, this patch has a drawback.

[1] Enable CONFIG_KERNEL_LZ4 on the system
    without lz4c installed

[2] Run 'make' and you will get the error
    "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled"

[3] Run 'make menuconfig' and
    switch from CONFIG_KERNEL_LZ4 to CONFIG_KERNEL_GZIP

[4] Run 'make' and you will still get the same error
    even after you have chosen to use GZIP instead of LZ4.




> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: linux-kbuild@vger.kernel.org
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index a0c32de80482..f91de649234b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -788,6 +788,12 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
>  LDFLAGS_vmlinux += --gc-sections
>  endif
>
> +ifdef CONFIG_KERNEL_LZ4
> +ifeq ($(CONFIG_SHELL which lz4c),)
> +$(error "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled")
> +endif
> +endif
> +
>  # arch Makefile may override CC so keep this after arch Makefile is included
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>
> --
> 2.19.0.271.gfe8321ec057f
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-11  4:26 ` Masahiro Yamada
@ 2018-10-11  8:52   ` Borislav Petkov
  2018-10-11 15:08     ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2018-10-11  8:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kernel Mailing List, Michal Marek, Linux Kbuild mailing list

On Thu, Oct 11, 2018 at 01:26:23PM +0900, Masahiro Yamada wrote:
> So, the cause of the failure is clear enough
> from the build log.

Sure but it wasts unnecessary build cycles and appears only after all
the compilation units have been produced.

> It is weird to check only lz4c.
> If CONFIG_KERNEL_LZO is enabled, but lzop is not installed,
> I see this log
> 
>   LZO     arch/x86/boot/compressed/vmlinux.bin.lzo
> /bin/sh: 1: lzop: not found
> 
> It is still clear what to do, though.

Right, we should test for that too. Unless distros start installing it
by default. It is not installed by default on a debian guest here, at
least.

SLES15 either:

$ lzop
If 'lzop' is not a typo you can use command-not-found to lookup the package that contains it, like this:
    cnf lzop

> Is it necessary to check this earlier?

Yes, see above.

> If you get this error, you just need to install the tool.
> Then, you can re-run the incremental build.

Actually, I'd prefer the build to fail early so that machine can
continue with the next build.

> BTW, this patch has a drawback.
> 
> [1] Enable CONFIG_KERNEL_LZ4 on the system
>     without lz4c installed
> 
> [2] Run 'make' and you will get the error
>     "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled"
> 
> [3] Run 'make menuconfig' and
>     switch from CONFIG_KERNEL_LZ4 to CONFIG_KERNEL_GZIP
> 
> [4] Run 'make' and you will still get the same error
>     even after you have chosen to use GZIP instead of LZ4.

Well, there's code in Makefile to refresh include/config/auto.conf but
for whatever reason that make in step 4 doesn't run it.

And *that* looks like a bug - if a user runs "make menuconfig" and
.config gets updated, then include/config/auto.conf better get updated
too. Right?

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-11  8:52   ` Borislav Petkov
@ 2018-10-11 15:08     ` Masahiro Yamada
  2018-10-11 15:32       ` Borislav Petkov
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2018-10-11 15:08 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linux Kernel Mailing List, Michal Marek, Linux Kbuild mailing list

On Thu, Oct 11, 2018 at 5:54 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Oct 11, 2018 at 01:26:23PM +0900, Masahiro Yamada wrote:
> > So, the cause of the failure is clear enough
> > from the build log.
>
> Sure but it wasts unnecessary build cycles and appears only after all
> the compilation units have been produced.


Not a waste of time.

Install lz4c, and run 'make' again.
Almost all objects have been built there,
so you will finish it soon.

If you are building up a compile-test machine,
you will probably need to install various tools anyway.

I do not want to add ugly checks in random places.


> > It is weird to check only lz4c.
> > If CONFIG_KERNEL_LZO is enabled, but lzop is not installed,
> > I see this log
> >
> >   LZO     arch/x86/boot/compressed/vmlinux.bin.lzo
> > /bin/sh: 1: lzop: not found
> >
> > It is still clear what to do, though.
>
> Right, we should test for that too. Unless distros start installing it
> by default. It is not installed by default on a debian guest here, at
> least.
>
> SLES15 either:
>
> $ lzop
> If 'lzop' is not a typo you can use command-not-found to lookup the package that contains it, like this:
>     cnf lzop
>
> > Is it necessary to check this earlier?
>
> Yes, see above.
>
> > If you get this error, you just need to install the tool.
> > Then, you can re-run the incremental build.
>
> Actually, I'd prefer the build to fail early so that machine can
> continue with the next build.
>
> > BTW, this patch has a drawback.
> >
> > [1] Enable CONFIG_KERNEL_LZ4 on the system
> >     without lz4c installed
> >
> > [2] Run 'make' and you will get the error
> >     "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled"
> >
> > [3] Run 'make menuconfig' and
> >     switch from CONFIG_KERNEL_LZ4 to CONFIG_KERNEL_GZIP
> >
> > [4] Run 'make' and you will still get the same error
> >     even after you have chosen to use GZIP instead of LZ4.
>
> Well, there's code in Makefile to refresh include/config/auto.conf but
> for whatever reason that make in step 4 doesn't run it.
>
> And *that* looks like a bug - if a user runs "make menuconfig" and
> .config gets updated, then include/config/auto.conf better get updated
> too. Right?

Not a bug.
include/config/auto.conf gets updates.

But the top Makefile is parsed twice
with a stale auto.conf, and with a fresh auto.conf

It is wrong to add $(error ...)
in the plain context in the top Makefile.



> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-11 15:08     ` Masahiro Yamada
@ 2018-10-11 15:32       ` Borislav Petkov
  2018-10-16  9:17         ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2018-10-11 15:32 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kernel Mailing List, Michal Marek, Linux Kbuild mailing list

On Fri, Oct 12, 2018 at 12:08:07AM +0900, Masahiro Yamada wrote:
> Install lz4c, and run 'make' again.
> Almost all objects have been built there,
> so you will finish it soon.

So when this LZ4 thing got added at the time, the lz4 package had to be
downloaded and built and installed and it wasn't as easy as zypper in
lz4 or apt-get install lz4.

And I *might* install it if there were a human readable error message
which would tell me so.

And we should strive to be as user-friendly as possible. And no, this:

/bin/sh: 1: lz4c: not found
make[2]: *** [arch/x86/boot/compressed/Makefile:145: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
make: *** [arch/x86/Makefile:290: bzImage] Error 2
make: *** Waiting for unfinished jobs....

is not user-friendly.

> If you are building up a compile-test machine,
> you will probably need to install various tools anyway.

Yes, and look how perf tool solves this. Much much better.

> I do not want to add ugly checks in random places.

That's fair.

What would be a fitting place to add such checks and be able to issue a
human-readable error message to people?

Btw, we fail the same cryptic way when there's no openSSL headers
installed on the system:

---
scripts/sign-file.c:25:10: fatal error: openssl/opensslv.h: No such file or directory
 #include <openssl/opensslv.h>
          ^~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.host:90: scripts/sign-file] Error 1
make[1]: *** Waiting for unfinished jobs....
scripts/extract-cert.c:21:10: fatal error: openssl/bio.h: No such file or directory
 #include <openssl/bio.h>
          ^~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.host:90: scripts/extract-cert] Error 1
make: *** [Makefile:1065: scripts] Error 2
make: *** Waiting for unfinished jobs....
---

and all those beginners who are trying to build the kernel for the first
time would have hard time figuring out what's expected of them.

Now look at perf tool:

Makefile.config:445: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-dev
Makefile.config:491: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
Makefile.config:721: No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling

Much better IMO.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-10 22:23 [PATCH] kbuild: Fail the build early when no lz4 present Borislav Petkov
  2018-10-10 22:34 ` Randy Dunlap
  2018-10-11  4:26 ` Masahiro Yamada
@ 2018-10-11 20:33 ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2018-10-11 20:33 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: kbuild-all, LKML, Masahiro Yamada, Michal Marek, linux-kbuild

[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]

Hi Borislav,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc7 next-20181011]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Borislav-Petkov/kbuild-Fail-the-build-early-when-no-lz4-present/20181012-013752
config: i386-randconfig-b0-10112221 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> Makefile:793: *** "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled".  Stop.
   make: *** [sub-make] Error 2

vim +793 Makefile

   790	
   791	ifdef CONFIG_KERNEL_LZ4
   792	ifeq ($(CONFIG_SHELL which lz4c),)
 > 793	$(error "lz4 tool not found on this system but CONFIG_KERNEL_LZ4 enabled")
   794	endif
   795	endif
   796	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31850 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kbuild: Fail the build early when no lz4 present
  2018-10-11 15:32       ` Borislav Petkov
@ 2018-10-16  9:17         ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2018-10-16  9:17 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Linux Kernel Mailing List, Michal Marek, Linux Kbuild mailing list

On Fri, Oct 12, 2018 at 12:33 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Oct 12, 2018 at 12:08:07AM +0900, Masahiro Yamada wrote:
> > Install lz4c, and run 'make' again.
> > Almost all objects have been built there,
> > so you will finish it soon.
>
> So when this LZ4 thing got added at the time, the lz4 package had to be
> downloaded and built and installed and it wasn't as easy as zypper in
> lz4 or apt-get install lz4.
>
> And I *might* install it if there were a human readable error message
> which would tell me so.
>
> And we should strive to be as user-friendly as possible. And no, this:
>
> /bin/sh: 1: lz4c: not found
> make[2]: *** [arch/x86/boot/compressed/Makefile:145: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
> make: *** [arch/x86/Makefile:290: bzImage] Error 2
> make: *** Waiting for unfinished jobs....
>
> is not user-friendly.
>
> > If you are building up a compile-test machine,
> > you will probably need to install various tools anyway.
>
> Yes, and look how perf tool solves this. Much much better.
>
> > I do not want to add ugly checks in random places.
>
> That's fair.
>
> What would be a fitting place to add such checks and be able to issue a
> human-readable error message to people?

Sorry for delay.

I have been thinking of this
because I have a motivation to
move libelf check in the top Makefile to somewhere else.

I just made a trial.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-16  9:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-10 22:23 [PATCH] kbuild: Fail the build early when no lz4 present Borislav Petkov
2018-10-10 22:34 ` Randy Dunlap
2018-10-11  4:26 ` Masahiro Yamada
2018-10-11  8:52   ` Borislav Petkov
2018-10-11 15:08     ` Masahiro Yamada
2018-10-11 15:32       ` Borislav Petkov
2018-10-16  9:17         ` Masahiro Yamada
2018-10-11 20:33 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).