Closed Bug 860957 Opened 10 years ago Closed 9 years ago

Support partial tree builds with mixed recursive and non-recursive make files


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: gps, Assigned: gps)




(1 file)

The general plan is to start creating non-recursive make files once things move to files (see bug 850380 for likely the first victim - IDLs).

While non-recursive make files will result in faster builds, they will be disruption to existing build workflows. For years people have typed e.g. |make -C dom/foo| to build just dom/foo. |mach build| is effectively syntactic sugar wrapping that (at least for now).

If we were to blindly generate non-recursive make files [that are only invoked by the top-level Makefile], this would break partial tree builds for people who have come to rely on them. i.e. |make -C foo| would now only build recursive-defined targets (how it's currently done). All the non-recursive targets would not be built because, well, they are no longer defined in leaf Makefile (they are defined in non-recursive make files).

I'm going to assert that if we break |make -C foo| there will be general anarchy among developers. So, how do we preserve |make -C foo| while still enabling faster builds [via use of non-recursive make]?

One option is to write out the non-recursive make files at time and then have each leaf Makefile evaluate its "own" targets in those files as part of Makefile recursion. I don't like this option because you have the overhead of invoking multiple make files in each directory. And, you likely don't get the best speed gains possible because you are only evaluating the non-recursive rules a few at a time instead of all at once (with as large of a -j value as you can throw at it).

Here is what I'm thinking.

For top-level builds the top-level Makefile will call out to the non-recursive make files at the appropriate time (we are planning to do this anyway I think). During recursion, no target from a non-recursive make file is evaluated (at least ideally). However, the auto-generated files contain a "pointer" back to the non-recursive make files that are relevant for that directory. If |make -C foo| is invoked, the build system sees the current build is a partial tree build and "pulls in" the non-recursive targets for consideration. The leaf Makefile don't have to maintain any rules. So, the rules in the non-recursive make files are always authoritative.

A similar proposal is one where a partial tree build invokes all the non-recursive targets at the top of the build (or in each directory), just like a top-level build. The downside to this is that partial tree builds will likely take a little bit longer (since the set of evaluated targets is much larger). But, it does avoid the issue where partial tree builds get the build in a weird state. That being said, I consider partial tree building a necessarily evil that comes with many footguns and is necessary until full, no-op builds complete in a second or two. If people get the tree in a weird state by not building the entire thing, that's their problem and I don't think we should be worried about this.

In this bug, I'm proposing we add a variable to the make files to denote whether we are in a top-level or partial tree build. Said variable will be exported to child make files as part of recursion.

I've implemented what I described in the initial comment. I /think/ it has all the features needed to land parallel IDL compilation.
Assignee: nobody → gps
Attachment #738158 - Flags: review?(mh+mozilla)
Comment on attachment 738158 [details] [diff] [review]
Support non-recursive target execution in partial builds, v1

Review of attachment 738158 [details] [diff] [review]:

::: js/src/
@@ +7,5 @@
>  topsrcdir	= @top_srcdir@
>  srcdir		= @srcdir@

You want to export, here
Attachment #738158 - Flags: review?(mh+mozilla) → review+
Blocks: 848530
No longer blocks: 848530
Closed: 9 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.