Re: [RFC PATCH 1/2] Kconfig: Introduce "uses" keyword

[Saeed Mahameed <saeedm@mellanox.com>]

On Mon, 2020-04-20 at 11:43 +0300, Jani Nikula wrote:
> On Sun, 19 Apr 2020, Masahiro Yamada <masahiroy@kernel.org> wrote:
> > On Sun, Apr 19, 2020 at 4:11 AM Nicolas Pitre <nico@fluxnic.net>
> > wrote:
> > > On Sun, 19 Apr 2020, Masahiro Yamada wrote:
> > > 
> > > > (FOO || !FOO) is difficult to understand, but
> > > > the behavior of "uses FOO" is as difficult to grasp.
> > > 
> > > Can't this be expressed as the following instead:
> > > 
> > >         depends on FOO if FOO
> > > 
> > > That would be a little clearer.
> > > 
> > > 
> > > Nicolas
> > 
> > 
> > 'depends on' does not take the 'if <expr>'
> > 
> > 'depends on A if B' is the syntax sugar of
> > 'depends on (A || !B), right ?
> > 
> > I do not know how clearer it would make things.
> > 
> > depends on (m || FOO != m)
> > is another equivalent, but we are always
> > talking about a matter of expression.
> > 
> > 
> > How important is it to stick to
> > depends on (FOO || !FOO)
> > or its equivalents?
> > 
> > 
> > If a driver wants to use the feature FOO
> > in most usecases, 'depends on FOO' is sensible.
> > 
> > If FOO is just optional, you can get rid of the dependency,
> > and IS_REACHABLE() will do logically correct things.
> 
> If by logically correct you mean the kernel builds, you're
> right. However the proliferation of IS_REACHABLE() is making the
> kernel
> config *harder* to understand. User enables FOO=m and expects BAR to
> use
> it, however if BAR=y it silently gets ignored. I have and I will
> oppose
> adding IS_REACHABLE() usage to i915 because it's just silently
> accepting
> configurations that should be flagged and forbidden at kconfig stage.
> 
> > I do not think IS_REACHABLE() is too bad,
> > but if it is confusing, we can add one more
> > option to make it explicit.
> > 
> > 
> > 
> > config DRIVER_X
> >        tristate "driver x"
> > 
> > config DRIVER_X_USES_FOO
> >        bool "use FOO from driver X"
> >        depends on DRIVER_X
> >        depends on DRIVER_X <= FOO
> >        help
> >          DRIVER_X works without FOO, but
> >          Using FOO will provide better usability.
> >          Say Y if you want to make driver X use FOO.
> > 
> > 
> > 
> > Of course,
> > 
> >       if (IS_ENABLED(CONFIG_DRIVER_X_USES_FOO))
> >                foo_init();
> > 
> > works like
> > 
> >       if (IS_REACHABLE(CONFIG_FOO))
> >                 foo_init();
> > 
> > 
> > At lease, it will eliminate a question like
> > "I loaded the module FOO, I swear.
> > But my built-in driver X still would not use FOO, why?"
> 

and duplicate this all over just to avoid new keyword.


> Please let's not make that a more widespread problem than it already
> is. I have yet to hear *one* good rationale for allowing that in the
> first place. And if that pops up, you can make it work by using
> IS_REACHABLE() *without* the depends, simply by checking if the
> module
> is there.
> 
> Most use cases increasingly solved by IS_REACHABLE() should use the
> "depends on FOO || FOO=n" construct, but the problem is that's not
> widely understood. I'd like to have another keyword for people to
> copy-paste into their Kconfigs.
> 

+1 

do all C developers know how the C compiler works ? of course not !
Same goes here, there is a demand for a new keyword, so people will
avoid copy and pate and can use the kconfig language in a higher
simplified level.

I just did a quick grep to find out how really people use depend on:

# All usage of depends on 
$ git ls-files | grep Kconfig | xargs grep -E "depends\s+on" | wc -l
15071

# simple single symbol expression usage 
$ git ls-files | grep Kconfig | xargs grep -E "depends\s+on\s+[A-Za-z0-
9_]+\s*$" | wc -l
8889

almost 60%.. 

people really like simple things especially for the tools they are
using "like kconfig", no one really wants to understand how it really
work under the hood if it is a one time thing that you need to setup
for your kernel project, unless it is really necessary ..

I wonder how many of those 8889 cases wanted a weak dependency but
couldn't figure out how to do it ? 

Users of depends on FOO || !FOO

$ git ls-files | grep Kconfig | xargs grep -E \
  "depends\s+on\s+([A-Za-z0-9_]+)\s*\|\|\s*(\!\s*\1|\1\s*=\s*n)" \
 | wc -l

156

a new keyword is required :) .. 


> In another mail I suggested
> 
> 	optionally depends on FOO
> 
> might be a better alternative than "uses".
> 
> 

how about just:
      optional FOO

It is clear and easy to document .. 


> BR,
> Jani.
>