by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH 06/16] kconfig: refactor choice value calculation

On Wed, Jun 12, 2024 at 2:56 AM Masahiro Yamada <[email protected]> wrote:
>
> Handling choices has always been in a PITA in Kconfig.
>
> For example, fixes and reverts were repeated for randconfig with
> KCONFIG_ALLCONFIG:
>
> - 422c809f03f0 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
> - 23a5dfdad22a ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
> - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG")
> - 490f16171119 ("Revert "kconfig: fix randomising choice entries in presence of KCONFIG_ALLCONFIG"")
>
> As these commits pointed out, randconfig does not randomize choices when
> KCONFIG_ALLCONFIG is used. This issue still remains.
>
> [Test Case]
>
> choice
> prompt "choose"
>
> config A
> bool "A"
>
> config B
> bool "B"
>
> endchoice
>
> $ echo > all.config
> $ make KCONFIG_ALLCONFIG=1 randconfig
>
> The output is always as follows:
>
> CONFIG_A=y
> # CONFIG_B is not set
>
> Not only randconfig, but other all*config variants are broken with
> KCONFIG_ALLCONFIG.
>
> With the same Kconfig,
>
> $ echo '# CONFIG_A is not set' > all.config
> $ make KCONFIG_ALLCONFIG=1 allyesconfig
>
> You will get this:
>
> CONFIG_A=y
> # CONFIG_B is not set
>
> This is incorrect because it does not respect all.config.
>
> The correct output should be:
>
> # CONFIG_A is not set
> CONFIG_B=y
>
> To handle user inputs more accurately, this commit refactors the code
> based on the following principles:
>
> - When a user value is given, Kconfig must set it immediately.
> Do not defer it by setting SYMBOL_NEED_SET_CHOICE_VALUES.
>
> - The SYMBOL_DEF_USER flag must not be cleared, unless a new config
> file is loaded. Kconfig must not forget user inputs.
>
> In addition, user values for choices must be managed with priority.
> If user inputs conflict within a choice block, the newest value wins.
> The values given by randconfig have lower priority than explicit user
> inputs.
>
> This commit implements it by using a linked list. Every time a choice
> block gets a new input, it is moved to the top of the list.
>
> Let me explain how it works.
>
> Let's say, we have a choice block that consists of three symbols:
> A, B, and C.
>
> Initially, the linked list looks like this:
>
> A(=?) --> B(=?) --> C(=?)
>
> Say, '# CONFIG_B is not set' is specified by KCONFIG_ALLCONFIG.
>
> B is set to 'n', and moved to the top of the linked list:
>
> B(=n) --> A(=?) --> C(=?)
>
> The randconfig shuffles the symbols without a user value.
>
> So, you will get:
>
> B(=n) --> A(=y) --> C(=y)
> or
> B(=n) --> C(=y) --> A(=y)
>
> When calculating the output, the linked list is traversed. The first
> visible symbol with =y is taken. You will get either CONFIG_A=y or
> CONFIG_C=y with equal probability.
>
> As another example, let's say the .config with the following content
> is loaded:
>
> CONFIG_B=y
> CONFIG_C=y
>
> The linked list will become:
>
> C(=y) --> B(=y) --> A(=?)
>
> Please note the last one is prioritized when a decision conflicts in
> the same file. This is reasonable behavior because merge_config.sh
> appends config fragments to the existing .config file.
>
> So, the output will be CONFIG_C=y if C is visible, but otherwise
> CONFIG_B=y.
>
> This is different from the former implementation; previously, Kconfig
> forgot CONFIG_B=y when CONFIG_C=y appeared later in the same file.
>
> In the new implementation, Kconfig remembers both CONFIG_B=y and
> CONFIG_C=y, prioritizing the former.

prioritizing the latter.

> If C is hidden due to unmet
> dependency, CONFIG_B=y arises as the second best.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---

--
Best Regards
Masahiro Yamada