Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220519164512.3180360-1-dlatypov@google.com> <CABVgOSnooocLsy2=a8rm7Y_m3DpffKtDam5_uYou+Y2tUkumRw@mail.gmail.com>
In-Reply-To: <CABVgOSnooocLsy2=a8rm7Y_m3DpffKtDam5_uYou+Y2tUkumRw@mail.gmail.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Fri, 20 May 2022 11:13:19 -0700
Message-ID: <CAGS_qxrDYKKoWy3UAuVqebT+3jp-ux_uyfbwX3OnJqVbnzaiJg@mail.gmail.com>
Subject: Re: [PATCH] kunit: tool: refactor internal kconfig handling, allow overriding
To:     David Gow <davidgow@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Shuah Khan <skhan@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, May 19, 2022 at 11:13 PM David Gow <davidgow@google.com> wrote:
>
> I like this, but do think there are a few gaps this doesn't handle
> properly. (Though exactly how we'd deal with them, I'm not yet sure.)
>
> In particular, it's not possible to disable a pair of options where
> one depends on the other: disabling the parent option will result in
> the child one not being present in the generated config. This will
> conflict both with "=y" and "=n/not set": we'd need a way to _remove_
> a kconfig option for that to work.

Do you have an example?
Because what you describe sounds like how we want it to work, but I'm
not sure if I'm misunderstanding the scenario you describe.

I was considering the case mentioned in the commit description.
I.e. we do --kunitconfig_add=CONFIG_KUNIT=n to the default kunitconfig.
That gives us complaints about these
 CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y,
 CONFIG_KUNIT_ALL_TESTS=y
options no longer being in the generated .config.
And I think that's exactly how it _should_ work, as this flag is a
low-level tool for tweaking individual options.

IMO, anything complicated should be done by editing the
kunitconfig/qemu_config files, in which case it's a lot less
cumbersome to disable multiple options by just deleting them.

Daniel