Skip to content

Commit

Permalink
Relax zerocopy-derive's MSRV policy (#1482)
Browse files Browse the repository at this point in the history
This also bumps zerocopy-derive's syn dependency to 2.0.46. We don't
actually need this for zerocopy-derive. However, in CI, we modify
zerocopy-derive to take an "exact" dependency on syn (ie, '=2.0.46'
instead of '2.0.46'). One of the transitive dependencies of our
`testutil` crate requires syn 2.0.46 or later. We could in principle
make CI smart enough to test with both syn versions, but we wouldn't
gain much for that complexity - it's easier to just bump the version.

Fixes #1085
See also #1481
  • Loading branch information
joshlf authored Jul 2, 2024
1 parent b90e1a6 commit 9e59a1a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 62 deletions.
40 changes: 17 additions & 23 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ jobs:
- name: Populate cache
uses: ./.github/actions/cache

# Ensure that Cargo resolves the minimum possible syn version so that if we
# accidentally make a change which depends upon features added in more
# recent versions of syn, we'll catch it in CI.
- name: Pin syn dependency
run: |
set -eo pipefail
# Override the exising `syn` dependency with one which requires an exact
# version.
cargo add -p zerocopy-derive 'syn@=2.0.46'
- name: Configure environment variables
run: |
set -eo pipefail
Expand Down Expand Up @@ -438,28 +448,6 @@ jobs:
- name: Check README.md
run: ./ci/check_readme.sh

check_msrv:
needs: generate_cache
runs-on: ubuntu-latest
name: Check MSRVs match
steps:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7

- name: Populate cache
uses: ./.github/actions/cache

# Make sure that the MSRV in zerocopy's and zerocopy-derive's `Cargo.toml`
# files are the same. In CI, we test with a single MSRV (the one indicated
# in zerocopy's `Cargo.toml`), so it's important that:
# - zerocopy-derive's MSRV is not lower than zerocopy's (we don't test with
# a lower MSRV in CI, so we couldn't guarantee that zerocopy-derive
# actually built and ran on a lower MSRV)
# - zerocopy-derive's MSRV is not higher than zerocopy's (this would mean
# that compiling zerocopy with the `derive` feature enabled would fail
# on its own published MSRV)
- name: Check MSRVs match
run: ./ci/check_msrv.sh

check_versions:
needs: generate_cache
runs-on: ubuntu-latest
Expand Down Expand Up @@ -504,6 +492,12 @@ jobs:
# in parallel.
#
# [1] https://stackoverflow.com/a/42139535/836390
# See comment on "Pin syn dependency" job for why we do this. It needs
# to happen before the subsequent `cargo check`, so we don't
# background it.
cargo add -p zerocopy-derive 'syn@=2.0.46' &> /dev/null
cargo check --workspace --tests &> /dev/null &
cargo metadata &> /dev/null &
cargo install cargo-readme --version 3.2.0 &> /dev/null &
Expand Down Expand Up @@ -559,7 +553,7 @@ jobs:
# https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks
if: failure()
runs-on: ubuntu-latest
needs: [build_test, kani, check_fmt, check_readme, check_msrv, check_versions, generate_cache, check-all-toolchains-tested, check-job-dependencies, run-git-hooks]
needs: [build_test, kani, check_fmt, check_readme, check_versions, generate_cache, check-all-toolchains-tested, check-job-dependencies, run-git-hooks]
steps:
- name: Mark the job as failed
run: exit 1
19 changes: 15 additions & 4 deletions POLICIES.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,21 @@ documented guarantees do not hold.

## MSRV

Our minimum supported Rust version (MSRV) is encoded in our `Cargo.toml` file.
We consider an increase in MSRV to be a semver-breaking change, and will only
increase our MSRV during semver-breaking version changes (e.g., 0.1 -> 0.2, 1.0
-> 2.0, etc).
<!-- Our policy used to be simply that MSRV was a breaking change in all
circumstances. This implicitly relied on syn having the same MSRV policy, which
it does not. See #1085 and #1088. -->

Without the `derive` feature enabled, zerocopy's minimum supported Rust version
(MSRV) is encoded the `package.rust-version` field in its `Cargo.toml` file. For
zerocopy, we consider an increase in MSRV to be a semver-breaking change, and
will only increase our MSRV during semver-breaking version changes (e.g., 0.1 ->
0.2, 1.0 -> 2.0, etc).

For zerocopy with the `derive` feature enabled, and for the zerocopy-derive
crate, we inherit the MSRV of our sole external dependency, syn. As of this
writing (2024-07-02), syn does *not* consider MSRV increases to be
semver-breaking changes. Thus, using the `derive` feature may result in the
effective MSRV increasing within a semver version train.

## Yanking

Expand Down
28 changes: 0 additions & 28 deletions ci/check_msrv.sh

This file was deleted.

2 changes: 0 additions & 2 deletions githooks/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ echo "Running pre-push git hook: $0"
./ci/check_fmt.sh & FMT_PID=$!
./ci/check_all_toolchains_tested.sh >/dev/null & TOOLCHAINS_PID=$!
./ci/check_job_dependencies.sh >/dev/null & JOB_DEPS_PID=$!
./ci/check_msrv.sh >/dev/null & MSRV_PID=$!
./ci/check_readme.sh >/dev/null & README_PID=$!
./ci/check_versions.sh >/dev/null & VERSIONS_PID=$!

Expand All @@ -30,7 +29,6 @@ echo "Running pre-push git hook: $0"
wait $FMT_PID
wait $TOOLCHAINS_PID
wait $JOB_DEPS_PID
wait $MSRV_PID
wait $README_PID
wait $VERSIONS_PID

Expand Down
6 changes: 1 addition & 5 deletions zerocopy-derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ authors = ["Joshua Liebow-Feeser <[email protected]>"]
description = "Custom derive for traits from the zerocopy crate"
license = "BSD-2-Clause OR Apache-2.0 OR MIT"
repository = "https://github.com/google/zerocopy"
rust-version = "1.56.0"

# We prefer to include tests when publishing to crates.io so that Crater [1] can
# detect regressions in our test suite. These two tests are excessively large,
Expand All @@ -29,10 +28,7 @@ proc-macro = true
[dependencies]
proc-macro2 = "1.0.1"
quote = "1.0.10"
# This pinned dependency is a temporary work-around for #1085. Per #1088, we
# will not ship 0.8 until we've removed this work-around and replaced it with a
# more permanent solution.
syn = "=2.0.55"
syn = "2.0.46"

[dev-dependencies]
# We don't use this directly, but trybuild does. On the MSRV toolchain, the
Expand Down

0 comments on commit 9e59a1a

Please sign in to comment.