Re: [PATCH 1/4] kconfig: introduce the "imply" keyword

On Thu, 20 Oct 2016, Edward Cree wrote:

On 20/10/16 18:04, Nicolas Pitre wrote:

quoted

On Thu, 20 Oct 2016, Edward Cree wrote:

quoted

Also, I don't think having any FOO=y should preclude BAZ=m.  Suppose both
FOO and FOO2 imply BAZ, FOO=y and FOO2=m.

Some people didn't like the fact that you could turn a driver from m to
y and silently lose some features if they were provided by a subsystem
that also used to be m, which arguably is not the same as being
explicitly disabled.  With "select" this is not a problem as the target
symbol is also promoted to y in that case, so I wanted to preserve that
property.

Right, but that's an argument for pushing the subsystem's default to y,
not for preventing changing the subsystem back to m afterwards.

quoted

quoted

Then if BAZ-features are only
desired for driver FOO2, BAz=m makes sense.

In that case it would make more sense to add a config option related to
FOO asking if BAZ features are desired for that driver (there is already
one occurrence of that with PTP).  Or you could simply drop the "imply"
statement from the FOO config entry.

But the desire is a property of the user, not of the driver.  If you're
willing to add CONFIG_FOO_BAZ to every combination of (driver, subsystem)
then "imply" becomes unnecessary, doesn't it?

Absolutely.  And if that's something that inspires you please be my 
guest.  So far, though, this apparently didn't inspire the majority of 
driver authors who preferred to have a smaller set of config options and 
forcefully pull in the BAZ features with a "select".  But "select" comes 
with its set of evils which "imply" is meant to overcome.

Conversely, if you *don't*
want to have to do that, then "imply" needs to only ever deal in defaults,
not in limitations.

As I explained, It still has to prevent BAZ=m if FOO moves from m to y 
otherwise this would effectively have the same result as BAZ=n in 
practice and that is not what people expect if BAZ actually isn't n in 
your .config file.  That's why "select" also has that particular 
semantic.

Here "imply" is meant to be a weaker form of "select".  If you prefer 
not to have that limitation imposed by either "select" and "imply" then 
simply don't use them at all.  Nothing forces you to use any of them if 
your code can cope with any config combination.

In those cases where "imply" is used, you could drop it altogether 
already. But that's for driver authors to decide. If they went with 
"select" in the first place, there might be a reason, and "imply" is 
there to preserve that reason, semantically at least, without the 
handcuff effect that "select" imposes on the whole thing.

quoted

quoted

There is also the case of drivers with the ability to detect at runtime
whether BAZ is present, rather than making the decision at build time, but
I'm not sure how common that is.

Right now that's how PTP support is done.  Drivers can optimize things
at build time, but most of them simply cope with a NULL return from
ptp_clock_register().  Hence the imply statement becomes a big
configuration hint rather than some hard build dependency.

Right, so those drivers can use PTP if they're y and PTP is m, as long 
as the PTP module is loaded when they probe.

Not at the moment. There is no way for PTP to dynamically signal to 
interested drivers its presence at run time.  And drivers, when 
built-in, typically probe their hardware during the boot process even 
before you have the chance to load any module. If that ever changes, 
then the imply or select statement could simply be dropped.

But current "imply" semantics won't allow that...

And that's on purpose.

I think that Josh's suggestion (have the UI warn you if you set BAZ to m
while FOO=y) is the right approach, but I also think it should be done
now rather than at some unspecified future time.

Please advocate this with kconfig UI authors.  My recursion stack is 
already quite deep.

Otherwise you forbid
potentially valid configs.

Like I said, if FOO=y and BAZ=m is a valid config, all you have to do is 
omit "imply BAZ" or "select BAZ" from the FOO config entry.  It's as 
simple as that.


Nicolas