On 07.12.2023 14:44, Oleksii wrote: > On Thu, 2023-12-07 at 11:00 +0100, Jan Beulich wrote: >> On 07.12.2023 10:22, Oleksii wrote: >>> On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote: >>>>> On 24.11.2023 11:30, Oleksii Kurochko wrote: >>>>>>> The patch also fixes the build script as conf util expects >>>>>>> to have each config on separate line. >>>>> >>>>> The approach doesn't really scale (it's already odd that you >>>>> add >>>>> the >>>>> (apparently) same set four times. There's also zero >>>>> justification >>>>> for >>>>> this kind of an adjustment (as per discussion elsewhere I don't >>>>> think >>>>> we should go this route, and hence arguments towards convincing >>>>> me >>>>> [and >>>>> perhaps others] would be needed here). >>> I agree that this may not be the best approach, but it seems like >>> we >>> don't have too many options to turn off a config for randconfig. >>> >>> To be honest, in my opinion (IMO), randconfig should either be >>> removed >>> or allowed to fail until most of the functionality is ready. >>> Otherwise, >>> there should be a need to introduce HAS_* or depend on >>> 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs. >>> >>> Could you please suggest a better option? >> >> As to dropping randconfig tests, I'd like to refer you to Andrew, who >> is of the opinion that it was wrong to drop them for ppc. (I'm >> agreeing >> with him when taking a theoretical perspective, but I'm not happy >> with >> the practical consequences.) >> >> As to a better approach: Instead of listing the same set of options >> several times, can't there be a template config which is used to >> force >> randconfig to not touch certain settings? In fact at least for non- >> randconfig purposes I thought tiny64_defconfig / riscv64_defconfig >> already serve kind of a similar purpose. Imo the EXTRA_*CONFIG >> overrides >> are there for at most very few special case settings, not for >> purposes >> like you use them here. > The template will be the really a good option. > > What do you think about the following patch which introduces arch- > specific allrandom.config? > > diff --git a/xen/Makefile b/xen/Makefile > index ca571103c8..cb1eca76c2 100644 > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -336,11 +336,14 @@ ifeq ($(config-build),y) > # *config targets only - make sure prerequisites are updated, and > descend > # in tools/kconfig to make the *config target > > +ARCH_ALLRANDOM_CONFIG := > $(srctree)/arch/$(SRCARCH)/configs/allrandom.config > + > # Create a file for KCONFIG_ALLCONFIG which depends on the > environment. > # This will be use by kconfig targets > allyesconfig/allmodconfig/allnoconfig/randconfig > filechk_kconfig_allconfig = \ > $(if $(findstring n,$(XEN_HAS_CHECKPOLICY)), echo > 'CONFIG_XSM_FLASK_POLICY=n';) \ > - $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \ > + $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG); \ > + $(if $(wildcard $(ARCH_ALLRANDOM_CONFIG)), cat > $(ARCH_ALLRANDOM_CONFIG);) ) \ > :
Something along these lines may be okay, but why would the name be "allrandom" when the config is used elsewhere as well? Further, besides keeping randconfig and all*config from creating unusable configs, it will at least want considering whether in other cases that set of fixed values shouldn't be used as well then. Jan
