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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK7LNARsA8P=OmRzMqc9Xa3TWw=zk+Fi7WUGENdOCYaF6fYMrw@mail.gmail.com>
Date:   Wed, 13 Oct 2021 16:00:46 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Vegard Nossum <vegard.nossum@...cle.com>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [RFC PATCH] kbuild: only prompt for compressors that are actually usable

On Wed, Oct 13, 2021 at 2:39 PM Vegard Nossum <vegard.nossum@...cle.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@...cle.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@...cle.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