-
Notifications
You must be signed in to change notification settings - Fork 81
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
Reduce our MSRV as much as possible #554
Comments
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
Modify the way that certain memory safety properties are validated in our `transmute!`, `transmute_ref!`, and `transmute_mut!` macros to allow that code to compile on 1.60. Makes progress on #554
I'm a little curious about the impact of this (as a general goal, not specifically the impact to
Perhaps it's more feasible for In #557 you reference libc MSRV and targets of Your current target reached ( Once you pass In contrast of that full ecosystem, the Note the widening "broken deps" compatibility, along with the steep "incompatible" increase after going below Possibly relevant: mitsuhiko/insta#289 (comment)
Until the MSRV resolver work is stabilized and crates adopt declaring What once resolved and installed correctly via What will your plan be once you reached the MSRV target you're aiming for? Will you continue to support that for N years? Semver bumps are apparently advised to not be affected by MSRV bumps for valid reasons, so using minor version bumps for MSRV and keeping those release streams updated with patch releases doesn't appear to be advised? (crates like While the MSRV policy resolver work should improve on this situation, by the time crates have more widely adopted declaring
You'd have a better idea of all this I assume, along with actual users requiring it? ( #557 is marked with the label With Rust
|
Thanks for the detailed analysis! I think I can answer most of your questions, but let me know if I've missed anything:
|
In order to do this, we make a few changes: - In 1.56, panicking is not supported in `const fn`s. In order to support 1.56 while still panicking in `const fn`s in later versions, we make a few changes: - We introduce a `build.rs` script which detects the Rust toolchain version and emits the cfg `zerocopy_panic_in_const_fn` if the Rust version is at least 1.57. - In some `const fn`s, we put debug assertions behind `#[cfg(zerocopy_panic_in_const_fn)]`. - In some `const fn`s, we replace `unreachable!()` with non-panicking code when `#[cfg(not(zerocopy_panic_in_const_fn))]`. - In one case, we compile a method as either a `const fn` or a non-`const fn` depending on `zerocopy_panic_in_const_fn`. - We replace `&*dst` with `transmute(dst)`. - We make sure that, in `impl` blocks, `const` parameters always come last. - We make the `byteorder` feature/crate dependency off by default. This is a breaking change, and so we bump the version number to 0.8.0-alpha. Release 0.8.0-alpha. Makes progress on #554
TODO: Populate commit message with details about updated infrastructure in this commit (notably, testutil crate's use of MSRV vs MWRV and CI's passing of --ignore-rust-version) Makes progress on #554
Makes progress on #554
In order to lower our MSRV as far as we'd like, we will be losing access to some features that we currently use. Some of these features cannot be simply worked around, and impose limitations on what APIs we can support (for example, what trait bounds can exist on `const fn`s). This commit works around this problem by introducing a general-purpose framework for detecting the toolchain version at compile time, and conditionally compiling code based on whether or not the toolchain is recent enough. This ensures that no user code will break. In particular, at any given time, if our MSRV is version N, then all code we publish is compatible wtih version N. If, later, we lower our MSRV to M < N, we can ensure that all functionality from the old version of zerocopy is still present when compiling on versions at least as recent as N. We know that none of our old users had an MSRV lower than N since they were using zerocopy with an MSRV of N. Thus, publishing a new version in which some features are not available on versions lower than N is not problematic for existing users. This approach introduces a new degree of freedom that we need to be sure to test in CI. Previously, in CI, we tested on the MSRV, stable, and nightly toolchain versions. Now, our behavior can differ on an arbitrary number of toolchains, and we need to make sure to test on every such toolchain. To accomplish this, we put the source of truth for these versions in `Cargo.toml`. We introduce a `build.rs` file which parses these versions and emits `--cfg` variables which can be used by our Rust code. We update `cargo.sh` to be able to make use of these versions, and add every such version as a toolchain to test in CI. Finally, we add a CI job which parses `Cargo.toml` and `.github/workflows/ci.yml` to ensure that every version listed in `Cargo.toml` is tested in CI. In order to test this framework, this commit lowers our MSRV to 1.58.0. Makes progress on #554
In order to lower our MSRV as far as we'd like, we will be losing access to some features that we currently use. Some of these features cannot be simply worked around, and impose limitations on what APIs we can support (for example, what trait bounds can exist on `const fn`s). This commit works around this problem by introducing a general-purpose framework for detecting the toolchain version at compile time, and conditionally compiling code based on whether or not the toolchain is recent enough. This ensures that no user code will break. In particular, at any given time, if our MSRV is version N, then all code we publish is compatible wtih version N. If, later, we lower our MSRV to M < N, we can ensure that all functionality from the old version of zerocopy is still present when compiling on versions at least as recent as N. We know that none of our old users had an MSRV lower than N since they were using zerocopy with an MSRV of N. Thus, publishing a new version in which some features are not available on versions lower than N is not problematic for existing users. This approach introduces a new degree of freedom that we need to be sure to test in CI. Previously, in CI, we tested on the MSRV, stable, and nightly toolchain versions. Now, our behavior can differ on an arbitrary number of toolchains, and we need to make sure to test on every such toolchain. To accomplish this, we put the source of truth for these versions in `Cargo.toml`. We introduce a `build.rs` file which parses these versions and emits `--cfg` variables which can be used by our Rust code. We update `cargo.sh` to be able to make use of these versions, and add every such version as a toolchain to test in CI. Finally, we add a CI job which parses `Cargo.toml` and `.github/workflows/ci.yml` to ensure that every version listed in `Cargo.toml` is tested in CI. In order to test this framework, this commit lowers our MSRV to 1.58.0. Makes progress on #554
Makes progress on #554
Makes progress on #554
Our MSRV is currently 1.61, which is higher than the MSRVs of many crates. Here are MSRVs of the subset of the top 100 most-recently-downloaded crates that contain
unsafe
code:Top 100 crates
We should work to reduce our MSRV as much as possible so that more crates can replace
unsafe
code with zerocopy.Mentoring instructions
Conditional compilation problems
One problem with lowering our MSRV is that some features we want to use were introduced in more recent Rust versions. Naively, one might expect that this makes lowering our MSRV dead in the water. However, it's possible to work around this, albeit with a lot of machinery.
The core insight is that it's okay for zerocopy to provide certain features only when compiled with certain Rust versions. For example, imagine that a certain feature is only available on 1.60 and above. Downstream crates can be in one of two buckets:
In both cases, it's consistent with semver rules for us to enable certain features only on more recent Rust versions.
This brings us to our first requirement: We need to support detecting the Rust toolchain version at compile time.
If we take this approach, it introduces a new problem: now zerocopy's behavior can differ by toolchain. If we continue to only test on MSRV, stable, and nightly toolchains, then there will be some toolchain versions which introduce different behavior relative to their parent, but on which we don't test. For example, if our MSRV is 1.56, and our pinned stable toolchain is 1.70, but we have a feature that is enabled on versions after 1.65, then it's possible that that feature is buggy on 1.65, 1.66, 1.67, 1.68, and 1.69. This implies that we also need to test on 1.65 (the first Rust version for which the feature is enabled). We need to do this for any Rust version for which we enable new behavior.
This brings us to our second requirement: In CI, we need to test on every Rust version which introduces new behavior.
This introduces another problem, though. How do we actually implement this? Naively, we could write our
build.rs
script to perform toolchain detection based on hard-coded toolchain versions, and then separately update our CI configuration to test on these toolchain versions. However, this risks bit rot: we could easily change the hard-coded versions in either place, but forget to update them in the other place. There would be no automatic way to detect this.Instead, we should have one source of truth for the list of toolchain versions. This brings us to our third requirement: Both
build.rs
and our CI scripts should parse a single source of truth for the list of toolchain versions.All three of these requirements are implemented in #843.
The text was updated successfully, but these errors were encountered: