[RFC PATCH] kbuild: only prompt for compressors that are actually usable

[RFC PATCH] kbuild: only prompt for compressors that are actually usable

[Vegard Nossum]

If a given compression algorithm for the kernel image is not usable on
the host system, there is no point prompting for it.

We can use the kconfig preprocessing feature to check if the command is
available or not. I've chosen to test this using "which", which exits
with success if the given command exists in PATH (or it is an absolute
path), which mimics exactly how it would be found in the kernel's
Makefiles.

This uses the make variables that are set in Makefile and/or the
command line, so you can do e.g.

  make KGZIP=pigz menuconfig

and it will test for the correct program.

I am intentionally adding these dependencies to e.g. KERNEL_LZ4, as
opposed to HAVE_KERNEL_LZ4, since the latter are "select"-ed
unconditionally by the architectures that use them, so they are not
suitable for depending on anything else.

I've put RFC in the subject as maybe there are downsides to this that
I'm not aware of.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 init/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 11f8a845f259d..f03f2b7962027 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -250,6 +250,7 @@ choice
 config KERNEL_GZIP
 	bool "Gzip"
 	depends on HAVE_KERNEL_GZIP
+	depends on $(success,which $(KGZIP))
 	help
 	  The old and tried gzip compression. It provides a good balance
 	  between compression ratio and decompression speed.
@@ -257,6 +258,7 @@ config KERNEL_GZIP
 config KERNEL_BZIP2
 	bool "Bzip2"
 	depends on HAVE_KERNEL_BZIP2
+	depends on $(success,which $(KBZIP2))
 	help
 	  Its compression ratio and speed is intermediate.
 	  Decompression speed is slowest among the choices.  The kernel
@@ -267,6 +269,7 @@ config KERNEL_BZIP2
 config KERNEL_LZMA
 	bool "LZMA"
 	depends on HAVE_KERNEL_LZMA
+	depends on $(success,which $(LZMA))
 	help
 	  This compression algorithm's ratio is best.  Decompression speed
 	  is between gzip and bzip2.  Compression is slowest.
@@ -275,6 +278,7 @@ config KERNEL_LZMA
 config KERNEL_XZ
 	bool "XZ"
 	depends on HAVE_KERNEL_XZ
+	depends on $(success,which $(XZ))
 	help
 	  XZ uses the LZMA2 algorithm and instruction set specific
 	  BCJ filters which can improve compression ratio of executable
@@ -290,6 +294,7 @@ config KERNEL_XZ
 config KERNEL_LZO
 	bool "LZO"
 	depends on HAVE_KERNEL_LZO
+	depends on $(success,which $(KLZOP))
 	help
 	  Its compression ratio is the poorest among the choices. The kernel
 	  size is about 10% bigger than gzip; however its speed
@@ -298,6 +303,7 @@ config KERNEL_LZO
 config KERNEL_LZ4
 	bool "LZ4"
 	depends on HAVE_KERNEL_LZ4
+	depends on $(success,which $(LZ4))
 	help
 	  LZ4 is an LZ77-type compressor with a fixed, byte-oriented encoding.
 	  A preliminary version of LZ4 de/compression tool is available at
@@ -310,6 +316,7 @@ config KERNEL_LZ4
 config KERNEL_ZSTD
 	bool "ZSTD"
 	depends on HAVE_KERNEL_ZSTD
+	depends on $(success,which $(ZSTD))
 	help
 	  ZSTD is a compression algorithm targeting intermediate compression
 	  with fast decompression speed. It will compress better than GZIP and
-- 
2.23.0.718.g5ad94255a8

Re: [RFC PATCH] kbuild: only prompt for compressors that are actually usable

[Randy Dunlap]

On 10/12/21 10:01 AM, Vegard Nossum wrote:
> If a given compression algorithm for the kernel image is not usable on
> the host system, there is no point prompting for it.
> 
> We can use the kconfig preprocessing feature to check if the command is
> available or not. I've chosen to test this using "which", which exits
> with success if the given command exists in PATH (or it is an absolute
> path), which mimics exactly how it would be found in the kernel's
> Makefiles.

Hi Vegard,

I have made a few patches that used "which", but I was always told
that the POSIX spelling of that command is "command -v", so that is
preferable.

> This uses the make variables that are set in Makefile and/or the
> command line, so you can do e.g.
> 
>    make KGZIP=pigz menuconfig
> 
> and it will test for the correct program.
> 
> I am intentionally adding these dependencies to e.g. KERNEL_LZ4, as
> opposed to HAVE_KERNEL_LZ4, since the latter are "select"-ed
> unconditionally by the architectures that use them, so they are not
> suitable for depending on anything else.
> 
> I've put RFC in the subject as maybe there are downsides to this that
> I'm not aware of.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>   init/Kconfig | 7 +++++++
>   1 file changed, 7 insertions(+)


-- 
~Randy

Re: [RFC PATCH] kbuild: only prompt for compressors that are actually usable

[Masahiro Yamada]

On Wed, Oct 13, 2021 at 2:01 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
> If a given compression algorithm for the kernel image is not usable on
> the host system, there is no point prompting for it.
>
> We can use the kconfig preprocessing feature to check if the command is
> available or not. I've chosen to test this using "which", which exits
> with success if the given command exists in PATH (or it is an absolute
> path), which mimics exactly how it would be found in the kernel's
> Makefiles.
>
> This uses the make variables that are set in Makefile and/or the
> command line, so you can do e.g.
>
>   make KGZIP=pigz menuconfig
>
> and it will test for the correct program.
>
> I am intentionally adding these dependencies to e.g. KERNEL_LZ4, as
> opposed to HAVE_KERNEL_LZ4, since the latter are "select"-ed
> unconditionally by the architectures that use them, so they are not
> suitable for depending on anything else.
>
> I've put RFC in the subject as maybe there are downsides to this that
> I'm not aware of.
>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---


I think we should keep the host-tools dependency open in general.
You can easily install necessary packages.








>  init/Kconfig | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 11f8a845f259d..f03f2b7962027 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -250,6 +250,7 @@ choice
>  config KERNEL_GZIP
>         bool "Gzip"
>         depends on HAVE_KERNEL_GZIP
> +       depends on $(success,which $(KGZIP))
>         help
>           The old and tried gzip compression. It provides a good balance
>           between compression ratio and decompression speed.
> @@ -257,6 +258,7 @@ config KERNEL_GZIP
>  config KERNEL_BZIP2
>         bool "Bzip2"
>         depends on HAVE_KERNEL_BZIP2
> +       depends on $(success,which $(KBZIP2))
>         help
>           Its compression ratio and speed is intermediate.
>           Decompression speed is slowest among the choices.  The kernel
> @@ -267,6 +269,7 @@ config KERNEL_BZIP2
>  config KERNEL_LZMA
>         bool "LZMA"
>         depends on HAVE_KERNEL_LZMA
> +       depends on $(success,which $(LZMA))
>         help
>           This compression algorithm's ratio is best.  Decompression speed
>           is between gzip and bzip2.  Compression is slowest.
> @@ -275,6 +278,7 @@ config KERNEL_LZMA
>  config KERNEL_XZ
>         bool "XZ"
>         depends on HAVE_KERNEL_XZ
> +       depends on $(success,which $(XZ))
>         help
>           XZ uses the LZMA2 algorithm and instruction set specific
>           BCJ filters which can improve compression ratio of executable
> @@ -290,6 +294,7 @@ config KERNEL_XZ
>  config KERNEL_LZO
>         bool "LZO"
>         depends on HAVE_KERNEL_LZO
> +       depends on $(success,which $(KLZOP))
>         help
>           Its compression ratio is the poorest among the choices. The kernel
>           size is about 10% bigger than gzip; however its speed
> @@ -298,6 +303,7 @@ config KERNEL_LZO
>  config KERNEL_LZ4
>         bool "LZ4"
>         depends on HAVE_KERNEL_LZ4
> +       depends on $(success,which $(LZ4))
>         help
>           LZ4 is an LZ77-type compressor with a fixed, byte-oriented encoding.
>           A preliminary version of LZ4 de/compression tool is available at
> @@ -310,6 +316,7 @@ config KERNEL_LZ4
>  config KERNEL_ZSTD
>         bool "ZSTD"
>         depends on HAVE_KERNEL_ZSTD
> +       depends on $(success,which $(ZSTD))
>         help
>           ZSTD is a compression algorithm targeting intermediate compression
>           with fast decompression speed. It will compress better than GZIP and
> --
> 2.23.0.718.g5ad94255a8
>


-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH] kbuild: only prompt for compressors that are actually usable

[Vegard Nossum]

On 10/13/21 3:34 AM, Masahiro Yamada wrote:
> On Wed, Oct 13, 2021 at 2:01 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>>
>> If a given compression algorithm for the kernel image is not usable on
>> the host system, there is no point prompting for it.
>>
>> We can use the kconfig preprocessing feature to check if the command is
>> available or not. I've chosen to test this using "which", which exits
>> with success if the given command exists in PATH (or it is an absolute
>> path), which mimics exactly how it would be found in the kernel's
>> Makefiles.
>>
>> This uses the make variables that are set in Makefile and/or the
>> command line, so you can do e.g.
>>
>>   make KGZIP=pigz menuconfig
>>
>> and it will test for the correct program.
>>
>> I am intentionally adding these dependencies to e.g. KERNEL_LZ4, as
>> opposed to HAVE_KERNEL_LZ4, since the latter are "select"-ed
>> unconditionally by the architectures that use them, so they are not
>> suitable for depending on anything else.
>>
>> I've put RFC in the subject as maybe there are downsides to this that
>> I'm not aware of.
>>
>> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
>> ---
> 
> 
> I think we should keep the host-tools dependency open in general.
> You can easily install necessary packages.

