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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Xidorn Quan [:xidorn] UTC+10]

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.

Comment 1

[Bobby Holley (:bholley)]

Ted, is this a one-liner or something more involved?

Comment 2

[(not currently active) Ted Mielczarek]

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

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Thanks for the pointer. I guess I can try to add it myself, since I'm probably the one who wants this the most.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1350810 part 1 - Output binding files to dist dir in addition.

Review commit: https://reviewboard.mozilla.org/r/124504/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124504/

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1350810 - Package and upload stylo binding files as artifact.

Review commit: https://reviewboard.mozilla.org/r/124506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124506/

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

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.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8852281 [details]
Bug 1350810 part 1 - Output binding files to dist dir in addition.

https://reviewboard.mozilla.org/r/124504/#review127146

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

I would say this really makes my life much better.

Comment 9

[(not currently active) Ted Mielczarek]

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?

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

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...

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8852282 [details]
Bug 1350810 - Package and upload stylo binding files as artifact.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124506/diff/1-2/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d1d8024cc28
Package and upload stylo binding files as artifact. r=ted

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d1d8024cc28