Re: [PATCH] build/xen: fail to rebuild if Kconfig fails

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.02.2024 12:04, Roger Pau Monné wrote:
> On Thu, Feb 15, 2024 at 11:43:02AM +0100, Jan Beulich wrote:
>> On 15.02.2024 11:28, Roger Pau Monné wrote:
>>> On Thu, Feb 15, 2024 at 10:49:31AM +0100, Jan Beulich wrote:
>>>> On 15.02.2024 10:30, Roger Pau Monne wrote:
>>>>> --- a/xen/Makefile
>>>>> +++ b/xen/Makefile
>>>>> @@ -358,10 +358,10 @@ config: tools_fixdep outputmakefile FORCE
>>>>>  else # !config-build
>>>>>  
>>>>>  ifeq ($(need-config),y)
>>>>> --include include/config/auto.conf
>>>>>  # Read in dependencies to all Kconfig* files, make sure to run 
>>>>> syncconfig if
>>>>>  # changes are detected.
>>>>>  -include include/config/auto.conf.cmd
>>>>> +include include/config/auto.conf
>>>>
>>>> With the - dropped, ...
>>>>
>>>>> @@ -375,6 +375,7 @@ $(KCONFIG_CONFIG): tools_fixdep
>>>>>  # This exploits the 'multi-target pattern rule' trick.
>>>>>  # The syncconfig should be executed only once to make all the targets.
>>>>>  include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
>>>>> + rm -rf include/config/$*.conf
>>>>>   $(Q)$(MAKE) $(build)=tools/kconfig syncconfig
>>>>
>>>> ... is this really necessary? The error status from the sub-make is ignored
>>>> only because of the -, isn't it?
>>>
>>> Without the `rm` the include/config/auto.conf is not removed by
>>> Kconfig on error, so the include will still succeed but use the stale
>>> auto.conf file.
>>>
>>> Keep in mind on rebuilds include/config/auto.conf is already present,
>>> so the rule is only executed for the include/config/auto.conf.cmd
>>> target.
>>
>> But the sub-make ought to return failure, which ought to then stop the
>> build process?
> 
> For some reason it doesn't, not at least with GNU Make 4.3.
> 
> It stops the build if the '-' is dropped from the include of
> include/config/auto.conf.cmd.  But that will always fail as
> include/config/auto.conf.cmd is never created.
> 
> Maybe there's something weird with our makefile, I certainly don't
> know that much, but as noted in the commit message,
> include/config/auto.conf.cmd failing doesn't cause the build to
> stop.

How about the below as an alternative? I'm not overly happy with the
double ifneq, but I also don't see a good other option.

This way the missing auto.conf is detected slightly later, but this
may well be good enough. Then again I might be overlooking yet
something else that this breaks ...

Jan

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -375,6 +375,7 @@ $(KCONFIG_CONFIG): tools_fixdep
 # This exploits the 'multi-target pattern rule' trick.
 # The syncconfig should be executed only once to make all the targets.
 include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
+       $(Q)rm -f include/config/$*.conf
        $(Q)$(MAKE) $(build)=tools/kconfig syncconfig
 
 ifeq ($(CONFIG_DEBUG),y)
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -15,7 +15,11 @@ srcdir := $(srctree)/$(src)
 PHONY := __build
 __build:
 
--include $(objtree)/include/config/auto.conf
+ifneq ($(obj),tools)
+ifneq ($(obj),tools/kconfig)
+include $(objtree)/include/config/auto.conf
+endif
+endif
 
 include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include