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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

> >> 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.

Without the re-ordering the execution is not stopped on failure to
generate include/config/auto.conf:

# gmake -j8 xen clang=y
gmake -C xen install
gmake[1]: Entering directory '/root/src/xen/xen'
rm -rf include/config/auto.conf
tools/kconfig/conf  --syncconfig Kconfig
common/Kconfig:2: syntax error
common/Kconfig:1: invalid statement
gmake[2]: *** [tools/kconfig/Makefile:73: syncconfig] Error 1
gmake[1]: Failed to remake makefile 'include/config/auto.conf'.
  UPD     include/xen/compile.h
 Xen 4.19-unstable
gmake[3]: Nothing to be done for 'all'.
[...]

With the re-ordering:

# gmake -j8 xen clang=y
gmake -C xen install
gmake[1]: Entering directory '/root/src/xen/xen'
rm -rf include/config/auto.conf
tools/kconfig/conf  --syncconfig Kconfig
common/Kconfig:2: syntax error
common/Kconfig:1: invalid statement
gmake[2]: *** [tools/kconfig/Makefile:73: syncconfig] Error 1
gmake[1]: *** [Makefile:379: include/config/auto.conf] Error 2
gmake[1]: Leaving directory '/root/src/xen/xen'
gmake: *** [Makefile:143: install-xen] Error 2
#

So the re-ordering is meaningful.

Thanks, Roger.