| Message ID | 20181010222333.17794-1-bp@alien8.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: Fail the build early when no lz4 present | expand |
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,
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 >
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.
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.
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.
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
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.
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)