Re: [PATCH] Documentation: kbuild: explain handling optional dependencies

[Nicolas Schier <nicolas@fjasle.eu>]

[-- Attachment #1: Type: text/plain, Size: 4544 bytes --]

On Wed, Sep 13, 2023 at 09:55:36PM +0200 Arnd Bergmann wrote:
> On Wed, Sep 13, 2023, at 21:48, Nicolas Schier wrote:
> > On Wed, Sep 13, 2023 at 01:37:52PM +0200 Arnd Bergmann wrote:
> >
> >>  Documentation/kbuild/kconfig-language.rst | 26 +++++++++++++++++++++++
> >>  1 file changed, 26 insertions(+)
> >> 
> >> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> >> index 858ed5d80defe..89dea587a469a 100644
> >> --- a/Documentation/kbuild/kconfig-language.rst
> >> +++ b/Documentation/kbuild/kconfig-language.rst
> >> @@ -573,6 +573,32 @@ above, leading to:
> >>  	bool "Support for foo hardware"
> >>  	depends on ARCH_FOO_VENDOR || COMPILE_TEST
> >>  
> >> +Optional dependencies
> >> +~~~~~~~~~~~~~~~~~~~~~
> >> +
> >> +Some drivers are able to optionally use a feature from another module
> >> +or build cleanly with that module disabled, but cause a link failure
> >> +when trying to use that loadable module from a built-in driver.
> >> +
> >> +The most common way to express this optional dependency in Kconfig logic
> >> +uses the slighly counterintuitive
> >
> > slighly -> slightly
> 
> Fixed, thanks
> 
> > For better RST compliance: could you explicitly start the code block e.g. by
> > appending '::' as in "... counterintuitive::"?
> 
> Ok, done.
> 
> >> +
> >> +  config FOO
> >> +	bool "Support for foo hardware"
> >> +	depends on BAR || !BAR
> >
> > are you sure that this is enough?  While testing, I needed to explicitly use
> > =y|=n:
> >
> >     depends on BAR=y || BAR=n
> >
> > to prevent FOO to be selectable iff BAR=m.
> 
> I see my problem, I made a different mistake here. Your version
> is correct for a 'bool' symbol as I had here, but the intention
> of this was to make it work for tristate symbols, which are the
> interesting case. I've fixed it up this way now, hope it now makes
> sense to you:
> 
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -581,19 +581,19 @@ or build cleanly with that module disabled, but cause a link failure
>  when trying to use that loadable module from a built-in driver.
>  
>  The most common way to express this optional dependency in Kconfig logic
> -uses the slighly counterintuitive
> +uses the slightly counterintuitive::
>  
>    config FOO
> -       bool "Support for foo hardware"
> +       tristate "Support for foo hardware"
>         depends on BAR || !BAR

ah, thanks, tristate kconfig symbols are really more interesting.  But I am
still not sure, whether this works as proposed:

With the 'config FOO' above and

  config BAR
  	tristate "Support for bar feature"

kconfig allows me to choose between these:

BAR=y  => FOO={N/m/y}
BAR=m  => FOO={N/m}
BAR=n  => FOO={N/m/y}

But with

  config FOO
  	tristate "Support for foo hardware"
  	depends on !BAR=m

I can choose between:

BAR=y  => FOO={N/m/y}
BAR=m  => FOO is not selectable
BAR=n  => FOO={N/m/y}

(Re-checked with BAR=IPV6 and FOO=WIREGUARD; CONFIG_WIREGUARD as 'depends on
IPV6 || !IPV6' in its kconfig definition, and both are tristate kconfig
symbols.)

Thus, it seems to me, that the intuitive way is the way forward (and several
Kconfigs are out-of-date with regard to 'depends on !X=m'.  Or do I still miss
your point?

Kind regards,
Nicolas



>  This means that there is either a dependency on BAR that disallows
>  the combination of FOO=y with BAR=m, or BAR is completely disabled.
>  For a more formalized approach if there are multiple drivers that have
> -the same dependency, a helper symbol can be used, like
> +the same dependency, a helper symbol can be used, like::
>  
>    config FOO
> -       bool "Support for foo hardware"
> +       tristate "Support for foo hardware"
>         depends on BAR_OPTIONAL
>  
>    config BAR_OPTIONAL
> 
> >> +This means that there is either a dependency on BAR that disallows
> >> +the combination of FOO=y with BAR=m, or BAR is completely disabled.
> >
> > For me, this sentence is hard to parse (but I am not a native speaker); what
> > about something like this:
> >
> > This means that FOO can only be enabled, iff BAR is either built-in or
> > completely disabled.  If BAR is built as a module, FOO cannot be enabled.
> 
> That would describe the version you suggested, but that's a
> different issue. Let me know if you still think it needs
> clarification after fixing the example.
> 
>       Arnd

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]