Message-ID: <CAAFQd5AhN5m8eaGsrKfh1gHPLiOVd9_3BwoHpr7u6iY92Ft-bg@mail.gmail.com>
Date:   Wed, 30 Aug 2023 16:30:47 +0900
From:   Tomasz Figa <tfiga@...omium.org>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Ying Sun <sunying@...iscas.ac.cn>,
        Jesse T <mr.bossman075@...il.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        Jonathan Corbet <corbet@....net>, linux-kbuild@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] kconfig: introduce listunknownconfig

On Sat, Aug 26, 2023 at 10:11 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@...omium.org> wrote:
> >
> > Hi Masahiro,
> >
> > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky
> > > <senozhatsky@...omium.org> wrote:
> > > >
> > > > On (23/08/21 21:27), Masahiro Yamada wrote:
> > > > >
> > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both:
> > > > >
> > > > >   - A CONFIG option is hidden by unmet dependency (Ying Sun's case)
> > > > >   - A CONFIG option no longer exists  (your case)
> > > > >   - Anything else we need to be careful
> > > >
> > > > A quick question: is it too late to suggest an alternative name?
> > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically
> > > > run sanity checks on the config.
> > >
> > >
> > > Ying's is not applied yet. So, it is not too late.
> > >
> > > But, I started to be a little worried
> > > because it is unpredictable how many KCONFIG_* env
> > > variables will increase until people are satisfied.
> > >
> >
> > Is there really a problem with having those? There are a lot of
> > different env variables affecting different parts of the kernel build.
> > If they are useful, and even better, allow catching issues quickly,
> > should we favor less options or usefulness for users?
>
>
>
> I am considering how to implement it.
>
>
>
> One way is to add env variables as a new request arises.
>
> Sergey is doing two things by one option.
>
>
>    KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config
> or defconfig
>    KCONFIG_WARN_TO_ERROR       : turn warnings into errors
>
>
>
> Another way is to handle those as command line options.
>
>   -Wunknown-symbol
>   -Werror             (associated with W=e)
>   -Wall               (associated with W=1)
>
>
>
>   $ make W=1e olddefconfig
>
>
> will work to sanity check.
>
>

I see, I think I misunderstood your previous message, sorry. Agreed
that there could be other approaches than an environment variable and
a command line option could definitely work as well. I'll leave the
details to you and Sergey, but ideally we would have something that is
simple to use both in scripts (e.g. distro build systems) and in
manual build for end users

>
>
>
> > > >
> > > > And one more question: those sanity checks seem very reasonable.
> > > > Is there any reason we would not want to keep them ON by default?
> > > > And those brave souls, that do not wish for the tool to very that
> > > > the .config is sane and nothing will get downgraded/disabled, can
> > > > always set KCONFIG_SANITY_CHECKS to 0.
> > >
> > >
> > > Kconfig is meant to resolve the dependency without causing an error.
> > > If a feature is not available, it is automatically, silently hidden,
> > > and that works well.
> >
> > How do you come to the conclusion that it works well? I've heard many
> > people unhappy about the way Kconfig works. How does one know that
> > something is missing (and should maybe be fixed?) if Kconfig silently
> > hides it?
>
>
> Kconfig has worked like that for a long time, but I do not know
> how to detect non-existing symbols.
>
>

I think a tool to detect symbols present in old config, but missing in
new kernel solves the "upgraded config" part of the problem.

The other part ("new config") would probably be solved by some kind of
a tool that looks at the currently present hardware and spews a list
of Kconfig options together with their dependencies, but arguably
that's not something that would be a part of Kconfig itself.

For the graphical configuration tools like menuconfig I could imagine
that the options with unmet dependencies could be still displayed but
greyed out, so at least one can open the help for the item and check
which dependencies are missing.

>
>
> >
> > >
> > > When a compiler does not support a particular feature,
> > > 'depends on $(cc-option )' hides that CONFIG option.
> > > Kconfig is meant to work like that.
> > >
> > >
> > >
> > > For your case, it is case-by-case.
> > >
> > > Let's say a stale code is removed from upstream.
> > >
> > > After "obj-$(CONFIG_FOO) += foo.o" is removed
> > > from upstream, CONFIG_FOO in the .config is a "don't care".
> > >
> > > If it were an error, all arch/*/configs/*_defconfig
> > > must be cleaned up at the same time.
> > >
> >
> > I'd argue that clean up should actually happen. An identically named
> > option could be added in the future again and mean something different
> > and would end up accidentally selected by those old defconfigs.
>
>
> For renaming, I agree with you.
> All defconfig files should be updated to keep the equivalent behavior.
>
> For code removal, defconfig cleaning can be deferred because
> that would possibly cause conflicts across subsystems.
>

True. In that case it sounds like the new behavior definitely needs to
be optional.

> Reusing the same CONFIG name for different meaning must be
> sorted out properly but that rarely happens, I guess.
>

Sure, I have to admit that it was a bit of an imaginary case. :)

>
>
> > >
> > > So, sometimes it is helpful, but sometimes noisy.
> > >
> > >
> > >
> > >
> > > For the MFD_RK808 case particularly,
> > > I believe Kconfig showed MFD_RK8XX_I2C
> > > as a new option.
> >
> > Among tens or hundreds of other new options. Good luck making sure
> > that you didn't miss it.
> >
> > >
> > > Or, when you bumped to a new kernel version,
> > > you could run 'make listnewconfig'.
> > > (See 17baab68d337a0bf4654091e2b4cd67c3fdb44d8.
> > > Redhat says they review every new config option.)
> > >
> >
> > What is the listnewconfig supposed to show?
>
>
> Documented in Documentation/kbuild/kconfig.rst
> line 16 - 34.
>

I see, thanks.

Best regards,
Tomasz

Powered by blists - more mailing lists