So just to be clear, you object to the patch because it hides the
possibility of using a different compression algorithm from the user and
doesn't give them a chance to know that it exists when it's not already
installed?

I don't really think this is much different from any other choice block
in the config where the visibility of the choices have dependencies on
other config options.

In my case, the reason for doing this patch was that I was seeing build
failures during randomized testing (satrandconfig) due to missing
programs, and these build failures appear only at the end of potentially
very time-consuming builds.

Maybe we can introduce a new option similar to COMPILE_TEST (or
ADVANCED_OPTIONS) so that the options are showed by default, even when
the host program is not available?

+config UNAVAILABLE_COMPRESSORS
+       bool "Prompt for compressors that are not available"
+       help
+         Note: Enabling this option can lead to build failures if
+         the chosen compressor is not available on the host machine.

 config KERNEL_GZIP
        bool "Gzip"
        depends on HAVE_KERNEL_GZIP
+       depends on UNAVAILABLE_COMPRESSORS || $(success,which $(KGZIP))
        help
          The old and tried gzip compression. It provides a good balance
          between compression ratio and decompression speed.


Vegard

Re: [RFC PATCH] kbuild: only prompt for compressors that are actually usable

[Masahiro Yamada]

On Wed, Oct 13, 2021 at 2:39 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
>
> On 10/13/21 3:34 AM, Masahiro Yamada wrote:
> > On Wed, Oct 13, 2021 at 2:01 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> >>
> >> If a given compression algorithm for the kernel image is not usable on
> >> the host system, there is no point prompting for it.
> >>
> >> We can use the kconfig preprocessing feature to check if the command is
> >> available or not. I've chosen to test this using "which", which exits
> >> with success if the given command exists in PATH (or it is an absolute
> >> path), which mimics exactly how it would be found in the kernel's
> >> Makefiles.
> >>
> >> This uses the make variables that are set in Makefile and/or the
> >> command line, so you can do e.g.
> >>
> >>   make KGZIP=pigz menuconfig
> >>
> >> and it will test for the correct program.
> >>
> >> I am intentionally adding these dependencies to e.g. KERNEL_LZ4, as
> >> opposed to HAVE_KERNEL_LZ4, since the latter are "select"-ed
> >> unconditionally by the architectures that use them, so they are not
> >> suitable for depending on anything else.
> >>
> >> I've put RFC in the subject as maybe there are downsides to this that
> >> I'm not aware of.
> >>
> >> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> >> ---
> >
> >
> > I think we should keep the host-tools dependency open in general.
> > You can easily install necessary packages.
>
> So just to be clear, you object to the patch because it hides the
> possibility of using a different compression algorithm from the user and
> doesn't give them a chance to know that it exists when it's not already
> installed?

Yes, I object to it.



> I don't really think this is much different from any other choice block
> in the config where the visibility of the choices have dependencies on
> other config options.

Kconfig checks some compiler features, otherwise we have no way
to avoid build errors.


> In my case, the reason for doing this patch was that I was seeing build
> failures during randomized testing (satrandconfig) due to missing
> programs, and these build failures appear only at the end of potentially
> very time-consuming builds.

I do not know Oracle's case, but
I think other CI systems install all needed packages for randconfig.

Similar discussions happened a couple of times in the past.
We keep the tools dependency open since it is just a matter of
"apt install" or "dnf install" basically.



"and equally importantly, your build servers will actually do a better
job of covering the different build options."  (Linus Torvalds) [1]

[1]: https://lore.kernel.org/all/CAHk-=wjjiYjCp61gdAMpDOsUBU-A2hFFKJoVx5VAC7yV4K6WYg@mail.gmail.com/




> Maybe we can introduce a new option similar to COMPILE_TEST (or
> ADVANCED_OPTIONS) so that the options are showed by default, even when
> the host program is not available?


I do not think it is worth it.




> +config UNAVAILABLE_COMPRESSORS
> +       bool "Prompt for compressors that are not available"
> +       help
> +         Note: Enabling this option can lead to build failures if
> +         the chosen compressor is not available on the host machine.
>
>  config KERNEL_GZIP
>         bool "Gzip"
>         depends on HAVE_KERNEL_GZIP
> +       depends on UNAVAILABLE_COMPRESSORS || $(success,which $(KGZIP))
>         help
>           The old and tried gzip compression. It provides a good balance
>           between compression ratio and decompression speed.
>
>
> Vegard



-- 
Best Regards
Masahiro Yamada