Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide on policy for ICU4X and Rust version compatibility #3425

Closed
sffc opened this issue May 11, 2023 · 14 comments · Fixed by #3608
Closed

Decide on policy for ICU4X and Rust version compatibility #3425

sffc opened this issue May 11, 2023 · 14 comments · Fixed by #3608
Assignees
Labels
S-tiny Size: Less than an hour (trivial fixes) T-docs-tests Type: Code change outside core library

Comments

@sffc
Copy link
Member

sffc commented May 11, 2023

We've had some past discussions in this area, such as:

The problem I want to talk about today is what we should do with the Rust compiler makes breaking changes. This has now happened to us recently on both the nightly and the stable release channels:

We have a nightly CI now, which is good because it can alert us to upcoming breakages. @Manishearth pointed out that we may be able to work with the Rust team on timelines for rolling out features that would break us.

First, some definitions:

Item Acronym Definition
Minimum Supported Rust Version MSRV The lowest Rust stable where we guarantee the release to work*.
Maximum Supported Rust Version MxSRV The highest Rust stable where we guarantee the release to work.
Minimum Supported Rust Nightly MSRN The lowest Rust nightly where we guarantee the release to work.
Maximum Supported Rust Nightly MxSRN The highest Rust nightly where we guarantee the release to work.

* "Guarantee to work" means that the full ICU4X test suite at the given version should pass without errors on the stated Rust compiler version.

I'd like to see us advocate for the following guarantees:

  • MSRV: Updated once per major ICU4X release. Does not change during a major release. Our MSRV in the 1.x channel in 1.61.
  • MxSRV: Guaranteed for 9 months. We work with Rust upstream to ensure that any ICU4X lockfile created at the tip of the ICU4X release stream within the last 9 months will continue to compile on Rust stable.
  • MSRN: Guaranteed for 6 months. At release time, ICU4X should work on a nightly compiler that is at most 6 months old. For example, if the release is on 2023-05-11, then a nightly compiler of 2022-11-11 or newer should work.
  • MxSRN: Guaranteed for 3 months. Any client with an ICU4X lockfile created at the tip of the ICU4X release stream should be able to update their nightly for up to 3 months without breaking.

I want to highlight the following aspects of this policy:

  1. MSRV and MSRN are enforced during ICU4X releases. We restrain ourselves from using features introduced by newer versions of the compiler.
  2. MxSRV and MxSRN can only be enforced by actions taken in upstream Rust, and our team engages with them as necessary to attempt to achieve that.
  3. Please note that MxSRV and MxSRN reference latest lockfile. What I mean by this is that if someone installs "latest ICU4X" on a certain date, that installation should continue working for the stated time period without updating their lockfile. For example, ICU4X 1.1 was released on 2023-01-26, which means that people who ran cargo add on 2023-01-25 are still on ICU4X 1.0. This would imply that we seek out a guarantee where that particular ICU4X version should continue working until 2023-10-25.
  4. Patch releases can reset the clock, but they should not be an alternative to this policy. For example, if we release a patch release on 2023-03-15 that makes ICU4X future-proof, then that date should be the anchor for the MxSRV/MxSRN calculations.

Discuss with:

Optional:

@sffc sffc added the discuss Discuss at a future ICU4X-SC meeting label May 11, 2023
@sffc
Copy link
Member Author

sffc commented May 15, 2023

https://xkcd.com/2224/

XKCD 2224

@sffc sffc added the discuss-triaged The stakeholders for this issue have been identified and it can be discussed out-of-band label May 25, 2023
@Manishearth
Copy link
Member

Manishearth commented May 31, 2023

I'm in favor of this, but I would also include the potential for violating MSRN in patch releases when MxSRN is at risk (and upstream isn't willing or capable of doing the necessary mitigation). Basically, I think it should be okay to make main commits and potentially patch releases that fix MxSRN (making stuff work on the latest nightly) when things break, even if MSRN is violated.

The reason is the same reason I've given before: using Rust nightly on a pinned nightly that you are unwilling to update is not very well supported in the ecosystem, and while I understand the use case for doing so, I think in cases of unresolvable conflict of needs we should pick the direction that has friction in the use case where there already is a lot of friction (and is not generally considered good practice).

