Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220627221446.82157-1-dlatypov@google.com> <20220627221446.82157-2-dlatypov@google.com>
In-Reply-To: <20220627221446.82157-2-dlatypov@google.com>
From:   Brendan Higgins <brendanhiggins@google.com>
Date:   Wed, 6 Jul 2022 16:30:11 -0400
Message-ID: <CAFd5g44AkBpQeNr2WMf3e=-bHuEree_vH2sj3+JnsbBp=mQgyg@mail.gmail.com>
Subject: Re: [PATCH v3 2/3] kunit: tool: make --kunitconfig repeatable,
 blindly concat
To:     Daniel Latypov <dlatypov@google.com>
Cc:     davidgow@google.com, linux-kernel@vger.kernel.org,
        kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
        skhan@linuxfoundation.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jun 27, 2022 at 6:14 PM 'Daniel Latypov' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> It's come up a few times that it would be useful to have --kunitconfig
> be repeatable [1][2].
>
> This could be done before with a bit of shell-fu, e.g.
>   $ find fs/ -name '.kunitconfig' -exec cat {} + | \
>     ./tools/testing/kunit/kunit.py run --kunitconfig=/dev/stdin
> or equivalently:
>   $ cat fs/ext4/.kunitconfig fs/fat/.kunitconfig | \
>     ./tools/testing/kunit/kunit.py run --kunitconfig=/dev/stdin
>
> But this can be fairly clunky to use in practice.
>
> And having explicit support in kunit.py opens the door to having more
> config fragments of interest, e.g. options for PCI on UML [1], UML
> coverage [2], variants of tests [3].
> There's another argument to be made that users can just use multiple
> --kconfig_add's, but this gets very clunky very fast (e.g. [2]).
>
> Note: there's a big caveat here that some kconfig options might be
> incompatible. We try to give a clearish error message in the simple case
> where the same option appears multiple times with conflicting values,
> but more subtle ones (e.g. mutually exclusive options) will be
> potentially very confusing for the user. I don't know we can do better.
>
> Note 2: if you want to combine a --kunitconfig with the default, you
> either have to do to specify the current build_dir
> > --kunitconfig=.kunit --kunitconfig=additional.config
> or
> > --kunitconfig=tools/testing/kunit/configs/default.config --kunitconifg=additional.config
> each of which have their downsides (former depends on --build_dir,
> doesn't work if you don't have a .kunitconfig yet), etc.
>
> Example with conflicting values:
> > $ ./tools/testing/kunit/kunit.py config --kunitconfig=lib/kunit --kunitconfig=/dev/stdin <<EOF
> > CONFIG_KUNIT_TEST=n
> > CONFIG_KUNIT=m
> > EOF
> > ...
> > kunit_kernel.ConfigError: Multiple values specified for 2 options in kunitconfig:
> > CONFIG_KUNIT=y
> >   vs from /dev/stdin
> > CONFIG_KUNIT=m
> >
> > CONFIG_KUNIT_TEST=y
> >   vs from /dev/stdin
> > # CONFIG_KUNIT_TEST is not set
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2022-June/357616.html
> [2] https://lore.kernel.org/linux-kselftest/CAFd5g45f3X3xF2vz2BkTHRqOC4uW6GZxtUUMaP5mwwbK8uNVtA@mail.gmail.com/
> [3] https://lore.kernel.org/linux-kselftest/CANpmjNOdSy6DuO6CYZ4UxhGxqhjzx4tn0sJMbRqo2xRFv9kX6Q@mail.gmail.com/
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>