From 9e59a1a8c1b1a5a363ad94b4699e6765772a61c5 Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Tue, 2 Jul 2024 12:35:04 -0700 Subject: [PATCH] Relax zerocopy-derive's MSRV policy (#1482) 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 --- .github/workflows/ci.yml | 40 ++++++++++++++++---------------------- POLICIES.md | 19 ++++++++++++++---- ci/check_msrv.sh | 28 -------------------------- githooks/pre-push | 2 -- zerocopy-derive/Cargo.toml | 6 +----- 5 files changed, 33 insertions(+), 62 deletions(-) delete mode 100755 ci/check_msrv.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76d015bd75..8f7bfcc9dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 @@ -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 @@ -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 & @@ -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 diff --git a/POLICIES.md b/POLICIES.md index 13ddeafbcc..474e92277f 100644 --- a/POLICIES.md +++ b/POLICIES.md @@ -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). + + +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 diff --git a/ci/check_msrv.sh b/ci/check_msrv.sh deleted file mode 100755 index 5cfb27f0cc..0000000000 --- a/ci/check_msrv.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright 2024 The Fuchsia Authors -# -# Licensed under a BSD-style license , Apache License, Version 2.0 -# , or the MIT -# license , at your option. -# This file may not be copied, modified, or distributed except according to -# those terms. - -set -eo pipefail - -# Usage: msrv -function msrv { - cargo metadata -q --format-version 1 | jq -r ".packages[] | select(.name == \"$1\").rust_version" -} - -ver_zerocopy=$(msrv zerocopy) -ver_zerocopy_derive=$(msrv zerocopy-derive) - -if [[ "$ver_zerocopy" == "$ver_zerocopy_derive" ]]; then - echo "Same MSRV ($ver_zerocopy) found for zerocopy and zerocopy-derive." | tee -a $GITHUB_STEP_SUMMARY - exit 0 -else - echo "Different MSRVs found for zerocopy ($ver_zerocopy) and zerocopy-derive ($ver_zerocopy_derive)." \ - | tee -a $GITHUB_STEP_SUMMARY >&2 - exit 1 -fi diff --git a/githooks/pre-push b/githooks/pre-push index 7f32fc8832..2008373afd 100755 --- a/githooks/pre-push +++ b/githooks/pre-push @@ -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=$! @@ -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 diff --git a/zerocopy-derive/Cargo.toml b/zerocopy-derive/Cargo.toml index e63759f70a..b3e2171b98 100644 --- a/zerocopy-derive/Cargo.toml +++ b/zerocopy-derive/Cargo.toml @@ -14,7 +14,6 @@ authors = ["Joshua Liebow-Feeser "] 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, @@ -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