Closed Bug 466486 Opened 13 years ago Closed 12 years ago
Don't use a subshell to recurse over DIRS when DIRS is empty
While researching build time, I discovered that we're using a subshell to recurse over subdirectories even when there aren't any. This is wasteful, especially on Windows where processes cost a lot.
Attachment #349760 - Flags: review?(ted.mielczarek)
Comment on attachment 349760 [details] [diff] [review] Avoid shells for recursing empty DIRS, rev. 1 Do we ever wind up with DIRS set to an empty space? Couldn't we just use |ifdef DIRS| here? Also, amazing what silliness you can find with a little research!
Attachment #349760 - Flags: review?(ted.mielczarek) → review+
I found at least one case with something like: DIRS += \ $(EMPTYVALUE) \ $(NULL) oh, for make variables that are lists and not scalars ;-)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
this broke x86_64/linux builds
backed out... weird regressions with some directories not getting built
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 349760 [details] [diff] [review] Avoid shells for recursing empty DIRS, rev. 1 >- @$(EXIT_ON_ERROR) \ >- $(foreach dir,$(STATIC_DIRS),$(MAKE) -C $(dir); ) true >+ +$(LOOP_OVER_STATIC_DIRS) >-ifdef TOOL_DIRS >- @$(EXIT_ON_ERROR) \ >- $(foreach dir,$(TOOL_DIRS),$(UPDATE_TITLE) $(MAKE) -C $(dir) libs; ) true >-endif >+ +$(LOOP_OVER_TOOL_DIRS) Unfortunately $(LOOP_OVER_XXX_DIRS) uses $@ and so these aren't the same...
In this version, you aren't allowed to modify DIRS after you've included rules.mk... it would have already caused problems, such as missing submakefile dependencies. Unfortunately I can't think of a good way to enforce this rule, but oh well... This also fixes misuse of LOOP_OVER_STATIC_DIRS and LOOP_OVER_TOOL_DIRS
Looks like both browser/Makefile.in and mail/Makefile.in muck with DIRS after including rules.mk.
That turns out not to be a problem in practice (because DIRS was already set to something), but I've patched it.
Attachment #353702 - Flags: review?(ted.mielczarek) → review+
Second time's the charm: http://hg.mozilla.org/mozilla-central/rev/d11ccfc1d754
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment on attachment 353702 [details] [diff] [review] Avoid shells for recursing empty DIRS, rev. 2.1 a191=beltzner
Attachment #353702 - Flags: approval1.9.1? → approval1.9.1+
You need to log in before you can comment on or make changes to this bug.