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

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

On Thu, Feb 15, 2024 at 02:02:41PM +0100, Jan Beulich wrote:
> 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.

Hm, yes, having the checks against specific paths is IMO not ideal.

> 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

Trying to understand this, I assume it's to avoid an infinite
dependency loop that generating include/config/auto.conf requires some
tools that are build using xen/Rules.mk?

Thanks, Roger.