Re: [PATCH v3 3/7] selftests/nolibc: add extra configs customize support

Hi Zhangjin,

On Sun, Jul 30, 2023 at 02:01:31PM +0800, Zhangjin Wu wrote:

...

...

...

Seems 'nolibc-test-config' is really more meaningful than 'defconfig', especially when we want to use tinyconfig through it?

$ make nolibc-test-config DEFCONFIG=tinyconfig

As a user, I'd ask "why not make tinyconfig" ? But see my other message, now I'm having strong doubts about the relevance of tinyconfig if it works as bad as you described it.

I have added a nolibc tinyconfig target before, as the same reason, based on your suggestion, I have removed the tinyconfig target and even moved the extconfig to this defconfig to avoid add more similar or extra complex targets in nolibc Makefile. before, tinyconfig + extconfig together work for nolibc-test, so, tinyconfig is the same as the top-level one, it should be removed as your suggested.

But since now, we are ready to get a real different target from the top-level one, we may be able to have different targets for 'defconfig+EXTRA_CONFIG' and 'tinyconfig+EXTRA_CONFIG' like this:

nolibc-test-config:
        $(Q)echo $(MAKE_KERNEL) mrproper $(or $(CONFIG),defconfig)

It should be $(or $(CONFIG),$(DEFCONFIG))

...

        $(Q)echo $(srctree)/scripts/kconfig/merge_config.sh -Q -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTRA_CONFIG),$(wildcard $(CURDIR)/configs/$c))
        $(Q)echo $(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
        $(Q)echo $(MAKE_KERNEL) prepare

nolibc-test-defconfig nolibc-test-tinyconfig: nolibc-test-config
nolibc-test-tinyconfig: CONFIG=tinyconfig

The above two are not really required in this powerpc series, let's delay them to the next tinyconfig part1 series.

In this series, we only need:

 nolibc-test-config:
         $(Q)$(MAKE_KERNEL) mrproper $(or $(CONFIG),$(DEFCONFIG))
         $(Q)$(srctree)/scripts/kconfig/merge_config.sh -Q -O "$(objtree)" -m "$(KERNEL_CONFIG)" $(foreach c,$(EXTRA_CONFIG),$(wildcard $(CURDIR)/configs/$c))
         $(Q)$(MAKE_KERNEL) KCONFIG_ALLCONFIG="$(KERNEL_CONFIG)" allnoconfig
         $(Q)$(MAKE_KERNEL) prepare

And this:

# extra configs/ files appended to .config during the nolibc-test-config target
# include common + architecture specific
EXTRA_CONFIG         = common.config $(XARCH).config

And the update of the help target too.

If both of you are happy with this? let's do it ;-)

Well, I understand "nolibc-test-config" as "the config suitable to correctly run nolibc-test". In that case I agree. We can indeed start from the defconfig, and if we manage to refine it we can later map it to anything else that's known to work. And if we eventually split nolibc-test into multiple tests (libc, syscalls, ipc, network, whatever) then we might as well adopt different configs in the future. I'd also want to keep defconfig on its own because it relies on each arch's defconfig and is a simple way to verify they work (as they should). As I said, till now I haven't met issues with defconfig so probably we don't need to patch it, but we could revisit that option later if needed.

I'm still having that question about your "make allnoconfig" at the end, which for me simply replaces the config you've just created, thus which makes no sense. There might be a trick that I'm missing but if so it should be explained there because the only situation where anybody should start a question with "why" when reading code is when they discovered a bug.

Also please split the mrproper and defconfig on two distinc lines, as there's no point anymore in keeping them as a single one.

Since you're using the extra options for nolibc-test-config, I think they should be called "nolibc-test-common.config" and "nolibc-test-$XARCH.config" so that we don't restart with tons of "if" and macros when adding a new target.

Thomas, are you also OK with this ?

Thanks, Willy