Everything related to XPIDL directory traversal is horrible. As part of working on bug 1253110 XPIDL make foo is causing me excessive pain. So I'm just going to inject a little more sanity into the chaos.

SUBMAKE is somewhat evil. Eliminate the SUBMAKE for generating idl-parser files by properly adding a directory traversal dependency to ensure the idl-parser files are generated before processing XPIDL files. This possibly also eliminates a race condition where xpcom/idl-parser/xpidl:export could get processes at the same time. Review commit: https://reviewboard.mozilla.org/r/50537/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50537/

Currently, we process the dist/idl install manifest both in the top-level Makefile.in and inside xpcom/xpidl/Makefile.in:export. I'm intent on removing the redundancy and on removing xpcom/xpidl/Makefile. In preparation for this, move the install manifest processing invocation to config/makefiles/xpidl/Makefile.in. This required a new make target and a make reinvocation because make sucks. This is the same as before, just all inside the same Makefile.in now. Review commit: https://reviewboard.mozilla.org/r/50539/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50539/

Before this commit, both the root Makefile and config/makefiles/xpidl/Makefile were processing the dist/idl install manifest. This was redundant. This patches eliminates the redundancy by removing the dist/idl install manifest processing in the root Makefile. A nice benefit of this is some one-off hacks in the faster backend have been eliminated. Review commit: https://reviewboard.mozilla.org/r/50541/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50541/

Comment on attachment 8748809 [details] MozReview Request: Bug 1270229 - Add directory dependency on idl-parser; r?glandium https://reviewboard.mozilla.org/r/50537/#review48339

Comment on attachment 8748810 [details] MozReview Request: Bug 1270229 - Process dist/idl manifest in actual xpidl Makefile; r?glandium https://reviewboard.mozilla.org/r/50539/#review48347 ::: config/makefiles/xpidl/Makefile.in:79 (Diff revision 1) > > -xpidl:: $(xpt_files) $(chrome_manifests) $(interfaces_manifests) > +# We can't have the install manifest processing as an actual dependency of > +# the xpts because it is .PHONY and would cause the target to always be out > +# of date. So we have make reinvoke itself with another rule. > +.PHONY: xpidl > +xpidl:: I'd rather you didn't touch top-level Makefile, and "simply" remove the manual install manifest processing from xpcom/xpidl. Why? Because: - your change is going to break mach build faster - on a recursive build, the dist_idl install manifest processing happens during pre-export, so everything in the export tier is guaranteed to happen after - on a mach build faster build, we have the right dependencies making the dist_idl install manifest processed before xpidl (and xpcom/xpidl is happily ignored) - the only thing that actually needs this manual dist_idl install manifest processing is the NONRECURSIVE_TARGETS processing. And the dependency rules for that could be changed so that the rules from the top-level Makefile are used instead of those from xpcom/xpidl: NONRECURSIVE_TARGETS_export_xpidl_DIRECTORY = $(DEPTH) NONRECURSIVE_TARGETS_export_xpidl_TARGETS = install-dist_idl

Comment on attachment 8748811 [details] MozReview Request: Bug 1270229 - Remove dist/idl install manifest processing from root Makefile; r?glandium https://reviewboard.mozilla.org/r/50541/#review48349 ::: config/faster/rules.mk:101 (Diff revision 1) > -# The xpidl target in config/makefiles/xpidl requires the install manifest for > -# dist/idl to have been processed. When using the hybrid > -# FasterMake/RecursiveMake backend, this dependency is handled in the top-level > +# The xpidl target in config/makefiles/xpidl all the install manifests for > +# dist/bin to have been processed because it adds interfaces.manifest > +# references with buildlist.py. This breaks running mach build faster when it's not part of artifact builds.

I'm not actively working on this.