obi1kenobi / cargo-semver-checks

Scan your Rust crate for semver violations.
Apache License 2.0
1.13k stars 71 forks source link

False-positives caused by newer versions of path dependencies of the scanned crate #902

Open obi1kenobi opened 1 week ago

obi1kenobi commented 1 week ago

Steps to reproduce the bug with the above code

When specifying a package by path dependency, cargo is only willing to use that package's own declared path dependencies and ignores newer SemVer-compatible dependency versions. It refuses to update to newer versions even if explicitly asked with cargo update or even cargo update --precise.

This causes false positives. For example:

Repro:

git clone git@github.com:tokio-rs/tokio.git
cd tokio
git checkout d7b7c6131774ab631be6529fef3680abfeeb4781
cd ..

mkdir repro
cd repro
cat <<EOF >Cargo.toml
[package]
name = "rustdoc"
version = "0.0.0"
edition = "2021"
publish = false

[workspace]
members = []

[dependencies.tokio-stream]
path = "../tokio/tokio-stream"
features = ["default", "fs", "io-util", "net", "signal", "sync", "time", "tokio-util"]

[lib]
path = "lib.rs"
required-features = []
EOF

mkdir src
touch src/lib.rs

# To generate the lockfile and confirm the manifest is valid.
cargo check

# Note tokio v1.25.0 is selected. This is the version that 
# the path dependency on tokio-stream pulls in.
cargo tree --invert --package tokio
# Outputs:
# tokio v1.25.0 (.../tokio/tokio)
# ├── tokio-stream v0.1.12 (.../tokio/tokio-stream)
# │   └── rustdoc v0.0.0 (.../tokio/target/semver-checks/local-tokio_stream-0_1_12)
# └── tokio-util v0.7.7 (.../tokio/tokio-util)
#     └── tokio-stream v0.1.12 (.../tokio/tokio-stream) (*)

# Observe a bunch of crates get updated.
# But tokio is not one of them, even though 1.40 is available!
cargo update

# Even explicitly attempting to update tokio fails!
cargo update --precise 1.40.0 tokio
# Outputs:
#     Updating crates.io index
#     Updating tokio v1.25.0 (/.../tokio/tokio) -> v1.25.0
# note: pass `--verbose` to see 21 unchanged dependencies behind latest

Actual Behaviour

Two items:

(1) There should be a way to run cargo update inside a project with a path dependency to force it to use latest SemVer-compatible versions of dependencies.

(2) cargo update --precise 1.40.0 tokio should either upgrade tokio to 1.40.0 as requested, or should exit with an error. It should not completely ignore the 1.40.0 argument and exit cleanly.

Expected Behaviour

(1) cargo update silently does not update versions of path dependencies of a path dependency. Instead, it pins the path dependencies and their path dependencies exactly. There does not appear to be a way to override this.

The first cargo update run after the setup above may update some non-path-related dependencies. Subsequent cargo update runs will look like this, and will not upgrade tokio to 1.40.0 nor mention it in any way.

$ cargo update
    Updating crates.io index
     Locking 0 packages to latest compatible versions
note: pass `--verbose` to see 21 unchanged dependencies behind latest

$ cargo update --verbose
    Updating crates.io index
     Locking 0 packages to latest compatible versions
   Unchanged mio v0.8.11 (latest: v1.0.2)
   Unchanged socket2 v0.4.10 (latest: v0.5.7)
   Unchanged wasi v0.11.0+wasi-snapshot-preview1 (latest: v0.13.2+wasi-0.2.1)
   Unchanged windows-sys v0.45.0 (latest: v0.59.0)
   Unchanged windows-sys v0.48.0 (latest: v0.59.0)
   Unchanged windows-targets v0.42.2 (latest: v0.52.6)
   Unchanged windows-targets v0.48.5 (latest: v0.52.6)
   Unchanged windows_aarch64_gnullvm v0.42.2 (latest: v0.52.6)
   Unchanged windows_aarch64_gnullvm v0.48.5 (latest: v0.52.6)
   Unchanged windows_aarch64_msvc v0.42.2 (latest: v0.52.6)
   Unchanged windows_aarch64_msvc v0.48.5 (latest: v0.52.6)
   Unchanged windows_i686_gnu v0.42.2 (latest: v0.52.6)
   Unchanged windows_i686_gnu v0.48.5 (latest: v0.52.6)
   Unchanged windows_i686_msvc v0.42.2 (latest: v0.52.6)
   Unchanged windows_i686_msvc v0.48.5 (latest: v0.52.6)
   Unchanged windows_x86_64_gnu v0.42.2 (latest: v0.52.6)
   Unchanged windows_x86_64_gnu v0.48.5 (latest: v0.52.6)
   Unchanged windows_x86_64_gnullvm v0.42.2 (latest: v0.52.6)
   Unchanged windows_x86_64_gnullvm v0.48.5 (latest: v0.52.6)
   Unchanged windows_x86_64_msvc v0.42.2 (latest: v0.52.6)
   Unchanged windows_x86_64_msvc v0.48.5 (latest: v0.52.6)

