| Message ID | 20200117105358.607910-10-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements | expand |
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. > +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? > +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". > + 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? > +%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? Jan
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,
On 03.02.2020 12:45, Anthony PERARD wrote: > 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. Then perhaps, along the lines of ... >>> +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. ... this, also use y/n? >>> +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. Ah, I see - we didn't have a %config target anywhere at all. Jan
diff --git a/xen/Makefile b/xen/Makefile index 5d72d82be260..80679b4a6b17 100644 --- 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 := +need-config := 1 + +ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),) + ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),) + need-config := + endif +endif + +ifneq ($(filter config %config,$(MAKECMDGOALS)),) + 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: ; + +%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 + +endif # need-config .PHONY: build install uninstall clean distclean MAP build install uninstall debug clean distclean MAP:: @@ -251,9 +315,6 @@ cscope: _MAP: $(NM) -n $(TARGET)-syms | grep -v '\(compiled\)\|\(\.o$$\)\|\( [aUw] \)\|\(\.\.ng$$\)\|\(LASH[RL]DI\)' > System.map -.PHONY: FORCE -FORCE: - %.o %.i %.s: %.c FORCE $(MAKE) -f $(BASEDIR)/Rules.mk -C $(*D) $(@F) @@ -274,25 +335,6 @@ $(foreach base,arch/x86/mm/guest_walk_% \ arch/x86/mm/shadow/guest_%, \ $(foreach ext,o i s,$(call build-intermediate,$(base).$(ext)))) -kconfig := oldconfig config menuconfig defconfig \ - nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig \ - randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig)) -.PHONY: $(kconfig) -$(kconfig): - $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@ - -include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG) - $(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig - -# 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 - -# Break the dependency chain for the first run -include/config/auto.conf.cmd: ; - --include $(BASEDIR)/include/config/auto.conf.cmd - .PHONY: cloc cloc: $(eval tmpfile := $(shell mktemp)) @@ -304,3 +346,11 @@ cloc: cloc --list-file=$(tmpfile) rm $(tmpfile) +endif #config-build + +PHONY += FORCE +FORCE: + +# Declare the contents of the PHONY variable as phony. We keep that +# information in a variable so we can use it in if_changed and friends. +.PHONY: $(PHONY)
We are going to generate the CFLAGS early from "xen/Makefile" instead of in "Rules.mk", but we need to include "config/auto.conf", so include it in "Makefile". Before including "config/auto.conf" we check which make target a user is calling, as some targets don't need "auto.conf". For targets that needs auto.conf, make will generate it (and a default .config if missing). root-make-done is to avoid doing the calculation again once Rules.mk takes over and is been executed with the root Makefile. When Rules.mk is including xen/Makefile, `config-build' and `need-config' are undefined so auto.conf will not be included again (it is already included by Rules.mk) and kconfig target are out of reach of Rules.mk. The way targets are filtered is inspireds by Kbuild, with some code imported from Linux. That's why there is PHONY variable that isn't used yet, for example. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/Makefile | 96 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 73 insertions(+), 23 deletions(-)