From f8eb710aa9a469cbb7633ecfb0cdecc05a0e3cfd Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 20 Aug 2024 13:27:37 -0700 Subject: [PATCH 1/3] Document process for breaking changes --- src/breaking-changes/new-trait-impls.md | 6 ++++++ src/breaking-changes/summary.md | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/breaking-changes/new-trait-impls.md b/src/breaking-changes/new-trait-impls.md index 33d4cf8..f3187c7 100644 --- a/src/breaking-changes/new-trait-impls.md +++ b/src/breaking-changes/new-trait-impls.md @@ -37,6 +37,12 @@ let b = Arc::from("a"); will no longer compile, because we've previously been relying on inference to figure out the `T` in `Box`. This kind of breakage can be ok, but a [crater](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) run should estimate the scope. +When implementing traits known to have this problem, crater should be run before initiating FCP, +so information on the scope of the breakage is available before deciding to accept the change. +This can include, but is not limited to, + +- From +- FromIterator ## Deref coercion breaks when a new impl is introduced diff --git a/src/breaking-changes/summary.md b/src/breaking-changes/summary.md index 9368382..44b49ac 100644 --- a/src/breaking-changes/summary.md +++ b/src/breaking-changes/summary.md @@ -4,5 +4,14 @@ Breaking changes should be avoided when possible. [RFC 1105](https://rust-lang.github.io/rfcs/1105-api-evolution.html) lays the foundations for what constitutes a breaking change. Breakage may be deemed acceptable or not based on its actual impact, which can be approximated with a [crater](https://github.com/rust-lang/crater/blob/master/docs/bot-usage.md) run. +Running crater should be done if nontrivial breakage is expected, so the information is +available during the final comment period. -If the impact isn't too high, looping in maintainers of broken crates and submitting PRs to fix them can be a valid strategy. +If the impact isn't too high, looping in maintainers of to-be-broken crates and submitting PRs +to fix them can be a valid strategy. However, this can only affect the crates in question, and +it does not automatically affect their dependents. Binary dependents may have already locked-in +a different, older version. + +## Breaking and the trains +If a PR is merged and it turns out to have caused code to not compile during the nightly or beta release cycle, +unless there is a trivial fix, the PR should be reverted and a crater run should assess the impact. From 7388b6d079ad7ce83627ac71407f145395fd6f50 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 25 Aug 2024 00:15:28 -0700 Subject: [PATCH 2/3] Document a trivial fix example for breaking changes --- src/breaking-changes/summary.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/breaking-changes/summary.md b/src/breaking-changes/summary.md index 44b49ac..97e8731 100644 --- a/src/breaking-changes/summary.md +++ b/src/breaking-changes/summary.md @@ -15,3 +15,19 @@ a different, older version. ## Breaking and the trains If a PR is merged and it turns out to have caused code to not compile during the nightly or beta release cycle, unless there is a trivial fix, the PR should be reverted and a crater run should assess the impact. + +### Model: A Trivial Fix + +On 2024, March 9th, [`Context::ext`] was added in [#123203]. It adds a `&mut dyn Any` field to Context. +On 2024, May 16th regression [#125193] appeared in beta crater: `Context` was no longer `UnwindSafe`. +Some code depended on it being so, but `&mut T` is `!UnwindSafe`, so Context also became `!UnwindSafe`. + +The PR to add `Context::ext` could have been reverted, but as the function is a nightly feature, +its implications for unwind safety are limited to those actually using that nightly feature. +Since nightly features are, by definition, permitted to have effects we may not want to stabilize, +the question of whether the unwind safety regression should be accepted was deferred by [#125392] +wrapping the field in `AssertUnwindSafe`. This allowed continued experimentation with the API. + +[#123203]: https://github.com/rust-lang/rust/pull/123203 +[#125193]: https://github.com/rust-lang/rust/pull/125193 +[#125392]: https://github.com/rust-lang/rust/pull/125392 From 5322b01f37a545ed661c9a4c08df1549b723a766 Mon Sep 17 00:00:00 2001 From: Jubilee Date: Tue, 27 Aug 2024 08:14:42 -0700 Subject: [PATCH 3/3] Admonish to wait on maintainers landing stuff Co-authored-by: Josh Triplett --- src/breaking-changes/summary.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/breaking-changes/summary.md b/src/breaking-changes/summary.md index 97e8731..a93be68 100644 --- a/src/breaking-changes/summary.md +++ b/src/breaking-changes/summary.md @@ -8,7 +8,7 @@ Running crater should be done if nontrivial breakage is expected, so the informa available during the final comment period. If the impact isn't too high, looping in maintainers of to-be-broken crates and submitting PRs -to fix them can be a valid strategy. However, this can only affect the crates in question, and +to fix them (and ensuring those PRs have been merged and released) can be a valid strategy. However, this can only affect the crates in question, and it does not automatically affect their dependents. Binary dependents may have already locked-in a different, older version.