Skip to content

Use cargo-semver-checks to make sure derive feature doesn't change API surface #422

Open

Description

Zerocopy comprises two crates: zerocopy and zerocopy-derive. Zerocopy has a disabled-by-default feature called derive. When it's enabled, zerocopy depends on zerocopy-derive and re-exports its derives so that users can just depend on zerocopy and don't have to also depend directly on zerocopy-derive.

Some of zerocopy's types implement the same traits that we provide derives for (currently, FromZeroes, FromBytes, AsBytes, and Unaligned). When the derive feature is enabled, we use the derives to derive implementations of those traits for our own types. When the derive feature is disabled, we implement those traits manually.

This creates a potential footgun: How do we know that the impls we emit when derive is enabled are identical to the impls that we emit when derive is disabled? Do they have the same bounds?

This issue tracks using the cargo-semver-checks crate to detect such mismatches for us. We already use cargo-semver-checks in CI:

# Check semver compatibility with the most recently-published version on
# crates.io. We do this in the matrix rather than in its own job so that it
# gets run on different targets. Some of our API is target-specific (e.g.,
# SIMD type impls), and so we need to run on each target.
#
# TODO(https://github.com/obi1kenobi/cargo-semver-checks-action/issues/54):
# Currently we don't actually do anything with `matrix.target`, so we're
# just duplicating work by running this job multiple times, each time
# targetting the host platform.
- name: Check semver compatibility
uses: obi1kenobi/cargo-semver-checks-action@v2
with:
# Don't semver check zerocopy-derive; as a proc macro, it doesn't have
# an API that cargo-semver-checks can understand.
package: zerocopy
feature-group: all-features
# TODO: Set this to the specific nightly we have pinned in CI. Not a big
# deal since this isn't affected by the trybuild stderr files, which is
# the reason we need to pin to a specific nightly.
rust-toolchain: nightly
if: matrix.crate == 'zerocopy' && matrix.features == '--all-features' && matrix.toolchain == 'nightly'

Specifically, this issue tracks using cargo-semver-checks to make sure that:

  • The derive feature is semver backwards-compatible with the absence of the derive feature
  • The absence of the derive feature is semver backwards-compatible with the derive feature

Combined, these two conditions should have the effect of ensuring that the APIs with and without derive are equivalent.

This issue tracks the following concrete tasks:

  • Using the technique described in this comment, update ci.yml's cargo-semver-checks use to check that:
    • The crate with the derive feature is semver backwards-compatible with the crate without the derive feature
    • The crate without the derive feature is semver backwards-compatible with the crate with the derive feature. Note that this case is extra tricky! The derive feature re-exports the derives, which means that the APIs are expected to differ! Specifically, we expect that the crate without derive will fail to be backwards-compatible because it has "removed" these exports compared to the crate with derive. You will need to do one of the following:
      • Figure out how to tell cargo-semver-checks to expect this difference, and allow it
      • Decide that there's no way to support this in cargo-semver-checks, and leave a comment in ci.yml explaining why we don't currently support this check
  • Intentionally push two PRs which include this technique and which update zerocopy's source code to violate compatibility:
    • One which makes a change which makes derive semver-incompatible with no-derive
    • One which makes a change which makes no-derive semver-incompatible with derive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    experience-mediumThis issue is of medium difficulty, and requires some experiencehelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions