Closed Bug 1304129 Opened 3 years ago Closed 3 years ago

Support GENERATED_FILES in the tup backend

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

We can get almost all of the GENERATED_FILES now. There are a few things we need to support in order to get the rest (FinalTargetPreprocessedFiles, HostSimplePrograms, and libxul.so), but I think it's reasonable to just build in some exceptions for those at the moment.

This also lets us re-use the GENERATED_FILES rule for xpidllex.py / xpidlyacc.py, rather than having a custom rule in the tup backend.

Because of tup's dependency checking, I found a few cases where we are underspecifying dependencies in moz.build, or not doing what we're intending. I don't think any of them would necessarily cause issues in the make backend at present, but they will be good to fix nonetheless.
Depends on: 1304131
Depends on: 1304133
Depends on: 1304136
Comment on attachment 8793017 [details]
Bug 1304129 - Build GENERATED_FILES in the tup backend;

https://reviewboard.mozilla.org/r/79838/#review79166

::: python/mozbuild/mozbuild/backend/tup.py:134
(Diff revision 1)
> +                cmd = self._py_action('file_generate')
> +                cmd.extend([
> +                    obj.script,
> +                    obj.method,
> +                    obj.outputs[0],
> +                    '%s.pp' % obj.outputs[0], # deps file required
> +                ])
> +                full_inputs = [f.full_path for f in obj.inputs]
> +                cmd.extend(full_inputs)

I suspect this code is duplicated. Something to keep in the back of your head when you write this backend is how we can reduce code duplication between backends.

I think having functions in a shared Python module that do common work would be effective.
Attachment #8793017 - Flags: review?(gps) → review+
Comment on attachment 8793018 [details]
Bug 1304129 - Use xpidllex.py / xpidlyacc.py from GENERATED_FILES;

https://reviewboard.mozilla.org/r/79840/#review79178
Attachment #8793018 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #3)
> Comment on attachment 8793017 [details]
> Bug 1304129 - Build GENERATED_FILES in the tup backend;
> 
> https://reviewboard.mozilla.org/r/79838/#review79166
> 
> ::: python/mozbuild/mozbuild/backend/tup.py:134
> (Diff revision 1)
> > +                cmd = self._py_action('file_generate')
> > +                cmd.extend([
> > +                    obj.script,
> > +                    obj.method,
> > +                    obj.outputs[0],
> > +                    '%s.pp' % obj.outputs[0], # deps file required
> > +                ])
> > +                full_inputs = [f.full_path for f in obj.inputs]
> > +                cmd.extend(full_inputs)
> 
> I suspect this code is duplicated. Something to keep in the back of your
> head when you write this backend is how we can reduce code duplication
> between backends.
> 
> I think having functions in a shared Python module that do common work would
> be effective.

Good point. Would it make sense to try to compute the command-line in the emitter? Then objects like GeneratedFile could have the full command-line needed to build ready to go.
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/db7b0c3c23b7
Build GENERATED_FILES in the tup backend; r=gps
https://hg.mozilla.org/integration/autoland/rev/b000934f10d9
Use xpidllex.py / xpidlyacc.py from GENERATED_FILES; r=gps
https://hg.mozilla.org/mozilla-central/rev/db7b0c3c23b7
https://hg.mozilla.org/mozilla-central/rev/b000934f10d9
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.