Support conditional deps using "depends on A if B"

[grahamroff-dev]

Extend the "depends on" syntax to support conditional dependencies using "depends on A if B". While functionally equivalent to "depends on !B || A", "depends on A if B" is much more readable.

This change is implemented by converting the "A if B" syntax into the "!B || A" syntax during "depends on" token processing.

[mbolivar]

It doesn't look like this is a 'standard' kconfig language feature:
https://docs.kernel.org/kbuild/kconfig-language.html#menu-attributes

Can you justify why it's worth an extension? https://docs.zephyrproject.org/latest/build/kconfig/extensions.html

[grahamroff-dev]

You are right that this would be a minor extension to the kconfig language. It has already been proposed as an update to the spec but it looks like the patch was not followed up on (https://lwn.net/Articles/818502/) after some initial discussion.

I can see a number of places in Zephyr where this would make the intended dependency a lot easier to understand. Some examples:

Kconfig.zephyr:
depends on !XIP || IS_BOOTLOADER can be more understandably expressed as depends on IS_BOOTLOADER if XIP

subsys/bluetooth/host/Kconfig:
depends on !BT_CTLR || BT_CTLR_DF_CTE_RX_SUPPORT →
depends on BT_CTLR_DF_CTE_RX_SUPPORT if BT_CTLR

drivers/bluetooth/hci/Kconfig:
depends on !PM || SOC_GECKO_PM_BACKEND_PMGR →
depends on SOC_GECKO_PM_BACKEND_PMGR if PM

drivers/ethernet/Kconfig.dwmac:
depends on (SOC_SERIES_STM32H7X && !ETH_STM32_HAL) || MMU →
depends on MMU if !SOC_SERIES_STM32H7X || ETH_STM32_HAL

[grahamroff-dev]

The language itself acknowledges that expressing conditional dependencies is confusing: https://docs.kernel.org/kbuild/kconfig-language.html#optional-dependencies

[mbolivar]

It has already been proposed as an update to the spec but it looks like the patch was not followed up on (https://lwn.net/Articles/818502/) after some initial discussion.

Very interesting, thanks for the reference.

I think this makes sense as an extension -- @carlescufi @nashif @tejlmand @kartben thoughts?

[mbolivar]

@grahamroff-dev I'm assuming that if this is merged and pulled into Zephyr, you would be willing to update the zephyr documentation for kconfig to list this as an additional extension, right?

Edit: a couple of additional thoughts if we decide to accept this:

• this would also need a mention in the release notes
• it'd be good to get some in-tree users or test code as well to make sure this doesn't regress

[decsny]

This change is implemented by converting the "A if B" syntax into the "!B || A" syntax during "depends on" token processing.

this is raising alarm bells for me. regardless of the decision to support the syntax or not, this implementation is a hack and I am afraid it will be a technical debt at best or buggy with all sorts of unexpected edge cases at worst. Obviously, testing is needed to be added for this, but I am not sure if it is worth it, if it's already possible to specify this logic. I would prefer not to add yet another surface area for bugs, the build tooling is already complex enough with endless amounts of trickery as it is

[bjarki-andreasen]

Dependencies that take the form "A depends on B if C" are a design flaw, as they are a laying violation.

A module can depend on another module, that is, "A depends on B". B must provide an interface which fulfills B, that is, no matter what other options are selected, B must always provide B. The form "A depends on B if C" can not fulfill this promise, as this indicate that C would modify B, that is not allowed. How would any other module which depends on B, deal with B suddenly providing a different promise, based on what should be a completely unrelated option?

Take the example depends on !PM || SOC_GECKO_PM_BACKEND_PMGR. If SOC_GECKO_PM_BACKEND_PMGR changes the behavior of PM, so that PM can not fulfill what BT_SILABS_EFR32 needs without SOC_GECKO_PM_BACKEND_PMGR as well, then PM is not used properly. PM would be unusable to any module which does not have knowledge of both PM and SOC_GECKO_PM_BACKEND_PMGR.

[bjarki-andreasen]

See comment

[Thalley]

I actually had this discussion with @tejlmand recently w.r.t. one of the Bluetooth specifications. depends of A if B isn't uncommon in Bluetooth. Support this sort of syntax in Kconfig would be helpful to not only capture conditional dependencies, but also make it easier to compare Zephyr's Kconfig dependencies with the dependencies from the Bluetooth specifications.

Here's an example of a set of conditional dependencies from the Common Audio Profile that use the language "Mandatory if X is supported" syntax

[grahamroff-dev]

I want to clarify that this change is not adding additional capability to the dependency system in Kconfig. It is already possible, and done in many places (I listed a few above), to logically have a conditional dependency using the syntactically confusing (!B || A). The purpose of this change is to allow expressing this in a more readable way as "A if B". I do not believe the implementation is a hack, it is a clean and simple way to handle this syntax feature.

A module can depend on another module, that is, "A depends on B". B must provide an interface which fulfills B, that is, no matter what other options are selected, B must always provide B. The form "A depends on B if C" can not fulfill this promise, as this indicate that C would modify B, that is not allowed

I believe this is a misinterpretation of what the conditional is implying. B always provides the same capabilities (C does not modify B or is even related to B necessarily) - but those capabilities are only required by A when C is also selected. I agree that such dependencies should be examined carefully to make sure the code design is logical, but it would not always be a layering violation to have them (as I believe the examples above indicate).

I can certainly update the documentation and provide test cases in whatever location such things are hosted for the Zephyr scripting tools.

[tejlmand]

Extend the "depends on" syntax to support conditional dependencies using "depends on A if B". While functionally equivalent to "depends on !B || A", "depends on A if B" is much more readable.

you're changing the meaning of if and depends on for the language.
Which again gives me the impression that the spec itself has not been read, meaning risk of bugs / unintended side-effect and thus tech debt increases.

Regarding depends on then the spec states:

• dependencies: depends on <expr>

This defines a dependency for this menu entry. If multiple dependencies are defined, they are connected with ‘&&’. Dependencies are applied to all other options within this menu entry (which also accept an “if” expression), so these two examples are equivalent:

bool "foo" if BAR
default y if BAR

and:

depends on BAR
bool "foo"
default y

based on above, then allowing the construct depends on BAR if BAZ would require it to be equivalent to:

bool "foo" if BAR if BAZ
default y if BAR if BAZ

which syntactically would be wrong, whereas today's construct of depends on BAR || !BAZ is valid:

bool "foo" if BAR || !BAZ
default y if BAR || !BAZ

Ref: https://docs.kernel.org/kbuild/kconfig-language.html#menu-attributes

[grahamroff-dev]

The conditional dependency should be defined as:

“depends on” <expr> [ if <expr2> ]

So it is the <expr> that would be propagated to all the menu items, but only if <expr2> is true. That keeps the statement that Dependencies are applied to all other options within this menu entry (which also accept an “if” expression) valid if you consider the applied dependency as itself being conditional (meaning only apply the dependency to other options if <expr2> is true).

Looking at your example (which technically has the order swapped for the two arguments), I would argue that

bool "foo" if !BAZ || BAR
default y if !BAZ || BAR

Is a lot less readable than:

depends on BAR if BAZ
bool "foo"
default y

Both express the same thing logically and evaluate to the same result, but one is much simpler to understand.

[tejlmand]

which technically has the order swapped for the two arguments

!BAZ || BAR == BAR || !BAZ so the order doesn't matter.

Looking at your example

"my" example is based on the specification which states that the depends on <expr> is equivalent to write the same <expr> using if <expr> on all options on the menu entry supporting if statements.

With your proposed change, then the depends on <expr> if <expr2> no longer have a direct equivalent.

The equivalent to depends on written like below is syntactically invalid:

bool "foo" if <expr> if <expr2>
default y if <expr> if <expr2>

you would need to write another form, such as:

bool "foo" if <expr> || !<expr2>
default y if <expr> || !<expr2>

but || and ! is nowhere seen in the depends on <expr> if <expr2> expression.
So depends on constructs no longer have a direct if equivalent.

There is another long form alternative version, such as:

if <expr2>
    bool "foo" if <expr>
    default y if <expr>
else
    bool "foo"
    default y
endif  

but both forms removes the straight forward relationship between depends on and if.

The Kconfig specification itself lives in Linux kernel project, and we follow that specification.
Extensions made in kconfiglib are independent extensions which does not change the functionality or modifies existing functions / options.

Examples

• rsource is an extension which does not change syntax or behavior of source.
• configdefault is an extension which does not change syntax or behavior of config or the options supported by config.

[grahamroff-dev]

The Kconfig specification itself lives in Linux kernel project, and we follow that specification.
Extensions made in kconfiglib are independent extensions which does not change the functionality or modifies existing functions / options.

This is the objection that I was expecting in the first place ;)

I believe that the specification itself is lacking in not supporting this construct. All other elements support a trailing "if " except "depends on", their documentation indicates that conditional dependencies are confusing to represent (see link above), and many examples exist in Zephyr (have not looked in Linux) where such a conditional dependency exists and has to be expressed in the confusing syntax (BT folks even chimed in above that this syntax would be preferred as it aligns with their specification).

If I am understanding the main objection, it is that such a change is not just a simple extension of the specification, but to some degree "breaks" it by implying a syntax of bool foo if <A> if <B>. This could be addressed in the spec itself by clarifying the behavior/meaning of the depends on <A> if <B>syntax, something like:

dependencies: "depends on" <expr> ["if" <expr2> ]

This defines a dependency for this menu entry. If multiple dependencies are defined, they are connected with ‘&&’. A given dependency <expr> is applied to all other options within this menu entry (which also accept an “if” expression) if <expr2> is satisfied...

Or something along those lines at least. I fully understand not wanting to take such a change if the source-of-truth reference spec does not support it, so the next step would be to suggest it to the Kconfig maintainer in Linux to get their feedback. Do folks here see value in trying to do that, or are the objections to this update more fundamental than just a spec violation?

At the end of the day, I believe the patch provides a fully functional implementation of a logically intuitive and consistent syntax update - minus the actual spec update ;). If anyone has examples of some construct that does not behave logically, let me know and I can test them out (I have extensive tests for this).

