Re: [PATCH] Support conditional deps using "depends on X if Y"

[Graham Roff]

Jani,

> Right. I guess it takes a while to get used to the idiom A || !A. But
> then is it counter-productive to add an alternative that is apparently
> not much more helpful? And then we have two ways to express the same
> thing.

I think that expressing an optional dependency using "depends on
A if A" is easier to read and understand than "depends on A || !A". And
it is certainly easier to follow "depends on A if B" rather than
"depends on A || !B" - even realizing those are equivalent takes a
class in boolean logic ;)

Also, most other Kconfig attributes support the trailing "if <expr>"
so this makes the language more consistent.

> So the follow-up questions:
> 
> - Can we come up with a more obvious alternative to the specific case of
>   "A || !A"?

Meaning an entirely different syntax rather than "A if A"? Something
like "optional_depends on A" (just a made-up example to make the 
question clear)? That seems to be adding unnecessary syntax when the
conditional dependency covers both cases ("A if B" and "A if A"). If
there are any suggestions for a clearer way to express the purely
optional dependency case then that is worth looking at - but honestly
the entire concept seems a bit weird (even if widely used). It may 
just naturally be hard to express the concept in a simple manner.

> - Can we have examples of conversions from "A || !B" to "A if B" in
>   kernel Kconfigs? As in, don't add features without users.

(In the Zephyr pull-request I listed a number of examples from that 
project as well.)
You are correct that most commonly conditional dependencies are used
for *optional* dependencies, but there are a lot of other conditional
ones as well. Note when grepping for examples look for both "!A || B"
and "!B || A" - both forms are present in many places. A few examples:

arch/arm64/Kconfig:
  depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52 -->
  depends on ARM64_64K_PAGES if ARM64_VA_BITS_52
arch/mips/Kconfig:
  depends on SYS_SUPPORTS_HOTPLUG_CPU || !SMP -->
  depends on SYS_SUPPORTS_HOTPLUG_CPU if SMP
arch/riscv/Kconfig:
  depends on CC_HAS_MIN_FUNCTION_ALIGNMENT || !RISCV_ISA_C -->
  depends on CC_HAS_MIN_FUNCTION_ALIGNMENT if RISCV_ISA_C
arch/x86/Kconfig:
  depends on X86_64 || !SPARSEMEM -->
  depends on X86_64 if SPARSEMEM
drivers/acpi/Kconfig:
  depends on ACPI_WMI || !X86 -->
  depends on ACPI_WMI if X86
drivers/bluetooth/Kconfig:
  depends on USB || !BT_HCIBTUSB_MTK
  depends on USB if BT_HCIBTUSB_MTK
mm/Kconfig:
  depends on !ARM || CPU_CACHE_VIPT -->
  depends on CPU_CACHE_VIPT if ARM
kernel/Kconfig.locks:
  depends on !PREEMPTION || ARCH_INLINE_READ_UNLOCK -->
  depends on ARCH_INLINE_READ_UNLOCK if PREEMPTION

Are you suggesting an update to the commit text to call out the 
optional dependency use-case explicitly?

> My point is, there are like 10x more "A || !A" than there are "A || !B".
> Feels weird to advertize and document the thing for the latter, when the
> former is the more prevalent case.
> 
> $ git grep -E "depends on .*\b([A-Z0-9_]+) \|\| (\!\1\b|\1=n)"
> 
> I'm not at all opposed to the change per se.

The Kconfig documentation will cover both cases (in next patch). The
"A || !A" case is covered already in a special "Optional dependencies"
section which I will update to use "A if A" as a preferred syntax. I 
still suggest having the definition section for "depends on" using the 
easier to understand example of "depends on A if B".

Thanks, 

Graham