Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jan 30, 2020 at 02:06:18PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -49,7 +49,71 @@ default: build
> >  .PHONY: dist
> >  dist: install
> >  
> > -build install:: include/config/auto.conf
> > +
> > +ifndef root-make-done
> > +# section to run before calling Rules.mk, but only once.
> > +#
> > +# To make sure we do not include .config for any of the *config targets
> > +# catch them early, and hand them over to tools/kconfig/Makefile
> > +
> > +clean-targets := %clean
> > +no-dot-config-targets := $(clean-targets) \
> > +                    uninstall debug cloc \
> > +                    cscope TAGS tags MAP gtags \
> > +                    xenversion
> > +
> > +config-build       :=
> 
> Is this actually needed? While correct (afaict) together with the
> ifdef further down, I find this aspect of make behavior a little
> confusing, and hence it would seem slightly better if there was
> no empty definition of this symbol.

That's actually a very recent change in Linux source code. They used to
use ifeq($(config-build),1) and ifeq($(config-build),0). I can certainly
change back to use ifeq instead of ifdef.

> > +need-config        := 1
> 
> Here and below, would it be possible to use y instead of 1, to
> match how "true" gets expressed in various places elsewhere?
> Or would there again be deviation-from-Linux concerns?

It's probably fine to use "y". I don't think it matter, we need to make
quite a lot of changes compare to Linux anyway. I'll use "n" for the
negative.

> > +ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +   ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +           need-config :=
> > +   endif
> > +endif
> > +
> > +ifneq ($(filter config %config,$(MAKECMDGOALS)),)
> 
> Just $(filter %config, ...) suffices here, afaict, similar to
> above "%clean" also being used to cover "clean".

Yes, I'll remove the extra "config".

> > +   config-build := 1
> > +endif
> > +
> > +export root-make-done := 1
> > +endif # root-make-done
> > +
> > +ifdef config-build
> > +# 
> > ===========================================================================
> > +# *config targets only - make sure prerequisites are updated, and descend
> > +# in tools/kconfig to make the *config target
> > +
> > +config: FORCE
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +# Config.mk tries to include .config file, don't try to remake it
> > +%/.config: ;
> 
> This didn't exist before - why is it needed all of the sudden?

It's because I'm introducing a new target "%config". So when make
"-include $(XEN_ROOT)/.config" (as found in Config.mk) it check if the
file needs to be rebuilt, and find %config and thus run kconfig to build
.config.

Currently, Makefile list all the targets that needs to be built with
kconfig.

> > +%config: FORCE
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +else # !config-build
> > +
> > +ifdef need-config
> > +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
> > +
> > +# Allow people to just run `make` as before and not force them to configure
> > +$(KCONFIG_CONFIG):
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
> > +
> > +# The actual configuration files used during the build are stored in
> > +# include/generated/ and include/config/. Update them if .config is newer 
> > than
> > +# include/config/auto.conf (which mirrors .config).
> > +#
> > +# This exploits the 'multi-target pattern rule' trick.
> > +# The syncconfig should be executed only once to make all the targets.
> > +%/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> > +   $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) 
> > SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
> 
> Previously the target pattern was include/config/%.conf. Is there a
> particular reason for the switch?

That change was needed in Linux because the full target is:
    %/auto.conf %/auto.conf.cmd %/tristate.conf:
But since we don't need tristate.conf in Xen, I can go back to what we have.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel