1108750 - move unified compilation file combination logic out of recursivemake.py

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

I think jcranmer said the current positioning is kind of a pain, as it makes it difficult for alternate build backends to determine exactly what files are going to be built.  It's not immediately obvious to me how e.g. the Visual Studio backend handles this, but...

I think this involves, at a minimum, splitting out real backend objects for *SRCS, which now all live crowded on VariablePassthru.

Fixing this up for the WebIDL files and IPDL files is probably a bit challenging, so those bits may be punted down the road a bit.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: part 1 - move group_unified_files to mozbuild.util

RecursiveMakeBackend._group_unified_files doesn't contain any
functionality specific to the recursive make backend.  We would also
like to move the unification of generated IPDL and WebIDL source files
into the common build backend.  Moving _group_unified_files into the
common build backend would be the logical place for it, but the frontend
should also be able to handle unifying files so that backends don't have
to duplicate logic for UNIFIED_FILES.  Therefore, we choose to move it
to mozbuild.util as its final resting place.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 2 - move _write_unified_file to CommonBackend

Nothing about writing unified files is specific to the recursive make
backend, and if we want to write the unified files for IPDL and WebIDL
files, we'll need this functionality available in the common build
backend.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: part 3 - move unified source mapping to the callers of _add_unified_build_rules

_add_unified_build_rules shouldn't be in the business of determining how
to group files into their unified files.  That logic belongs in the
caller of _add_unified_build_rules.  Once that's done, the logic for
determining how to group files can migrate out of the recursive make
bakend.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: part 4 - remove handling of extra_dependencies from _add_unified_build_rules

No callers use it, and the makefile rules it generates don't really do
anything that the compiler won't already infer.  Let's get rid of it.

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: part 5 - move logic for writing all unified files into CommonBackend

We'll need to write out unified files for multiple backends, not just
the recursive make one.  Put that logic someplace where all build
backends can access it.

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: part 6 - move unified source writing to the callers of _add_unified_build_rules

_add_unified_build_rules shouldn't be in the business of writing out
unified files.  Some outside, higher-up logic should be doing that.

Comment 7

[Nathan Froyd [:froydnj]]

Attachment: part 7 - move IPDL unification logic to CommonBackend

After a bunch of tiny changes, we're finally ready to make real
progress.  We can now move the grouping of the generated IDPL C++ files
and the actual writing of the unified files for them into the common
build backend.  Derivative backends now only have to concern themselves
with adding the particular logic that compiling those files requires.

Comment 8

[Nathan Froyd [:froydnj]]

Attachment: part 8 - move WebIDL unification logic to CommonBackend

Similar to the changes made for IPDL files, this commit moves all of the
non-makefile related logic for WebIDL files out of the recursive make
backend and into the common build backend.  Derivative backends that
would like to do interesting things with WebIDL files now need to
implement _handle_webidl_build, which takes more parameters, but should
ideally require less duplication of logic.

Comment 9

[Nathan Froyd [:froydnj]]

This series doesn't touch the handling of UNIFIED_FILES yet.  I think it will need a similar subclass hook as parts 7 and 8 implement, but I thought I'd get this series out first and see if there was feedback on the general approach before doing that.

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8551466 [details] [diff] [review]
part 1 - move group_unified_files to mozbuild.util

>diff --git a/python/mozbuild/mozbuild/util.py b/python/mozbuild/mozbuild/util.py
>index 2ee4136..21d782c 100644
>--- a/python/mozbuild/mozbuild/util.py
>+++ b/python/mozbuild/mozbuild/util.py
>@@ -8,16 +8,17 @@
> from __future__ import unicode_literals
> 
> import collections
> import copy
> import difflib
> import errno
> import functools
> import hashlib
>+import itertools

I think you can now remove 'import itertools' from recursivemake.py

>+    def filter_out_dummy(iterable):
>+        return itertools.ifilter(lambda x: x != dummy_fill_value,
>+                                     iterable)

nit: the indent for 'iterable)' should be lined up with 'lambda x: ...'

Comment 11

[Michael Shal [:mshal]]

Comment on attachment 8551468 [details] [diff] [review]
part 2 - move _write_unified_file to CommonBackend

>diff --git a/python/mozbuild/mozbuild/backend/common.py b/python/mozbuild/mozbuild/backend/common.py
>index 73eae9c..ecde1c7 100644
>--- a/python/mozbuild/mozbuild/backend/common.py
>+++ b/python/mozbuild/mozbuild/backend/common.py
>@@ -25,16 +25,19 @@ from ..frontend.data import (
>     TestManifest,
>     TestWebIDLFile,
>     XPIDLFile,
>     WebIDLFile,
> )
> 
> from collections import defaultdict
> 
>+from ..util import (
>+    group_unified_files,
>+)

It doesn't look like this is actually needed until part 7, but meh :)

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6bda775985ab
https://hg.mozilla.org/mozilla-central/rev/a55bd91ead7d
https://hg.mozilla.org/mozilla-central/rev/e0a55a8b70bf
https://hg.mozilla.org/mozilla-central/rev/e80c66667c38
https://hg.mozilla.org/mozilla-central/rev/3951263969f8
https://hg.mozilla.org/mozilla-central/rev/d23cbb9c5271
https://hg.mozilla.org/mozilla-central/rev/a75da472515f
https://hg.mozilla.org/mozilla-central/rev/2c010ad45bd3