Closed Bug 1350810 Opened 5 years ago Closed 5 years ago

Have build machines upload generated binding files as artifacts for stylo builds on try

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Stylo])

Attachments

(1 file, 1 obsolete file)

I think it would help a lot if stylo try build can upload generated binding files as artifacts. We can use the generated files there to update the in-tree binding files.

We have two issues for our current practice which relies on developer themselves to update the binding files:
1. Binding files generated from different platforms can vary significantly, which makes the files hard to review, and the history hard to audit.
2. Developers may need to run a separate Servo geckolib build to generate the files.

The first issue is the main motivation for me to propose this, actually. The binding files generated on Windows are much different than those generated from other two platforms, so I'm always hesitated to update binding files. When I really need to, I either manual edit the in-tree files, or switch to my Mac laptop, both of which are unproductive.

If we can have the build machine upload the generated binding files, we would have a single source of binding files, which makes updating the in-tree copy easier.

The generated binding files are in objdir/toolkit/library/{target}/debug/build/style-*/out/gecko. We can just package this directory as a new artifact.
Ted, is this a one-liner or something more involved?
Depends on: stylo-tooling
Flags: needinfo?(ted)
Summary: stylo: Have build machines upload generated binding files as artifacts for stylo builds on try → Have build machines upload generated binding files as artifacts for stylo builds on try
Whiteboard: [Stylo]
More than one, but if you just want to get a tarball out of the build you need to:
1) Generate it using a rule that's a dependency of make-package: https://dxr.mozilla.org/mozilla-central/rev/9577ddeaafd85554c2a855f385a87472a089d5c0/toolkit/mozapps/installer/packager.mk#91
2) Mention it in UPLOAD_FILES: https://dxr.mozilla.org/mozilla-central/rev/9577ddeaafd85554c2a855f385a87472a089d5c0/toolkit/mozapps/installer/upload-files.mk#397
Flags: needinfo?(ted)
Thanks for the pointer. I guess I can try to add it myself, since I'm probably the one who wants this the most.
Assignee: nobody → xidorn+moz
Try push with this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=84a6681e6b556a670a956b8fcd4d56fec0eac534&selectedJob=87135509

There is a
> artifact uploaded: target.stylo-bindings.zip
shown in the Job Details now.
No longer depends on: stylo-tooling
Comment on attachment 8852281 [details]
Bug 1350810 part 1 - Output binding files to dist dir in addition.

https://reviewboard.mozilla.org/r/124504/#review127146
Attachment #8852281 - Flags: review?(emilio+bugs) → review+
I would say this really makes my life much better.
Comment on attachment 8852282 [details]
Bug 1350810 - Package and upload stylo binding files as artifact.

https://reviewboard.mozilla.org/r/124506/#review127616

I assume we'll be able to remove this once we have bindgen working as part of the build and no longer need to have checked-in bindings?
Attachment #8852282 - Flags: review?(ted) → review+
We have bindgen working as part of the build... in stylo build. But Servo's CI still need the checked-in bindings for the geckolib build there...
Attachment #8852281 - Attachment is obsolete: true
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0d1d8024cc28
Package and upload stylo binding files as artifact. r=ted
https://hg.mozilla.org/mozilla-central/rev/0d1d8024cc28
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.