Should we allow the pinned nightly to diverge from the pinned stable?

[Manishearth]

ICU4X is a library which runs on Stable Rust, however there are some cases where an ICU4X client may need to use nightly Rust. In particular, nightly Rust is necessary for -Zbuild-std which we use for size-optimized, wasm, and Cortex-M builds.

We CI these, and for that purpose CI pins a nightly (currently nightly-2022-04-05¹) for a bunch of these features. We have historically attempted to use a nightly that is close to the point where the Rust stable release branched off into the release trains. This has a benefit of the behavior between the two versions being identical.

However, there are some reasons to not do this.

Firstly, we have a bunch of other constraints on the nightly version as well. Every time we upgrade, we need to take care about the LLVM version² and Rust bugs. The biggest issue, which cropped up in #2700, is that we can't rely on backports for these nightlies.

Basically, Rust will happily backport breakage fixes to stable and beta versions (and we've relied on this in the past), however these backported versions will still be "stable"-like versions (no nightly features). There is no similar mechanism for nightlies.

So, for example, if -Zbuild-std is broken (as it is in #2700, due to rust-lang/rust#98378), we cannot ask rustc to backport the fix for us: at the very best they will backport it to Rust stable/beta, where we can't use it anyway.

If this occurs during an attempt to update our pinned stable, we're stuck waiting out the release cycle until we can upgrade to a newer stable and nightly (and it forces us to a newer stable: we may not necessarily desire that!).

This "waiting out the release cycle" ought to be partly ameliorated by #2718, as we will hopefully be able to catch issues at the drop of a hat (and get them fixed sooner rather than later).

But I think there's a very decent reason to be open to such divergences in the future.

Footnotes

1. The verify-ffi task additionally uses nightly-2022-08-25 due to issues in rustdoc-types. This is a test and does not affect clients. ↩

2. It may be worth figuring out a way out of this problem by building LLVM and storing a binary somewhere, since this is just needed for one part of one test. ↩

[Manishearth]

There's some prior discussion on this in #2700 (comment), where @sffc has some valid points, especially about compatibility for clippy and things like the size optimized build.

My position regarding clippy (already stated there as well) is twofold.

Firstly, I think it's overall fine for us to recommend users stick to pinned stable clippy. It may require a rebuild, but clippy isn't a typical part of the edit-compile-debug cycle and it might be an okay cost to pay to run a full stable build of your component (that you have been building on nightly) before making the PR. This only affects people developing the ICU4X functionality that requires nightly, which is somewhat rare.

Secondly, I think we can keep pinned-clippy clean by ensuring it is clean during the upgrade and having the cronjob CI (#2718) notify a couple people (happy to volunteer for this) when a PR doesn't satisfy a new lint. I don't think this is strictly necessary (from my previous point), but i do think this is workable and oughtn't be too noisy.

[sffc]

When choosing the ICU4X nightly version, there are several factors that we should consider, and the answer may not be the same each time.

1. Be as close to the latest ancestor of the chosen stable release as possible
2. Uses an LLVM version that is widely available
3. Doesn't have blockers like -Zbuild-std being broken

Constraint (1) gives us a starting point, which I'll call the "ideal" nightly. If the ideal nightly satisfies constraints 2 and 3, then we should use it and move on.

If constraint 2 is violated, then it most likely means we need to rewind to the latest nightly that uses the previous LLVM version.

If constraint 3 is violated, we can wind either forward or backward to the closest nightly that isn't broken, but which still uses a good LLVM version.

[sffc]

It may be worth figuring out a way out of this problem by building LLVM and storing a binary somewhere, since this is just needed for one part of one test.

The LLVM version constraint is about the ICU4X development experience. It is required for cargo make ci-test-ffi. It's a bad experience to say that in order to run ICU4X test jobs locally, you may need to go and build an LLVM version from scratch and set up whatever environment variables are necessary in order to point ICU4X to it. Also, although it is currently used in only one test, I am intending to add more similar tests for code size in other components.

[Manishearth]

I think this is a fine policy. I think (1) as the ideal/"default" nightly makes a lot of sense, I mostly want our policy to be that moving away from (1) should not be traded off against other things like "wait a while to update stable" etc. I.e. we should pick (1) as long as there are no reasons to not do so, and if we need to move off of (1) for any reason whatsoever we should just do it without discussion.

(2) and (3) are far more strong constraitns for me; (3) is a blocker, of course, and I see (2) as something we should strive to achieve as much as possible but worst case there may be escape hatches.

So basically, in order of precedence:

• Nightly should actually work (point 3): This is a blocker
• Nightly should use a widely used LLVM (point 2): This is almost a blocker; can be voilated in favor of the previous entry but shoudl be mitigated.
• Nightly should be close to the pinned stable (point 1): This is never a blocker and just a nice-to-have.

[Manishearth]

The LLVM version constraint is about the ICU4X development experience. It is required for cargo make ci-test-ffi. It's a bad experience to say that in order to run ICU4X test jobs locally, you may need to go and build an LLVM version from scratch and set up whatever environment variables are necessary in order to point ICU4X to it. Also, although it is currently used in only one test, I am intending to add more similar tests for code size in other components.

Oh, I'm aware of why we use it.

I was suggesting that we stash a prebuilt binary .so or something somewhere (or just fetch it from the LLVM website and stash it in some local folder) as a part of the makefile task, and split it out of make ci-test-ffi proper (I would still have ci-test-ffi run fixeddecimal_tiny tests that don't need llvm).

I think there are mitigations for this on the table, I also don't think we have a strong need to solve it.

(I'm just unhappy about distro release cycles constraining our nightly version choice)

[sffc]

Here's my visualization of it:

Clearly we can't choose the broken release, so then we would choose between alternatives 1 and 2.

[Manishearth]

yep, that seems fine

[robertbastian]

The LLVM version constraint is about the ICU4X development experience. It is required for cargo make ci-test-ffi. It's a bad experience to say that in order to run ICU4X test jobs locally, you may need to go and build an LLVM version from scratch and set up whatever environment variables are necessary in order to point ICU4X to it. Also, although it is currently used in only one test, I am intending to add more similar tests for code size in other components.

FWIW on macOS these don't work anyway, I've always just been running them on CI. It's not that bad.

[robertbastian]

I'm thinking of updating the nightly to the last day of LLVM14, 2022-08-12. Thoughts?

There are some CI tasks that don't need LLVM compatibility. E.g. the memory benchmark runs on nightly, but we could just run that on +nightly as well.

[Manishearth]

I'm fine with it

[sffc]

If 2022-08-12 is on LLVM 14 then that works fine for me. We can probably do LLVM 15 soonish.

[sffc]

I think we can close this, yes? We can update nightly ad-hoc / as-needed, and we have another thread about MSRV.