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

[Jan Beulich <jbeulich@xxxxxxxx>]

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?

>> I also don't really follow the need to re-order the include-s above. Their
>> ordering ought to be benign, as per make's doc stating "If an included
>> makefile cannot be found in any of these directories it is not an
>> immediately fatal error; processing of the makefile containing the include
>> continues." While the description talks about this, I'm afraid I don't
>> really understand "... the .cmd target is executed before including ...":
>> What .cmd target are you talking about there?
> 
> Without the reordering the include of include/config/auto.conf will
> always succeed on rebuilds, because the include is done before
> executing the include/config/%.conf.cmd target that does the `rm`.

That's a dual target: It also handles include/config/%.conf. I.e.
because of this ...

> With the current order the include of include/config/%.conf.cmd that
> triggers the re-build of auto.conf happens after having included the
> file already.

... either include would trigger this same rule. IOW I'm afraid I'm still
not seeing what is gained by the re-ordering. I'm also unconvinced that
"triggers" in the sense you use it is actually applicable. Quoting make
doc again: "Once it has finished reading makefiles, make will try to
remake any that are out of date or don’t exist." To me this means that
first all makefile reading will finish, and then whichever included files
need re-making will be re-made.

Jan