google / google/zerocopy

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

Open
#422 1 comment 0 reactions 0 assignees View on GitHub
experience-medium help wanted
Dominant language
Rust
Stars
2.6k
Forks
179
Avg merge
1d 19h
Merged PRs (30d)
29

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](https://docs.rs/zerocopy/latest/zerocopy/#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:

https://github.com/google/zerocopy/blob/3bb9a54ff458e104f7eb24beda7f8fb09794e1a5/.github/workflows/ci.yml#L207-L227

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](https://github.com/obi1kenobi/cargo-semver-checks/issues/538#issuecomment-1734225318), 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`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.