Re: [LTP] [PATCH v3 3/3] build: Parallelize subdirectory recursion in generic_trunk_target

[Petr Vorel <pvorel@suse.cz>]

Hi Li, Andrea,

> >  ifdef VERBOSE
> > -	@set -e; for dir in $(SUBDIRS); do \
> > -	    $(MAKE) -C $$dir -f "$(abs_srcdir)/$$dir/Makefile" $@; \
> > -	done
> > +	$(MAKE) -C "$(patsubst %-all,%,$@)" -f "$(abs_srcdir)/$(patsubst %-all,%,$@)/Makefile" all
> >  else
> > -	@set -e; for dir in $(SUBDIRS); do \
> > -	    $(MAKE) --no-print-directory -C $$dir -f "$(abs_srcdir)/$$dir/Makefile" $@; \
> > -	done
> > +	@$(MAKE) --no-print-directory -C "$(patsubst %-all,%,$@)" -f "$(abs_srcdir)/$(patsubst %-all,%,$@)/Makefile" all
> > +endif
> > +
> > +$(addsuffix -install,$(SUBDIRS)): trunk-install | $(SUBDIRS) $(MAKE_DEPS)
> > +ifdef VERBOSE
> > +	$(MAKE) -C "$(patsubst %-install,%,$@)" -f "$(abs_srcdir)/$(patsubst %-install,%,$@)/Makefile" install
> > +else
> > +	@$(MAKE) --no-print-directory -C "$(patsubst %-install,%,$@)" -f "$(abs_srcdir)/$(patsubst %-install,%,$@)/Makefile" install
> > +endif
> > +
> > +$(addsuffix -check,$(SUBDIRS)): trunk-check | $(SUBDIRS) $(MAKE_DEPS)
> > +ifdef VERBOSE
> > +	$(MAKE) -C "$(patsubst %-check,%,$@)" -f "$(abs_srcdir)/$(patsubst %-check,%,$@)/Makefile" check
> > +else
> > +	@$(MAKE) --no-print-directory -C "$(patsubst %-check,%,$@)" -f "$(abs_srcdir)/$(patsubst %-check,%,$@)/Makefile" check
> >  endif

> Maybe we could refactor those lines to something like this:

> ifdef VERBOSE
> Q :=
> SUBMAKE := $(MAKE)
> else
> Q := @
> SUBMAKE := $(MAKE) --no-print-directory
> endif

> all:     $(addsuffix -all,$(SUBDIRS))
> install: $(addsuffix -install,$(SUBDIRS))
> check:   $(addsuffix -check,$(SUBDIRS))

> .PHONY: $(addsuffix -all,$(SUBDIRS)) $(addsuffix -install,$(SUBDIRS)) $(addsuffix -check,$(SUBDIRS))

> $(addsuffix -all,$(SUBDIRS)): %-all: trunk-all | $(SUBDIRS) $(MAKE_DEPS)
> 	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" all

> $(addsuffix -install,$(SUBDIRS)): %-install: trunk-install | $(SUBDIRS) $(MAKE_DEPS)
> 	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" install

> $(addsuffix -check,$(SUBDIRS)): %-check: trunk-check | $(SUBDIRS) $(MAKE_DEPS)
> 	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" check

Very good point.
BTW similar cleanup with variable could be done in include/mk/rules.mk.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

I try to test the patch once github works again.
https://www.githubstatus.com/

Kind regards,
Petr

diff --git include/mk/generic_trunk_target.inc include/mk/generic_trunk_target.inc
index 6975507e81..8ed8ee89c0 100644
--- include/mk/generic_trunk_target.inc
+++ include/mk/generic_trunk_target.inc
@@ -78,11 +78,14 @@ endif
 
 install: trunk-install
 
-# Print out CURDIR to check for a recursion issue.
-ifeq ($(strip $(SUBDIRS)),)
-	$(warning CURDIR is: $(CURDIR))
-	$(error SUBDIRS empty -- did you want generic_leaf_target instead?)
+ifdef VERBOSE
+Q :=
+SUBMAKE := $(MAKE)
 else
+Q := @
+SUBMAKE := $(MAKE) --no-print-directory
+endif
+
 # Recurse into subdirectories in parallel instead of using a serial shell loop.
 # Each subdirectory is mapped to a phony target (<dir>-all, <dir>-install, etc.).
 # GNU Make can then schedule sibling directories concurrently under -jN.
@@ -94,27 +97,14 @@ check:   $(addsuffix -check,$(SUBDIRS))
 
 # For each target (e.g. 'open-all'), strip the suffix to get the directory name,
 # build trunk-local targets first, and ensure the subdirectory exists.
-$(addsuffix -all,$(SUBDIRS)): trunk-all | $(SUBDIRS) $(MAKE_DEPS)
-ifdef VERBOSE
-	$(MAKE) -C "$(patsubst %-all,%,$@)" -f "$(abs_srcdir)/$(patsubst %-all,%,$@)/Makefile" all
-else
-	@$(MAKE) --no-print-directory -C "$(patsubst %-all,%,$@)" -f "$(abs_srcdir)/$(patsubst %-all,%,$@)/Makefile" all
-endif
+$(addsuffix -all,$(SUBDIRS)): %-all: trunk-all | $(SUBDIRS) $(MAKE_DEPS)
+	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" all
 
-$(addsuffix -install,$(SUBDIRS)): trunk-install | $(SUBDIRS) $(MAKE_DEPS)
-ifdef VERBOSE
-	$(MAKE) -C "$(patsubst %-install,%,$@)" -f "$(abs_srcdir)/$(patsubst %-install,%,$@)/Makefile" install
-else
-	@$(MAKE) --no-print-directory -C "$(patsubst %-install,%,$@)" -f "$(abs_srcdir)/$(patsubst %-install,%,$@)/Makefile" install
-endif
+$(addsuffix -install,$(SUBDIRS)): %-install: trunk-install | $(SUBDIRS) $(MAKE_DEPS)
+	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" install
 
-$(addsuffix -check,$(SUBDIRS)): trunk-check | $(SUBDIRS) $(MAKE_DEPS)
-ifdef VERBOSE
-	$(MAKE) -C "$(patsubst %-check,%,$@)" -f "$(abs_srcdir)/$(patsubst %-check,%,$@)/Makefile" check
-else
-	@$(MAKE) --no-print-directory -C "$(patsubst %-check,%,$@)" -f "$(abs_srcdir)/$(patsubst %-check,%,$@)/Makefile" check
-endif
-endif
+$(addsuffix -check,$(SUBDIRS)): %-check: trunk-check | $(SUBDIRS) $(MAKE_DEPS)
+	$(Q)$(SUBMAKE) -C "$*" -f "$(abs_srcdir)/$*/Makefile" check
 
 check: trunk-check
 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp