Closed Bug 1108750 Opened 7 years ago Closed 7 years ago

move unified compilation file combination logic out of recursivemake.py

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla38

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(8 files)

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.
Depends on: 1109302
Assignee: nobody → nfroyd
Blocks: 1122812
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.
Attachment #8551466 - Flags: review?(mshal)
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.
Attachment #8551468 - Flags: review?(mshal)
_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.
Attachment #8551469 - Flags: review?(mshal)
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.
Attachment #8551470 - Flags: review?(mshal)
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.
Attachment #8551471 - Flags: review?(mshal)
_add_unified_build_rules shouldn't be in the business of writing out
unified files.  Some outside, higher-up logic should be doing that.
Attachment #8551472 - Flags: review?(mshal)
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.
Attachment #8551473 - Flags: review?(mshal)
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.
Attachment #8551474 - Flags: review?(mshal)
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 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: ...'
Attachment #8551466 - Flags: review?(mshal) → review+
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 :)
Attachment #8551468 - Flags: review?(mshal) → review+
Attachment #8551469 - Flags: review?(mshal) → review+
Attachment #8551470 - Flags: review?(mshal) → review+
Attachment #8551471 - Flags: review?(mshal) → review+
Attachment #8551472 - Flags: review?(mshal) → review+
Attachment #8551473 - Flags: review?(mshal) → review+
Attachment #8551474 - Flags: review?(mshal) → review+
Blocks: 1129635
Depends on: 1127882
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.