Closed Bug 1473069 Opened 6 years ago Closed 5 years ago

Stop using Rust nightly when 1.28 is stable

Categories

(Testing :: Code Coverage, enhancement)

enhancement
Not set
normal

Tracking

(firefox71 fixed)

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: marco, Assigned: marco)

References

Details

Attachments

(1 file)

Some fixes landed in Rust nightly that we need for coverage builds. Once these fixes are in Rust stable, we can switch to it (and use our experimental features with RUST_BOOTSTRAP, like we do for Searchfox (https://hg.mozilla.org/mozilla-central/file/9c02d2ecf22050bfee5d70c04a359d8aaff6eb91/browser/config/mozconfigs/linux64/debug-searchfox-clang#l17).
Ted, I was thinking of switching to RUSTC_BOOTSTRAP so that we could keep the same version between ccov builds and normal builds, but you mentioned in bug 1495012 that it'd be preferable to use Rust Nightly rather than RUSTC_BOOTSTRAP, can you clarify why? If it's better to use Rust Nightly for the time being, I'll WONTFIX this bug.
Flags: needinfo?(ted)
Our use of `RUSTC_BOOTSTRAP` was intended to be narrowly scoped to the simd work for hsivonen's encoding-rs crate: he couldn't achieve performance parity with the existing C code without simd, and we felt that there was a compelling argument there. We've committed to only requiring stable Rust for Firefox, so while we could use Nightly in our CI it makes it a lot harder to ensure that we're not going to require other Nightly-only features. For builds that we're not shipping to users, like code coverage, there's no issue with using nightly Rust, and if you're using unstable features that is probably a more sustainable path. My main worry is that if you use stable + `RUSTC_BOOTSTRAP` you run the risk of causing breakage when we attempt to upgrade our Rust compiler for all of our builds, and that puts the onus on whoever is doing that upgrading (likely chmanchester these days) to at least diagnose the issue, if not fix it. If you're using nightly then upgrading the toolchain you use for code coverage builds is your own choice.
Flags: needinfo?(ted)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX

We can use RUSTC_BOOTSTRAP with a stable rust, but using a separate toolchain from that used by shipped builds (it would at least make it very easy to update the toolchain to the right version).

Or, maybe Chris is fine with dealing with possible ccov fallout (it is pretty rare, so far no Rust update has caused the ccov build to fail).

This comes up periodically as the Rust toolchain used by ccov becomes old over time.

Status: RESOLVED → REOPENED
Flags: needinfo?(cmanchester)
Resolution: WONTFIX → ---

I don't quite understand the scenario where the stable + RUSTC_BOOTSTRAP case makes updates harder, but I don't mind the separate nightly toolchain either.

Flags: needinfo?(cmanchester)

(In reply to Chris Manchester (:chmanchester) from comment #4)

I don't quite understand the scenario where the stable + RUSTC_BOOTSTRAP case makes updates harder, but I don't mind the separate nightly toolchain either.

The problem would be if the update causes the code coverage build to fail, slowing down the process for you (but I guess not so much, you can just define a separate toolchain using the old version for ccov if ccov starts to fail with a newer version).
If you're OK with this situation, I'll switch to using stable + RUSTC_BOOTSTRAP.

(In reply to Marco Castelluccio [:marco] from comment #5)

(In reply to Chris Manchester (:chmanchester) from comment #4)

I don't quite understand the scenario where the stable + RUSTC_BOOTSTRAP case makes updates harder, but I don't mind the separate nightly toolchain either.

The problem would be if the update causes the code coverage build to fail, slowing down the process for you (but I guess not so much, you can just define a separate toolchain using the old version for ccov if ccov starts to fail with a newer version).
If you're OK with this situation, I'll switch to using stable + RUSTC_BOOTSTRAP.

It would definitely be nice to avoid having to update the nightly target every time we update our rustc version. Chris does this seem reasonable?

Flags: needinfo?(cmanchester)

Yes it seems reasonable. In practice it's up to whoever does the rustup to bump the coverage builds anyway because the nightly they're using falls behind the minimum required version, so the current situation's no improvement from the standpoint of worrying about coverage builds breaking with newer rustc.

Flags: needinfo?(cmanchester)
Blocks: 1587523
Pushed by mcastelluccio@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ebf7d4bb6884 Use stable Rust with RUSTC_BOOTSTRAP for code coverage builds. r=chmanchester
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → mcastelluccio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: