| Message ID | 20230913113801.1901152-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Documentation: kbuild: explain handling optional dependencies | expand |
Arnd Bergmann <arnd@kernel.org> writes: Hello Arnd, > From: Arnd Bergmann <arnd@arndb.de> > > This problem frequently comes up in randconfig testing, with > drivers failing to link because of a dependency on an optional > feature. > > The Kconfig language for this is very confusing, so try to > document it in "Kconfig hints" section. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Thanks for writing this since as you mention that Kconfig idiom is not intuitive. The docs looks great to me! Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
On Wed, Sep 13, 2023 at 01:37:52PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This problem frequently comes up in randconfig testing, with > drivers failing to link because of a dependency on an optional > feature. > > The Kconfig language for this is very confusing, so try to > document it in "Kconfig hints" section. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
On Wed, Sep 13, 2023 at 01:37:52PM +0200 Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This problem frequently comes up in randconfig testing, with > drivers failing to link because of a dependency on an optional > feature. > > The Kconfig language for this is very confusing, so try to > document it in "Kconfig hints" section. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Hi Arnd, thanks for documenting this! Three questions below: > 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 For better RST compliance: could you explicitly start the code block e.g. by appending '::' as in "... counterintuitive::"? > + > + 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. > + > +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. Kind regards, Nicolas
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 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
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
On Wed, Sep 13, 2023, at 22:34, Nicolas Schier wrote: > On Wed, Sep 13, 2023 at 09:55:36PM +0200 Arnd Bergmann wrote: >> 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} That is indeed the point: if BAR=m, we want to be able to pick FOO=m here, otherwise it is impossible to enabled everything as modules. Another correct way to express the same thing as the first would be config FOO tristate "Support for foo hardware" depends on !BAR=m || m which I find even more confusing than the 'BAR || !BAR' convention, though we have that in a couple of places. I just found another variant that I had not seen before: > (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? I'm not sure what you mean here, but it appears that one of us is missing the point ;-) Arnd
On Wed 13 Sep 2023 23:16:47 GMT, Arnd Bergmann wrote: > On Wed, Sep 13, 2023, at 22:34, Nicolas Schier wrote: > > On Wed, Sep 13, 2023 at 09:55:36PM +0200 Arnd Bergmann wrote: > > >> 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} > > That is indeed the point: if BAR=m, we want to be able to pick FOO=m > here, otherwise it is impossible to enabled everything as modules. oh, I misinterpreted your very first sentence; thanks for clarifying it to me and sorry for the noise. With the minor fixes: Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> Kind regards, Nicolas
"Arnd Bergmann" <arnd@arndb.de> writes: Hello Nicolas, > On Wed, Sep 13, 2023, at 22:34, Nicolas Schier wrote: >> On Wed, Sep 13, 2023 at 09:55:36PM +0200 Arnd Bergmann wrote: > [...] >> I can choose between: >> >> BAR=y => FOO={N/m/y} >> BAR=m => FOO is not selectable >> BAR=n => FOO={N/m/y} > > That is indeed the point: if BAR=m, we want to be able to pick FOO=m > here, otherwise it is impossible to enabled everything as modules. > > Another correct way to express the same thing as the first would > be > > config FOO > tristate "Support for foo hardware" > depends on !BAR=m || m > > which I find even more confusing than the 'BAR || !BAR' > convention, though we have that in a couple of places. > > I just found another variant that I had not seen before: > >> (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.) >> Which is correct because WIREGUARD can be built with IPV6 disabled, but if both options are enabled then WIREGUARD can only be built-in if the IPV6 option is also built-in. WIREGUARD must be a module if IPV6 is also a module, but can still be a module if IPV6 is built-in. In other words, what this idiom express is that the following configs are possible: IPV6=n => WIREGUARD=y IPV6=n => WIREGUARD=m IPV6=y => WIREGUARD=y IPV6=y => WIREGUARD=m IPV6=m => WIREGUARD=m but the following option is not possible: IPV6=m => WIREGUARD=y
On Thu, Sep 14, 2023, at 05:51, Nicolas Schier wrote: > On Wed 13 Sep 2023 23:16:47 GMT, Arnd Bergmann wrote: >> On Wed, Sep 13, 2023, at 22:34, Nicolas Schier wrote: >> > >> > BAR=y => FOO={N/m/y} >> > BAR=m => FOO is not selectable >> > BAR=n => FOO={N/m/y} >> >> That is indeed the point: if BAR=m, we want to be able to pick FOO=m >> here, otherwise it is impossible to enabled everything as modules. > > oh, I misinterpreted your very first sentence; thanks for clarifying it to me > and sorry for the noise. > With the minor fixes: > > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > Ok, thanks! I understand that the text is still confusing, so if anyone has an idea for how to improve it further, let me know, otherwise I'll send what I have now with type fixes as v2. Arnd
On Wed, 13 Sep 2023, Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This problem frequently comes up in randconfig testing, with > drivers failing to link because of a dependency on an optional > feature. > > The Kconfig language for this is very confusing, so try to > document it in "Kconfig hints" section. Thanks for doing this. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > 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 > + > + config FOO > + bool "Support for foo hardware" > + depends on BAR || !BAR depends on BAR || BAR=n seems to be an alternative that's about as common: $ git grep "depends on \([A-Z0-9_]\+\) || \!\1" | wc -l 109 $ git grep "depends on \([A-Z0-9_]\+\) || \1=n" | wc -l 107 Maybe worth mentioning both? BR, Jani. > + > +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 > + > + config FOO > + bool "Support for foo hardware" > + depends on BAR_OPTIONAL > + > + config BAR_OPTIONAL > + def_tristate BAR || !BAR > + > Kconfig recursive dependency limitations > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
On Thu, Sep 14, 2023, at 15:42, Jani Nikula wrote: > On Wed, 13 Sep 2023, Arnd Bergmann <arnd@kernel.org> wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> +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 >> + >> + config FOO >> + bool "Support for foo hardware" >> + depends on BAR || !BAR > > depends on BAR || BAR=n > > seems to be an alternative that's about as common: > > $ git grep "depends on \([A-Z0-9_]\+\) || \!\1" | wc -l > 109 > $ git grep "depends on \([A-Z0-9_]\+\) || \1=n" | wc -l > 107 > > Maybe worth mentioning both? I fear that would add more confusion than it avoids: "!BAR" is actually different from "BAR=n", but "BAR || !BAR" is the same as "BAR || BAR=n" here, and trying to explain this in the documentation would either make it incorrect or unhelpfully complicated. Arnd
On Thu, 14 Sep 2023, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Thu, Sep 14, 2023, at 15:42, Jani Nikula wrote: >> On Wed, 13 Sep 2023, Arnd Bergmann <arnd@kernel.org> wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> +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 >>> + >>> + config FOO >>> + bool "Support for foo hardware" >>> + depends on BAR || !BAR >> >> depends on BAR || BAR=n >> >> seems to be an alternative that's about as common: >> >> $ git grep "depends on \([A-Z0-9_]\+\) || \!\1" | wc -l >> 109 >> $ git grep "depends on \([A-Z0-9_]\+\) || \1=n" | wc -l >> 107 >> >> Maybe worth mentioning both? > > I fear that would add more confusion than it avoids: > "!BAR" is actually different from "BAR=n", but *head explodes* > "BAR || !BAR" is the same as "BAR || BAR=n" here, and > trying to explain this in the documentation would either > make it incorrect or unhelpfully complicated. Fair enough. BR, Jani.
On Thu, Sep 14, 2023 at 5:34 AM Nicolas Schier <nicolas@fjasle.eu> wrote: > > 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. Both FOO and BAR MUST be tristate to make this documentation sensible. If FOO is bool type, "depends on BAR || !BAR" becomes a no-op. As you notice, FOO and BAR become independent of each other. You may wonder why. Here, another unclear rule applies: 'depends on m' for a bool option is promoted to 'depends on y'. https://github.com/torvalds/linux/blob/v6.5/scripts/kconfig/symbol.c#L214
On Thu, Sep 14, 2023 at 11:57 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Sep 14, 2023, at 15:42, Jani Nikula wrote: > > On Wed, 13 Sep 2023, Arnd Bergmann <arnd@kernel.org> wrote: > >> From: Arnd Bergmann <arnd@arndb.de> > >> > >> +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 > >> + > >> + config FOO > >> + bool "Support for foo hardware" > >> + depends on BAR || !BAR > > > > depends on BAR || BAR=n > > > > seems to be an alternative that's about as common: > > > > $ git grep "depends on \([A-Z0-9_]\+\) || \!\1" | wc -l > > 109 > > $ git grep "depends on \([A-Z0-9_]\+\) || \1=n" | wc -l > > 107 > > > > Maybe worth mentioning both? > > I fear that would add more confusion than it avoids: > "!BAR" is actually different from "BAR=n", but > "BAR || !BAR" is the same as "BAR || BAR=n" here, and > trying to explain this in the documentation would either > make it incorrect or unhelpfully complicated. The rules are already explained in line 231-278 of Documentation/kbuild/kconfig-language.rst y, m, n are internally 2, 1, 0. !A returns (2 - A). A=B returns 2 if the equation is true, 0 otherwise. A||B returns max(A,B) Given those in my mind, this is simple math. For each case of BAR=y, =m, =n, BAR 2 1 0 !BAR 0 1 2 BAR=n 0 0 2 BAR||!BAR 2 1 2 BAR||BAR=n 2 1 2 BAR!=m||m 2 1 2 So, the last three are equivalent. They are equally complicated and confusing, though. After all, what we are doing is to create this matrix: | WIREGUARD | y m n ---------------------------- y | O O O IPV6 m | X O O n | O O O It is unclear why WIREGUARD must be entirely disabled just because of the optional feature being modular. My preference is to use IS_REACHABLE(CONFIG_IPV6) instead of IS_ENABLED(CONFIG_IPV6) under drivers/net/wireguard, then get rid of "depends on IPV6 || !IPV6) If you want to make it clearer on the Kconfig level, perhaps the following is also possible. config WIREGUARD tristate "WireGuard" config WIREGUARD_IPV6 def_bool y depends on WIREGUARD depends on IPV6 >= WIREGUARD config IPV6 tristate "IPV6"
On Thu, Sep 14, 2023, at 19:23, Masahiro Yamada wrote: > On Thu, Sep 14, 2023 at 11:57 PM Arnd Bergmann <arnd@arndb.de> wrote: >> On Thu, Sep 14, 2023, at 15:42, Jani Nikula wrote: > > It is unclear why WIREGUARD must be entirely disabled > just because of the optional feature being modular. I don't think anyone is asking for that, and the current "depends on IPV6 || !IPV6" seems fine here, and is consistent with dozens of other symbols. > My preference is to use IS_REACHABLE(CONFIG_IPV6) > instead of IS_ENABLED(CONFIG_IPV6) > under drivers/net/wireguard, then > get rid of "depends on IPV6 || !IPV6) My feeling is that this would be significantly worse from a usability point of view even if it made it a little easier for maintainers: When a user selects both IPV6 and WIREGUARD, they expect to be able to use them together, and a normal user setting WIREGUARD=y would have a hard time figuring out why that leads it becoming IPv4-only. > If you want to make it clearer on the Kconfig level, > perhaps the following is also possible. > > > config WIREGUARD > tristate "WireGuard" > > config WIREGUARD_IPV6 > def_bool y > depends on WIREGUARD > depends on IPV6 >= WIREGUARD > > config IPV6 > tristate "IPV6" That has the same downside, with the added problem of also confusing kernel developers with the '>=' Kconfig syntax, which IMHO makes no sense unless one knows way too much about Kconfig internals. Arnd
On Fri, 15 Sep 2023, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Thu, Sep 14, 2023, at 19:23, Masahiro Yamada wrote: >> On Thu, Sep 14, 2023 at 11:57 PM Arnd Bergmann <arnd@arndb.de> wrote: >>> On Thu, Sep 14, 2023, at 15:42, Jani Nikula wrote: >> >> It is unclear why WIREGUARD must be entirely disabled >> just because of the optional feature being modular. > > I don't think anyone is asking for that, and the current > "depends on IPV6 || !IPV6" seems fine here, and is consistent > with dozens of other symbols. > >> My preference is to use IS_REACHABLE(CONFIG_IPV6) >> instead of IS_ENABLED(CONFIG_IPV6) >> under drivers/net/wireguard, then >> get rid of "depends on IPV6 || !IPV6) > > My feeling is that this would be significantly worse from a > usability point of view even if it made it a little easier > for maintainers: > > When a user selects both IPV6 and WIREGUARD, they expect > to be able to use them together, and a normal user setting > WIREGUARD=y would have a hard time figuring out why that > leads it becoming IPv4-only. I think IS_REACHABLE() is in most cases just plain wrong, and should only be used as the last resort. I think the kconfig should express what the dependencies are, and you should get kconfig or build errors if you get it wrong, and not paper over them with IS_REACHABLE(). Configuring the kernel is hard enough, and IS_REACHABLE() silences the issues with no messages to the user at any point. If the user gets it wrong, it just doesn't work like they expect, they have no clues why, and they have to peruse the kernel source to figure it out. (Or, more likely, file a bug and waste the kernel developer/maintainer time to get the configuration right.) IS_REACHABLE() considered harmful. BR, Jani. > >> If you want to make it clearer on the Kconfig level, >> perhaps the following is also possible. >> >> >> config WIREGUARD >> tristate "WireGuard" >> >> config WIREGUARD_IPV6 >> def_bool y >> depends on WIREGUARD >> depends on IPV6 >= WIREGUARD >> >> config IPV6 >> tristate "IPV6" > > That has the same downside, with the added problem > of also confusing kernel developers with the '>=' > Kconfig syntax, which IMHO makes no sense unless one > knows way too much about Kconfig internals. > > Arnd
On Fri, Sep 15, 2023, at 09:34, Jani Nikula wrote: > > IS_REACHABLE() considered harmful. Absolutely agreed, and I'm sorry I introduced it in the first place in commit 9b174527e7b75 ("[media] Add and use IS_REACHABLE macro"). At the time, it was only used by drivers/media, which used to have a lot of open-coded instances of it and a lot of wrong checks. Having a formal syntax for it was an improvement for drivers/media since it was more broken before, but it's usually a mistake to use it when there is another solution, and we probably should have tried harder to fix the dependencies in drivers/media at the time. Arnd
On 9/15/23 00:44, Arnd Bergmann wrote: > On Fri, Sep 15, 2023, at 09:34, Jani Nikula wrote: >> >> IS_REACHABLE() considered harmful. > +1 absolutely. > Absolutely agreed, and I'm sorry I introduced it in the > first place in commit 9b174527e7b75 ("[media] Add and use > IS_REACHABLE macro"). > > At the time, it was only used by drivers/media, which used > to have a lot of open-coded instances of it and a lot of > wrong checks. > > Having a formal syntax for it was an improvement for > drivers/media since it was more broken before, but it's > usually a mistake to use it when there is another solution, > and we probably should have tried harder to fix the > dependencies in drivers/media at the time. > > Arnd
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 + + config FOO + bool "Support for foo hardware" + depends on BAR || !BAR + +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 + + config FOO + bool "Support for foo hardware" + depends on BAR_OPTIONAL + + config BAR_OPTIONAL + def_tristate BAR || !BAR + Kconfig recursive dependency limitations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~