I think this is the only scenario where these four may come in conflict with each other. There is the theoretical scenario where MxSRV and MSRV are in conflict but that seems far more rare since I expect attempts to ask for mitigation paths to be much more consistently successful (and that these cases will be very very rare in the first place).


I also want to underscore for others that the MxSRV/MxSRN policies are good guidelines on when we ought to ask the Rust project to set timelines, but they are not beholden to anything we decide here. That said, I think in most cases a smooth migration path is available, and in most cases just going "hey we'd like this to work for a couple more months, can you provide a migration path for that long" ought to suffice.

@robertbastian
Copy link
Member

MSRV: Updated once per major ICU4X release. Does not change during a major release. Our MSRV in the 1.x channel in 1.61.

I usually see crates guaranteeing 6 months, i.e. 4 releases of MSRV. While all our dependencies are building with 1.61 (which is older) as of today, I don't think that will necessarily stay that way for the whole 1.0 release track. Compiler/language changes seem to make breaking changes on a similar time scale.

@Manishearth
Copy link
Member

Most crates yes, but larger / more foundational projects less so, because they wish to be used in scenarios where the stable Rust compiler will be rarely upgraded. I think ICU4X can be in that bucket. This is one of the reasons we have very few dependencies overall.

@robertbastian
Copy link
Member

robertbastian commented Jun 1, 2023

The latest clap requires 1.64, which means ICU4X doesn't build without a lock file anymore and we can't use cargo update anymore.

@Manishearth
Copy link
Member

Yeah I am willing to be more lax about this for datagen CLI.

@robertbastian
Copy link
Member

Per-crate MSRV is a lot more work to enforce, Rust currently doesn't have good tooling for it.

@Manishearth
Copy link
Member

I think we can for cargo check (just check capi and metacrate), the problem is that we may not be able to enforce it for cargo test without writing some of our own tooling.

@hsivonen
Copy link
Member

MSRV: Updated once per major ICU4X release. Does not change during a major release. Our MSRV in the 1.x channel in 1.61.

This seems very limiting for adopting new language features going forward, which seems bad. As @robertbastian notes, the rest of the Rust ecosystem seems to guarantee less. The cost of providing this guarantee seems unfavorable especially if other crates pretty much ensure that consumers need to update Rust itself on the order of months as opposed to years.

Is there a more elaborate rationale for why we should do the work to enable others to stay on older Rust versions than is generally customary in the ecosystem?

@Manishearth
Copy link
Member

Note that out of all the things talked about here, MSRV is the one that we have generally considered already established. Not necessarily sure if it is actually up for debate here since I believe this was part of our 1.0 guarantee (I could be wrong).

I do somewhat agree that this is far stricter than most of the Rust ecosystem. However it is not as strict as quite a few of the foundational Rust crates (e.g. regex and log are 1.60, serde is 1.19, etc). The closer you are to the bottom of a dependency tree, the more likely it is that the crates do this.

We lose out on opportunities for use in other crates that care about this without a sufficient MSRV.

Part of the thing here is: we are not planning to only be used by the Rust ecosystem. Those who have already bought in to Rust may be very used to this, but for those who haven't, constant toolchain churn can be a major downside.

Furthermore, software shipped on distros can have problems with this. Debian stable's rustc is currently 1.63, not far off from where we pin MSRV. Other distros are different. Firefox has managed to depend on beta rustc but AIUI some distros consider that a special case and not everyone has that clout.

Finally, since we are conservative about dependencies anyway, I am not overly worried about this, with the exception of datagen. I am fine with us dropping MSRV requirements for datagen and just keeping them for components and capi. Overall maintaining MSRV has not been a major pain in my experience.


I have a very different position when it comes to nightly versions, because as I've said elsewhere the Rust project does not particularly cater to people who pin nightlies in an immovable way. We really should not bind ourselves too strongly when it comes to that, and the proposed policy does seem to strike a nice balance between trying to be predictable around nightlies but ultimately being best-effort.

