Closed Bug 1454825 Opened 2 years ago Closed 2 years ago

Tup backend should handle dependent GENERATED_FILES better

Categories

(Firefox Build System :: General, enhancement)

3 Branch
enhancement
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 1 open bug)

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.
Looks like someone landed something relies on this before me, and Btup has been broken before I landed bug 1452542 :)
Yeah, that was a separate issue since the way xpidl was built has changed. There's a patch to fix it in bug 1454811.
Attachment #8969123 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
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+
(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.
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/559cd1e5c775
Tup backend: Handle dependent GENERATED_FILES better; r=chmanchester
https://hg.mozilla.org/mozilla-central/rev/559cd1e5c775
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.