Received-SPF: pass (google.com: domain of linux-kernel+bounces-210949-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240611175536.3518179-1-masahiroy@kernel.org> <20240611175536.3518179-7-masahiroy@kernel.org>
In-Reply-To: <20240611175536.3518179-7-masahiroy@kernel.org>
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Wed, 12 Jun 2024 14:35:20 +0900
Message-ID: <CAK7LNAQFZu=z1ZT=6OUT8uiDN8mZS1++xGCNKc98Hc299cYqrQ@mail.gmail.com>
Subject: Re: [PATCH 06/16] kconfig: refactor choice value calculation
To: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 12, 2024 at 2:56=E2=80=AFAM Masahiro Yamada <masahiroy@kernel.o=
rg> 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 pres=
ence of KCONFIG_ALLCONFIG"")
>  - 8357b48549e1 ("kconfig: fix randomising choice entries in presence of =
KCONFIG_ALLCONFIG")
>  - 490f16171119 ("Revert "kconfig: fix randomising choice entries in pres=
ence 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=3D1 randconfig
>
> The output is always as follows:
>
>     CONFIG_A=3Dy
>     # 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=3D1 allyesconfig
>
> You will get this:
>
>     CONFIG_A=3Dy
>     # 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=3Dy
>
> 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(=3D?) --> B(=3D?) --> C(=3D?)
>
> 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(=3Dn) --> A(=3D?) --> C(=3D?)
>
> The randconfig shuffles the symbols without a user value.
>
> So, you will get:
>
>     B(=3Dn) --> A(=3Dy) --> C(=3Dy)
> or
>     B(=3Dn) --> C(=3Dy) --> A(=3Dy)
>
> When calculating the output, the linked list is traversed. The first
> visible symbol with =3Dy is taken. You will get either CONFIG_A=3Dy or
> CONFIG_C=3Dy with equal probability.
>
> As another example, let's say the .config with the following content
> is loaded:
>
>     CONFIG_B=3Dy
>     CONFIG_C=3Dy
>
> The linked list will become:
>
>     C(=3Dy) --> B(=3Dy) --> A(=3D?)
>
> 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=3Dy if C is visible, but otherwise
> CONFIG_B=3Dy.
>
> This is different from the former implementation; previously, Kconfig
> forgot CONFIG_B=3Dy when CONFIG_C=3Dy appeared later in the same file.
>
> In the new implementation, Kconfig remembers both CONFIG_B=3Dy and
> CONFIG_C=3Dy, prioritizing the former.


prioritizing the latter.


> If C is hidden due to unmet
> dependency, CONFIG_B=3Dy arises as the second best.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---


--=20
Best Regards
Masahiro Yamada