@robertbastian
Copy link
Member

I assume those crates with older MSRVs do conditional compilation to stay compatible with older compilers, but use new language features when possible. Given that we don't do that, it's unclear to me whether the marginal benefit of supporting old compilers outweighs the cost of not using new language features such as const slice::from_raw_parts (1.64), const char::from_u32 (1.67) {integer}::ilog10 (1.67), and std::cell::OnceCell (1.70)

@hsivonen
Copy link
Member

Those who have already bought in to Rust may be very used to this, but for those who haven't, constant toolchain churn can be a major downside.

Is there any concreteness about who this would be and what the constraints are? There's a lot of space between only supporting the latest Rust release and treating an MSRV change as a semver breaking change for ICU4X (i.e. pinning MSRV for ICU4X 1.x).

Notably, https://access.redhat.com/documentation/en-us/red_hat_developer_tools/1 suggests that Red Hat is updating the Rust Toolset for Red Hat Enterprise Linux every 4 Rust releases. If every 4 Rust releases is slow enough for the RHEL audience, how much real need is there for an even slower cadence?

Furthermore, software shipped on distros can have problems with this. Debian stable's rustc is currently 1.63, not far off from where we pin MSRV. Other distros are different. Firefox has managed to depend on beta rustc but AIUI some distros consider that a special case and not everyone has that clout.

I think it makes sense to take the position that Debian-shipped rustc should be used for the purpose of Debian-shipped packages being compiled with in-archive compilers, but I think we shouldn't consider it a valid use case to compile out-of-archive software using Debian-shipped rustc (as opposed to using fresh rustc from rustup on Debian).

Taking that position won't prevent Debian-shipped packages from depending on ICU4X: The in-archive copy of ICU4X would be a snapshot from around the same time as the in-archive rustc (or the in-archive rustc would be newer if a Firefox ESR major version change has resulted in Debian upgrading the in-archive rustc).

So I don't think "software shipped on distros" have a problem with a more rolling MSRV in upstream. If Debian x ships ICU4X 1.y as an in-archive package for other in-archive packages to depend on, we shouldn't expect Debian x to shipt ICU4X 1.y+1 anyway.

OTOH, taking the position that using Debian-shipped rustc to compile out-of-archive software on Debian is a valid use case means that Debian's timelines, which are way slower than Rust timelines, would be allowed to limit Rust feature adoption, which seems bad.

@sffc
Copy link
Member Author

sffc commented Jun 30, 2023

  • @Manishearth - Future versions are best-effort.
  • @robertbastian - I wouldn't formalize this.
  • @Manishearth - There are basically no guarantees for nightly; the assumption is that you need to be on a matching nightly version with your code.
  • @hsivonen - How did you come up with these numbers?
  • @sffc - Experience has shown that we need at least 6 months of buffer when upgrading i18n libraries. We should be able to decouple updates to i18n from updates to Rust.
  • @robertbastian - In the long term, we won't need nightly.
  • @hsivonen - What motivates this policy?
  • @Manishearth - Look at ICU4C
  • @sffc - We only have power over minimum version. We can make maximum version work on a best effort basis.
  • @Manishearth - For maximum version, we could allow updates to patch files. Maybe people don't want to update from 1.x to 1.y, but we could make a best effort to publish releases on 1.x.y to 1.x.z if there are upstream breakages.
  • @robertbastian - I don't think Rust will be in the position of Java 8 and C++11. It should be easier for clients to update Rust toolchains than Java or C++ toolchains.
  • @hsivonen - I think it would be sad to make policy based on what happened to Java and C++ when the rest of the Rust ecosystem doesn't buy into that view. Even long-term Linux distros ship newer versions of Rust and LLVM during their lifetimes. I would consider the MSRV guarantee for 6 months; if there are new Rust features that come along, it seems like after 6 months or so, people will have upgraded their CI pipelines.
  • @Manishearth - A reason to maintain a low MSRV is the lower down the tree you go, the more important MSRV can be. The lower your MSRV, the more likely you are to be used in a foundational context. I think it's important for us to be suitable to be used in that context. But, it's not necessarily an argument to pin MSRV on major releases. 6 months might be fine. I want people to be able to use newer Rust versions without having to update ICU4X. We could say that 6 months is a minimum, but we try hard to make that number be larger. Maybe more: 8 months? I don't want us to get in the habit of using new features, but things pile up. The main thing is that I don't want people to be compelled to update to 2.0 to get new Rust features. It might mean making patch releases on 1.4 for a while after 2.0 is released.
  • @sffc - Our product is ICU4X. The Rust compiler is an annoying tool we need to deal with. We should do everything in our power to make it so that users don't get annoyed by things breaking across Rust versions.
  • @robertbastian - At the moment ICU4X doesn't build on Rust 1.61 without a custom lockfile.
  • @hsivonen - I'm not thinking only about enterprise users, but I'm assuming that there is an enterprise constituency that is slow to upgrade their CI, and non-enterprise users can upgrade Rust more often. I would pick a duration in months, like 6 months, as opposed to using our energy to figure out how to do things in ancient Rust versions.
  • @Manishearth - I buy that you don't want people to have to learn too much about Rust, but you need to know the basics. But due to the lockfile issue, we can't really guarantee in either direction. For upgrading Rust on stable, we have a lot of leverage because "upgrade Rust" is designed to be the first tool you reach for.
  • @robertbastian - This is a lot of work for us without a lot of benefit for clients. It's not clear who those clients are.
  • @hsivonen - The more effort for these ghost clients means less productivity for developers of ICU4X. I think we can make a shorter timeframe, and if clients show up who push for a longer timeframe, we can revisit based on actual data.