(2) cargo update --precise 1.40.0 tokio completely ignores the --precise 1.40.0 requirement, and exits cleanly as a no-op.

$ cargo update --precise 1.40.0 tokio
    Updating crates.io index
    Updating tokio v1.25.0 (/.../local_code/tokio/tokio) -> v1.25.0
note: pass `--verbose` to see 21 unchanged dependencies behind latest

Generated System Information

Software version

cargo-semver-checks 0.34.0

Operating system

Linux 5.15.153.1-microsoft-standard-WSL2

Command-line

/.../.cargo/bin/cargo-semver-checks semver-checks --bugreport 

cargo version

> cargo -V 
cargo 1.80.1 (376290515 2024-07-16)

Compile time information

Build Configuration

No response

Additional Context

No response

obi1kenobi commented 1 week ago

The invocation of cargo-semver-checks that triggers the false-positives is this:

$ cargo semver-checks --manifest-path ../tokio/tokio-stream/Cargo.toml --release-type minor --exclude benches --exclude examples --exclude stress-test --exclude tests-build --exclude tests-integration
    Finished `release` profile [optimized] target(s) in 0.18s
     Running `target/release/cargo-semver-checks semver-checks --manifest-path ../tokio/tokio-stream/Cargo.toml --release-type minor --exclude benches --exclude examples --exclude stress-test --exclude tests-build --exclude tests-integration`
     Parsing tokio-stream v0.1.12 (current)
      Parsed [   0.869s] (current)
     Parsing tokio-stream v0.1.12 (baseline, cached)
      Parsed [   0.039s] (baseline)
    Checking tokio-stream v0.1.12 -> v0.1.12 (assume minor change)
     Checked [   0.014s] 80 checks: 79 pass, 1 fail, 0 warn, 7 skip

--- failure auto_trait_impl_removed: auto trait no longer implemented ---

Description:
A public type has stopped implementing one or more auto traits. This can break downstream code that depends on the traits being implemented.
        ref: https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.34.0/src/lints/auto_trait_impl_removed.ron

Failed in:
  type ReceiverStream is no longer UnwindSafe, in /.../tokio/tokio-stream/src/wrappers/mpsc_bounded.rs:11
  type ReceiverStream is no longer RefUnwindSafe, in /.../tokio/tokio-stream/src/wrappers/mpsc_bounded.rs:11
  type UnboundedReceiverStream is no longer UnwindSafe, in /.../tokio/tokio-stream/src/wrappers/mpsc_unbounded.rs:11
  type UnboundedReceiverStream is no longer RefUnwindSafe, in /.../tokio/tokio-stream/src/wrappers/mpsc_unbounded.rs:11

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [   0.966s] tokio-stream
obi1kenobi commented 1 week ago

cc @Darksonn on the off chance you notice this while working on tokio.

Scanning on new tokio versions should be fine, since the path dependency on tokio will point past 1.40.0. But re-scanning older commits can trigger this.

If you have opinions the cargo behavior, or ideas on how to get it to update transitive path dependencies to latest compatible versions, I'd love to hear them!

Darksonn commented 1 week ago

Interesting. Our CI job doesn't get any lock file as input, so I don't think we will hit this.

It sounds like the proper solution would be for tokio-util to use the local Tokio in both builds being compared?

obi1kenobi commented 1 week ago

I agree, I don't think you'll hit it either. Just wanted to preemptively warn you just in case, so you don't end up debugging a phantom breaking change.

Yes, we'll have to figure out a way to use the same version of tokio (and all other dependencies) on both sides of the comparison. Otherwise it's apples to oranges. I'm hoping we can somehow get cargo to use the version number from the transitive dependency, instead of the path component of the transitive path dependency. But I haven't figured out a way to do that yet.

Darksonn commented 1 week ago

Thanks for the heads up. I guess it could potentially affect our branches used for making backports to LTS releases.