| Message ID | 20210701141011.785641-13-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements | expand |
On 01.07.2021 16:09, Anthony PERARD wrote: > --- a/xen/test/Makefile > +++ b/xen/test/Makefile > @@ -4,15 +4,10 @@ tests all: build > > ifneq ($(XEN_TARGET_ARCH),x86_32) > # Xen 32-bit x86 hypervisor no longer supported, so has no test livepatches > -SUBDIRS += livepatch > +subdir-y += livepatch > endif As per xen/Rules.mk having subdir-y := $(subdir-y) $(filter %/, $(obj-y)) obj-y := $(patsubst %/, %/built_in.o, $(obj-y)) ... subdir-obj-y := $(filter %/built_in.o, $(obj-y)) this will result in building of livepatch/built_in.o afaict. Is this an intended but benign side effect? > install build subtree-force-update uninstall: %: > - set -e; for s in $(SUBDIRS); do \ > + set -e; for s in $(subdir-y); do \ > $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $*; \ > done > - > -clean:: > - set -e; for s in $(SUBDIRS); do \ > - $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $@; \ > - done And then why can't the generic recursion rule in xen/Rules.mk not also be used for the "build" target? (I guess "install" and "uninstall" need to remain separate, and don't think I know what "subtree-force-update" is about.) Jan
On Wed, Jul 07, 2021 at 05:26:13PM +0200, Jan Beulich wrote: > On 01.07.2021 16:09, Anthony PERARD wrote: > > --- a/xen/test/Makefile > > +++ b/xen/test/Makefile > > @@ -4,15 +4,10 @@ tests all: build > > > > ifneq ($(XEN_TARGET_ARCH),x86_32) > > # Xen 32-bit x86 hypervisor no longer supported, so has no test livepatches > > -SUBDIRS += livepatch > > +subdir-y += livepatch > > endif > > As per xen/Rules.mk having > > subdir-y := $(subdir-y) $(filter %/, $(obj-y)) > obj-y := $(patsubst %/, %/built_in.o, $(obj-y)) > ... > subdir-obj-y := $(filter %/built_in.o, $(obj-y)) > > this will result in building of livepatch/built_in.o afaict. Is > this an intended but benign side effect? Actually, nothing in Rules.mk is using $(subdir-y) other than updating it with possible subdir from $(obj-y). Recursion into a subdir only happen if it is listed in $(obj-y) and thus should be part of a built_in.o. Rules.mk doesn't have a mean to recurs otherwise. So nothing is actually going to try to build livepatch/build_in.o due to $(subdir-y). > > install build subtree-force-update uninstall: %: > > - set -e; for s in $(SUBDIRS); do \ > > + set -e; for s in $(subdir-y); do \ > > $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $*; \ > > done > > - > > -clean:: > > - set -e; for s in $(SUBDIRS); do \ > > - $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $@; \ > > - done > > And then why can't the generic recursion rule in xen/Rules.mk > not also be used for the "build" target? (I guess "install" and > "uninstall" need to remain separate, and don't think I know what > "subtree-force-update" is about.) There's some more changed in a later patch[1] to Rules.mk which would make it possible to remove the need for a "build" target and I actually remove the "build" target as well as the "subtree-force-update" target. Some more changes in tests/livepatch/ are done in patch[2] which actually allow to remove the "build" target. [1] build: build everything from the root dir, use obj=$subdir [2] build: rework test/livepatch/Makefile I think the "subtree-force-update" target as to do with having the same logic to deal with $(SUBDIRS) as the logic in tools/ and the top makefile, but might not have been actually hooked up. Cheers,
On 12.07.2021 17:22, Anthony PERARD wrote: > On Wed, Jul 07, 2021 at 05:26:13PM +0200, Jan Beulich wrote: >> On 01.07.2021 16:09, Anthony PERARD wrote: >>> --- a/xen/test/Makefile >>> +++ b/xen/test/Makefile >>> @@ -4,15 +4,10 @@ tests all: build >>> >>> ifneq ($(XEN_TARGET_ARCH),x86_32) >>> # Xen 32-bit x86 hypervisor no longer supported, so has no test livepatches >>> -SUBDIRS += livepatch >>> +subdir-y += livepatch >>> endif >> >> As per xen/Rules.mk having >> >> subdir-y := $(subdir-y) $(filter %/, $(obj-y)) >> obj-y := $(patsubst %/, %/built_in.o, $(obj-y)) >> ... >> subdir-obj-y := $(filter %/built_in.o, $(obj-y)) >> >> this will result in building of livepatch/built_in.o afaict. Is >> this an intended but benign side effect? > > Actually, nothing in Rules.mk is using $(subdir-y) other than updating > it with possible subdir from $(obj-y). > Recursion into a subdir only happen if it is listed in $(obj-y) and thus > should be part of a built_in.o. Rules.mk doesn't have a mean to recurs > otherwise. > > So nothing is actually going to try to build livepatch/build_in.o due to > $(subdir-y). Question then is: Isn't this actually a bug? >>> install build subtree-force-update uninstall: %: >>> - set -e; for s in $(SUBDIRS); do \ >>> + set -e; for s in $(subdir-y); do \ >>> $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $*; \ >>> done >>> - >>> -clean:: >>> - set -e; for s in $(SUBDIRS); do \ >>> - $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $@; \ >>> - done >> >> And then why can't the generic recursion rule in xen/Rules.mk >> not also be used for the "build" target? (I guess "install" and >> "uninstall" need to remain separate, and don't think I know what >> "subtree-force-update" is about.) > > There's some more changed in a later patch[1] to Rules.mk which would > make it possible to remove the need for a "build" target and I actually > remove the "build" target as well as the "subtree-force-update" target. > Some more changes in tests/livepatch/ are done in patch[2] which > actually allow to remove the "build" target. > > [1] build: build everything from the root dir, use obj=$subdir > [2] build: rework test/livepatch/Makefile > > I think the "subtree-force-update" target as to do with having the same > logic to deal with $(SUBDIRS) as the logic in tools/ and the top > makefile, but might not have been actually hooked up. Okay, I guess I need to get further through the series to see the final effects. Jan
On Tue, Jul 13, 2021 at 09:41:03AM +0200, Jan Beulich wrote: > On 12.07.2021 17:22, Anthony PERARD wrote: > > On Wed, Jul 07, 2021 at 05:26:13PM +0200, Jan Beulich wrote: > >> On 01.07.2021 16:09, Anthony PERARD wrote: > >>> --- a/xen/test/Makefile > >>> +++ b/xen/test/Makefile > >>> @@ -4,15 +4,10 @@ tests all: build > >>> > >>> ifneq ($(XEN_TARGET_ARCH),x86_32) > >>> # Xen 32-bit x86 hypervisor no longer supported, so has no test livepatches > >>> -SUBDIRS += livepatch > >>> +subdir-y += livepatch > >>> endif > >> > >> As per xen/Rules.mk having > >> > >> subdir-y := $(subdir-y) $(filter %/, $(obj-y)) > >> obj-y := $(patsubst %/, %/built_in.o, $(obj-y)) > >> ... > >> subdir-obj-y := $(filter %/built_in.o, $(obj-y)) > >> > >> this will result in building of livepatch/built_in.o afaict. Is > >> this an intended but benign side effect? > > > > Actually, nothing in Rules.mk is using $(subdir-y) other than updating > > it with possible subdir from $(obj-y). > > Recursion into a subdir only happen if it is listed in $(obj-y) and thus > > should be part of a built_in.o. Rules.mk doesn't have a mean to recurs > > otherwise. > > > > So nothing is actually going to try to build livepatch/build_in.o due to > > $(subdir-y). > > Question then is: Isn't this actually a bug? No, "obj-y += livepatch/" need to be used instead, to build "livepatch/built_in.o". Because "$(obj-y)" are objects to be included in a "built_in.o".
diff --git a/xen/test/Makefile b/xen/test/Makefile index aaa499664396..41e4d7bdb78b 100644 --- a/xen/test/Makefile +++ b/xen/test/Makefile @@ -4,15 +4,10 @@ tests all: build ifneq ($(XEN_TARGET_ARCH),x86_32) # Xen 32-bit x86 hypervisor no longer supported, so has no test livepatches -SUBDIRS += livepatch +subdir-y += livepatch endif install build subtree-force-update uninstall: %: - set -e; for s in $(SUBDIRS); do \ + set -e; for s in $(subdir-y); do \ $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $*; \ done - -clean:: - set -e; for s in $(SUBDIRS); do \ - $(MAKE) -f $(BASEDIR)/Rules.mk -C $$s $@; \ - done
This allows Makefile.clean to recurse into livepatch without help. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/test/Makefile | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)