@Manishearth - Actual proposal:

MSRV: We guarantee 6 months, attempt 9 months, and don't upgrade unless there's a need. If someone wants a feature older than 9 months, they get it; if it's between 6 and 9, they need to make an argument that it has a definite benefit to the project.

MxSRV: I think 9 months is reasonable for that. We work with upstream, and we do patch releases. We try to get this handled by Rust upstream as much as possible, and do patch releases if that doesn't work. Ideally push for things to go into an edition, but that's sometimes a harder sell. 9 months is an easier sell for upstream Rust.

  • @robertbastian - I think we should talk about # of Rust versions backward or forward, not dates. And we should also think about how far back we patch our code
  • @sffc - Keep in mind that my definition of MxSRV is based on installation date, not release date.

Revised proposal for stable compilers:

MSRV: guaranteed for 4 releases (N - 4); attempted for 6 releases (N - 6). For example, if today is Rust 1.70, we guarantee that the ICU4X release coming out today works on Rust 1.66, and attempts to work on Rust 1.64. To use a feature between N-4 and N-6, an ICU4X developer needs to make an argument that it has a definite benefit to the project (for example, a client need or a soundness issue).

MxSRV: guaranteed for 270 days from the ICU4X installation date. For example, if ICU4X 1.4 is released on 2023-11-30, then we push for stable Rust compilers released on dates up to and including 2024-08-25 to support an ICU4X lockfile produced on 2023-11-29 (installation date that still includes 1.3). Patch release updates may be required but are not preferred.

For nightly compilers: Make a best attempt at a support window with no specific guarantees, but an attempt at 3-6 months in the past and the future.

LGTM: @Manishearth @hsivonen @robertbastian @skius (@sffc)

@sffc sffc added T-docs-tests Type: Code change outside core library S-tiny Size: Less than an hour (trivial fixes) and removed discuss Discuss at a future ICU4X-SC meeting discuss-triaged The stakeholders for this issue have been identified and it can be discussed out-of-band labels Jun 30, 2023
@sffc sffc added this to the 1.3 Blocking ⟨P1⟩ milestone Jun 30, 2023
@sffc
Copy link
Member Author

sffc commented Jul 3, 2023

One more note: I'd like it if we can give 1.68.2 special status for the ICU4X 1.x release stream, as that is the last version of stable Rust that can build 1.0 and 1.1. Might be a moot point if we release 2.0 sufficiently early in 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-tiny Size: Less than an hour (trivial fixes) T-docs-tests Type: Code change outside core library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants