On 07.12.2023 15:51, Oleksii wrote:
> On Thu, 2023-12-07 at 15:11 +0100, Jan Beulich wrote:
>> 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?
> The naming is not optimal. "unused.config" or "ignored.config" would be
> a better choice.
I don't think "unused" or "ignored" are properly describing what such
a file would hold. I was vaguely playing with "fixed" or "forced", fwiw.
>> 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.
> If I understood you correctly, the other case is *defconfig targets.
> Therefore, the following targets might also need to be updated by
> merging "unused.config" with {defconfig,%_defconfig}:
>
>
> defconfig: $(obj)/conf
> ifneq ($(wildcard
> $(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),)
> @$(kecho) "*** Default configuration is based on
> '$(KBUILD_DEFCONFIG)'"
> $(Q)$< $(silent) --
> defconfig=arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) $(Kconfig)
> else
> @$(kecho) "*** Default configuration is based on target
> '$(KBUILD_DEFCONFIG)'"
> $(Q)$(MAKE) -f $(srctree)/Makefile $(KBUILD_DEFCONFIG)
> endif
>
> %_defconfig: $(obj)/conf
> $(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@
> $(Kconfig)
>
> However, I believe it's possible that for *defconfig, a configuration
> should be set to N, but in randconfig, it is still acceptable to be set
> to Y.
I'm not sure *defconfig should be affected by what's in the "locked
down" config, assuming they're consistent with what may not be changed.
But of course one could also model this incrementally, such that
defconfigs don't need to repeat what the "locked down" one has.
Anyway, I think this needs sending an RFC with the right people in Cc,
such that this can be properly discussed.
Jan
