Closed Bug 1369115 Opened 3 years ago Closed 3 years ago

Update win64 builders to rust 1.18.0-beta.4

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

Details

Attachments

(1 file)

Update nightly builds for 64-bit windows to the current rustc 1.18.0 beta. This picks up the fix in rust-lang/rust#40549 giving us better crash reports on this platform.

Improving feedback during our deployment of Rust code for Quantum is important, so I think the additional risk of bumping the toolchain for Firefox 55 is justified. The Rust 1.18.0 stable release is due a week before Firefox moves to beta; updating to a beta now reduces risk by giving us an extra too weeks of testing on nightly. There are typically few changes after this point in the Rust beta cycle. We can still meet our commitment to ship release Firefox built with stable Rust by updating to the final 1.18.0 in late nightly or early beta.

Previously Chris Peterson had recommended against this on the grounds that there wouldn't be much new Rust code in Firefox 55. After further discussion, Anthony Jones and I agreed it was worthwhile.
Assignee: nobody → giles
Comment on attachment 8873107 [details] [diff] [review]
Update win64 builders to rust-1.18.0 beta

Can you update the minimum rust version in python/mozboot/mozboot/base.py as well? We'd like to avoid issues like bug 1366542 where the bootstrap installs an older version of rust that isn't actually supported/tested.

https://bugzilla.mozilla.org/show_bug.cgi?id=1366542#c3
https://bugzilla.mozilla.org/show_bug.cgi?id=1366542#c5
Attachment #8873107 - Flags: review?(mshal) → review-
Thanks for the quick review.

I don't think that makes sense here. Crash reporting isn't critical for local developer builds, and making devs install a beta toolchain is something we've avoided so far. Since this bump is only on one platform, there's little risk of incompatible code making it into the tree.

If you do want this, do you want me to make the version check platform-specific, or would you prefer I update all the platforms, not just win64?
Flags: needinfo?(mshal)
My knee jerk reaction is I'd prefer to avoid per-platform one-offs. I'd also like to stop running pre-release Rust in CI. I understand it speeds up development and bootstrap has gone a long way towards removing the inconvenience of updating. But there's still a change cost that has to be paid.

I'm also sensitive to the idea that we lack CI coverage for minimally supported toolchains. We somehow get away with this for C++. But at the pace Rust is moving, I'm not as confident. That being said, I agree with comment #3 that in this instance there's probably little risk for breaking the previous version since we have coverage on other platforms. IMO the important thing here is we bump the minimum requirement once CI has completely lost coverage.

As for this patch, I could go both ways. I'd feel better if we bumped local developer builds to match CI. I'm fine limiting pre-release Rust to the specific platform it is required on.
FWIW, bug 1337955 tracks improving test automation for past and future rust toolchains.

> But there's still a change cost that has to be paid.

Can you elaborate on what the cost of changing is? Surely the point of the tooltool manifests is that updates are easy, and if tests pass things are (likely) fine?
Comment on attachment 8873107 [details] [diff] [review]
Update win64 builders to rust-1.18.0 beta

Requesting review again. I don't think bumping the minimum makes sense for this patch. See comment #3.
Flags: needinfo?(mshal)
Attachment #8873107 - Flags: review- → review?(mshal)
Attachment #8873107 - Flags: review?(mshal) → review+
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26dc7d6e8721
Update win64 builders to rust 1.18.0-beta.4. r=mshal
https://hg.mozilla.org/mozilla-central/rev/26dc7d6e8721
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Ted added a rustPanic test in bug 1300152 that is marked as a known failure on Win64. Though it would intermittently pass (bug 1352647), it should now always pass because Rust 1.18 includes a fix for Win64 panic handling.
Blocks: 1371372
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.