Closed Bug 1218042 Opened 7 years ago Closed 7 years ago

CompileDB backend doesn't include DOM bindings or IPDL sources

Categories

(Firefox Build System :: General, defect)

43 Branch
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(2 files, 1 obsolete file)

This is another symptom of a generic problem that we have: derived backends often don't call CommonBackend.consume_object (and so miss out on things like webidl sources), or call CommonBackend.consume_finalize (and so miss out on being notified of what to do with webidl generated files).  We're missing something important in how we structure our backends, and I'd like to fix it.
The only thing we need the obj for here is getting the objdir.  Future
patches will just have an objdir when calling this function, and not a
proper mozbuild object.  In light of these facts, let's change the
function to accept an objdir only, which will make those future patches
easier.
Attachment #8678900 - Flags: review?(mshal)
Calling CommonBackend.consume_object ensures that we process WebIDL and
IPDL files (and many other things) correctly.  Calling
CommonBackend.consume_finished ensures that the CompileDB backend gets
to see the unified bindings and protocol files that we generate, and add
those files to the compilation database.
Attachment #8678901 - Flags: review?(mshal)
Attachment #8678900 - Flags: review?(mshal) → review+
Comment on attachment 8678901 [details] [diff] [review]
part 2 - make the CompileDB backend follow the backend protocol

>     def consume_object(self, obj):
>+        consumed = CommonBackend.consume_object(self, obj)
>+
>+        if consumed:
>+            return True
>+
>         if isinstance(obj, UnifiedSources):
>             # For unified sources, only include the unified source file.
>             # Note that unified sources are never used for host sources.
>             for f in obj.unified_source_mapping:
>                 flags = self._get_dir_flags(obj.objdir)
>                 self._build_db_line(obj.objdir, self.environment, f[0],
>                                     obj.canonical_suffix, flags, False)

Since CommonBackend.consume_object() returns True when obj is a UnifiedSources(), I think this code block can go away. At least it doesn't get executed now when I run 'mach build-backend --backend=CompileDB'. Was that the intention of adding 'if consumed: return True'?
Attachment #8678901 - Flags: review?(mshal) → feedback+
Ah, thanks for pointing that out.  Fortunately, we have a hook for that.
Attachment #8679544 - Flags: review?(mshal)
Attachment #8678901 - Attachment is obsolete: true
Comment on attachment 8679544 [details] [diff] [review]
part 2 - make the CompileDB backend follow the backend protocol

Looks good!

>+    def _process_unified_sources(self, obj):
>+        # For unified sources, only include the unified source file.
>+        # Note that unified sources are never used for host sources.
>+        for f in obj.unified_source_mapping:
>+            flags = self._get_dir_flags(obj.objdir)
>+            self._build_db_line(obj.objdir, self.environment, f[0],
>+                                obj.canonical_suffix, flags, False)
>+        ***

nit: random whitespace on this line.
Attachment #8679544 - Flags: review?(mshal) → review+
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/5fea785542c2
https://hg.mozilla.org/mozilla-central/rev/39a137bbfeee
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.