1270229 - Refactor xpidl directory traversal

Status: NEW

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1270229 - Add directory dependency on idl-parser; r?glandium

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/

Comment 2

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1270229 - Process dist/idl manifest in actual xpidl Makefile; r?glandium

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/

Comment 3

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1270229 - Remove dist/idl install manifest processing from root Makefile; r?glandium

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 4

[Mike Hommey [:glandium]]

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 5

[Mike Hommey [:glandium]]

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 6

[Mike Hommey [:glandium]]

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.

Comment 7

[Gregory Szorc [:gps]]

I'm not actively working on this.