[tejlmand]

This is the objection that I was expecting in the first place ;)

that was the objection I gave in the first comment here: #21 (review)

you're changing the meaning of if and depends on for the language.

I even copied the text and link to the Zephyr kernel Kconfig language specification, including the text so these two examples are equivalent:.
Not sure how you could miss the fact that main objection was that you're not following the spec !!

This could be addressed in the spec itself by clarifying the behavior/meaning of the depends on <A> if <B> syntax.

Except that it's not only the text part, but you also need to consider the part regarding depends on being equivalent to if.
You keep ignoring the equivalent part.

A given dependency <expr> is applied to all other options within this menu entry (which also accept an “if” expression)

And what about this example:

config FOO
    bool "Foo"
    depends on BAR if BAZ
    depends on QUX

with your proposed text, then BAR should suddenly be applied to QUX if BAZ is true, because now depends on also excepts the if.
But spec also states: If multiple dependencies are defined, they are connected with ‘&&’, so kind of double.

Sure you can write your way out of that, but end-result goes from a very short text with a very easy to understand equivalent example today to something more complex.

Personally when you get to more complex scenarios, then it's easier to concatenate deps directly in mind with this:

config FOO
    bool "Foo"
    depends on BAR || !BAZ
    depends on QUX

config FOO
    depends on QUUX

to become (BAR || !BAZ) && QUX || QUUX.
And not having to know / remember the transformation of BAR if BAZ in above.

the next step would be to suggest it to the Kconfig maintainer in Linux to get their feedback. Do folks here see value in trying to do that, or are the objections to this update more fundamental than just a spec violation?

Sure, feel free to propose it upstream, that would be the correct place to start out.
Regarding the value, then for some simple cases, then perhaps the option will be easier for some, but when it comes to more complex cases, like I present in this comment, then I think current way of using depends on are easier to handle.