Closed
Bug 1454825
Opened 7 years ago
Closed 7 years ago
Tup backend should handle dependent GENERATED_FILES better
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: mshal, Assigned: mshal)
References
Details
Attachments
(1 file)
Tup currently doesn't handle commands in a directory that are out of order, so we have some hacks for certain GENERATED_FILES to delay outputting them. Ideally tup would just support this, but in the meantime we can handle this better by waiting to create the rule until the ObjDirPath inputs from the current directory are all emitted.
Comment 1•7 years ago
|
||
Looks like someone landed something relies on this before me, and Btup has been broken before I landed bug 1452542 :)
Assignee | ||
Comment 2•7 years ago
|
||
Yeah, that was a separate issue since the way xpidl was built has changed. There's a patch to fix it in bug 1454811.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8969123 -
Flags: review?(core-build-config-reviews) → review?(cmanchester)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8969123 [details]
Bug 1454825 - Tup backend: Handle dependent GENERATED_FILES better;
https://reviewboard.mozilla.org/r/237832/#review243556
::: python/mozbuild/mozbuild/backend/tup.py:183
(Diff revision 1)
> + def requires_delay(self, inputs):
> + # We need to delay the generated file rule in the Tupfile until the
> + # generated inputs in the current directory are processed. We do this by
> + # checking all ObjDirPaths to make sure they are in
> + # self.outputs, or are in other directories.
> + return any(isinstance(f, ObjDirPath) and f.target_basename not in self.outputs and mozpath.dirname(f.full_path) == self.objdir for f in inputs)
It would be nicer for find a way to split this across multiple lines, maybe by replacing the `any` with a `for` loop.
::: python/mozbuild/mozbuild/backend/tup.py:432
(Diff revision 1)
> + # Check if we can now handle any more generated files that were delayed
> + unhandled = []
> + for obj in backend_file.delayed_generated_files:
> + if backend_file.requires_delay(obj.inputs):
> + unhandled.append(obj)
> + else:
> + self._process_generated_file(backend_file, obj)
> + backend_file.delayed_generated_files = unhandled
Remove this if you determine you can, as discussed.
Attachment #8969123 -
Flags: review?(cmanchester) → review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Chris Manchester (:chmanchester) from comment #4)
> > + def requires_delay(self, inputs):
> > + # We need to delay the generated file rule in the Tupfile until the
> > + # generated inputs in the current directory are processed. We do this by
> > + # checking all ObjDirPaths to make sure they are in
> > + # self.outputs, or are in other directories.
> > + return any(isinstance(f, ObjDirPath) and f.target_basename not in self.outputs and mozpath.dirname(f.full_path) == self.objdir for f in inputs)
>
> It would be nicer for find a way to split this across multiple lines, maybe
> by replacing the `any` with a `for` loop.
Done.
>
> ::: python/mozbuild/mozbuild/backend/tup.py:432
> (Diff revision 1)
> > + # Check if we can now handle any more generated files that were delayed
> > + unhandled = []
> > + for obj in backend_file.delayed_generated_files:
> > + if backend_file.requires_delay(obj.inputs):
> > + unhandled.append(obj)
> > + else:
> > + self._process_generated_file(backend_file, obj)
> > + backend_file.delayed_generated_files = unhandled
>
> Remove this if you determine you can, as discussed.
Yeah, it turns out this was completely unnecessary.
Comment hidden (mozreview-request) |
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/559cd1e5c775
Tup backend: Handle dependent GENERATED_FILES better; r=chmanchester
Comment 8•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
Version: Version 3 → 3 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•