On 24.08.2021 12:50, Anthony PERARD wrote: > This implement out-of-tree support, there's two ways to create an > out-of-tree build tree (after that, `make` in that new directory > works): > make O=build > mkdir build; cd build; make -f ../Makefile > also works with an absolute path for both. > > This implementation only works if the source tree is clean, as we use > VPATH. > > This patch copies most new code with handling out-of-tree build from > Linux v5.12. > > Signed-off-by: Anthony PERARD <anthony.per...@citrix.com> > --- > xen/Makefile | 173 +++++++++++++++++++++++++++++------- > xen/Rules.mk | 11 ++- > xen/arch/arm/efi/Makefile | 3 + > xen/arch/x86/arch.mk | 3 + > xen/arch/x86/boot/Makefile | 5 +- > xen/arch/x86/efi/Makefile | 3 + > xen/include/Makefile | 9 +- > xen/scripts/mkmakefile | 17 ++++ > xen/test/livepatch/Makefile | 2 + > xen/xsm/flask/Makefile | 3 + > xen/xsm/flask/ss/Makefile | 3 + > 11 files changed, 194 insertions(+), 38 deletions(-) > create mode 100755 xen/scripts/mkmakefile
Linux have done away with this script just recently; I don't think we should introduce it. > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -1,3 +1,7 @@ > +# $(lastword,) for GNU Make older than 0.81 DYM 3.81? > +lastword = $(word $(words $(1)),$(1)) > +this-makefile := $(call lastword,$(MAKEFILE_LIST)) Oh - is it documented somewhere that this would always be last? > @@ -17,33 +21,18 @@ export XEN_BUILD_HOST ?= $(shell hostname) > PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 > 2>/dev/null) python) > export PYTHON ?= $(PYTHON_INTERPRETER) > > -export XEN_ROOT := $(CURDIR)/.. > +$(if $(filter __%, $(MAKECMDGOALS)), \ > + $(error targets prefixed with '__' are only for internal use)) > > -srctree := . > -objtree := . > -export srctree objtree > +# That's our default target when none is given on the command line > +PHONY := __all > +__all: > > # Do not use make's built-in rules and variables > MAKEFLAGS += -rR > > EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi > > -ARCH=$(XEN_TARGET_ARCH) > -SRCARCH=$(shell echo $(ARCH) | \ > - sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ > - -e s'/riscv.*/riscv/g') > -export ARCH SRCARCH > - > -# Don't break if the build process wasn't called from the top level > -# we need XEN_TARGET_ARCH to generate the proper config > -include $(XEN_ROOT)/Config.mk > - > -# Set ARCH/SUBARCH appropriately. > -export TARGET_SUBARCH := $(XEN_TARGET_ARCH) > -export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \ > - sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' > \ > - -e s'/riscv.*/riscv/g') > - > # Allow someone to change their config file > export KCONFIG_CONFIG ?= .config Would it be possible to split off some of the pure movement of pieces, to make it easier to see what (if anything) is actually changed at the same time as getting moved? > @@ -51,14 +40,9 @@ export CC CXX LD > > export TARGET := xen > > -.PHONY: default > -default: build > - > .PHONY: dist > dist: install > > -include scripts/Kbuild.include > - > ifneq ($(root-make-done),y) > # section to run before calling Rules.mk, but only once. > > @@ -130,14 +114,93 @@ endif > > ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),) > quiet := silent_ > + KBUILD_VERBOSE = 0 > endif In how far is this related here? Doesn't this belong in an earlier patch? > export quiet Q KBUILD_VERBOSE > > +# $(realpath,) for GNU Make older than 0.81 Again - 3.81? > +ifeq ($(abs_srctree),$(abs_objtree)) > + # building in the source tree > + srctree := . > + building_out_of_srctree := > +else > + ifeq ($(abs_srctree)/,$(dir $(abs_objtree))) > + # building in a subdirectory of the source tree > + srctree := .. > + else > + srctree := $(abs_srctree) > + endif > + building_out_of_srctree := 1 > +endif > + > +objtree := . > +VPATH := $(srctree) Would you mind using blank padding here, and perhaps fewer blanks than the tabs are worth? > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -38,7 +38,7 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \ > $(foreach r,rel rel.ro,data.$(r).local) > > # The filename build.mk has precedence over Makefile > -mk-dir := $(src) > +mk-dir := $(srctree)/$(src) > include $(if $(wildcard > $(mk-dir)/build.mk),$(mk-dir)/build.mk,$(mk-dir)/Makefile) Perhaps already when it was changed to $(src) the name has become slightly misleading, at least imo: I would rather expect a variable with this name to refer to the build dir/tree. Maybe "srcdir" or even shorted "sd" right from the start? (Reaching here I can finally see why having a shorthand is helpful.) > --- a/xen/arch/x86/efi/Makefile > +++ b/xen/arch/x86/efi/Makefile > @@ -1,5 +1,8 @@ > CFLAGS-y += -fshort-wchar > CFLAGS-y += -I$(srctree)/common/efi > +ifdef building_out_of_srctree > +CFLAGS-y += -I$(srctree)/$(src) > +endif At the example of this (where perhaps -iquote could be used again) - is it strictly necessary to have the ifdef-s around such? I.e. would things fail to work for an in-tree build without them? > --- a/xen/include/Makefile > +++ b/xen/include/Makefile > @@ -110,7 +110,7 @@ $(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) > $(src)/Makefile > $(foreach i, $(filter %.h,$^), \ > echo "#include "\"$(i)\" \ > | $(CC) -x c -std=c99 -Wall -Werror \ > - -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \ > + -include stdint.h $(foreach j, $($(patsubst > $(srctree)/%,%,$i)-prereq), -include $(j).h) \ Please split the $(foreach ...) from the earlier -include. > @@ -124,8 +124,8 @@ $(obj)/headers++.chk: $(PUBLIC_HEADERS) $(src)/Makefile > $(foreach i, $(filter %.h,$^), \ > echo "#include "\"$(i)\" \ > | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \ > - -include stdint.h -include $(src)/public/xen.h \ > - $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \ > + -include stdint.h -include $(srctree)/$(src)/public/xen.h \ > + $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include > c$(j)) -S -o /dev/null - \ Similarly here please split -S ... onto the next line. > --- a/xen/xsm/flask/ss/Makefile > +++ b/xen/xsm/flask/ss/Makefile > @@ -9,3 +9,6 @@ obj-y += conditional.o > obj-y += mls.o > > CFLAGS-y += -I$(srctree)/xsm/flask/include > +ifdef building_out_of_srctree > + CFLAGS-y += -I$(objtree)/xsm/flask/include There's no header in $(srctree)/xsm/flask/include in this case, so if you use "ifdef" here, shouldn't that other part move into an "else"? Jan
