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

[Zhangjin Wu]

Hi, Willy, Thomas

> > On Sun, Jul 30, 2023 at 12:54:45AM +0800, Zhangjin Wu wrote:
> > > > Also I find it odd to use $(ARCH) here, I would have expected $(XARCH)
> > > > since you probably want to distinguish ppc64 from ppc for example.
> > > >
> > > 
> > > Yes, we do, but the XARCH and ARCH mmapping patch is the 4th, will
> > > update this to XARCH, this one is the 3rd one, do we need to add this
> > > one after the 4th one?
> > 
> > OK indeed it's the 4th one that will modify this one then, no need
> > to reorder.
> > 
> > > > > > something like "make nolibctestconfig" to make an existing config ready for
> > > > > > nolibc-test.
> > > > > 
> > > > > Do you mean rename 'defconfig' to 'nolibctestconfig'? or something
> > > > > nolibc-test-config:
> > > > > 
> > > > >     nolibc-test-config:
> > > > > 	$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare
> > > > > 	$(Q)$(srctree)/scripts/kconfig/merge_config.sh -O "$(srctree)" -m "$(srctree)/.config" $(foreach c,$(EXTRA_CONFIG),$(wildcard $(CURDIR)/configs/$c))
> > > > > 	$(Q)$(MAKE) -C $(srctree) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) KCONFIG_ALLCONFIG="$(srctree)/.config" allnoconfig
> > > > > 
> > > > > It looks too long ;-)
> > > > 
> > > > I think that as long as we don't claim to call topdir's makefile targets
> > > > from this directory, we can reuse some similarly named targets which are
> > > > documented in "make help" and are non-ambiguous.
> > > 
> > > 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 ;-)

Best regards,
Zhangjin

> The complexity here is we have planned to support both defconfig and
> tinyconfig, what about this solution?
> 
> Thanks,
> Zhangjin
>
> > Willy