Cargo semver checks ignores my lockfile

[epage]

See

• https://github.com/rust-lang/cargo/actions/runs/8043357911/job/21965310440
• https://github.com/rust-lang/cargo/blob/master/ci/validate-version-bump.sh

For more context, see also zulip

According to the zulip thread, this is done because

• "baseline" for crates.io won't have a lockfile and will use the latest dependencies. If one of those is a public dependency and is re-exported, then we need to make sure the versions are aligned
  • In cargo's case, we are verifying a git revision which I don't think this applies here
• to verify the latest versions of your dependencies
  • This is outside of the scope of cargo-semver-checks, see instead Verifying latest dependencies

The problems with doing this

• This adds non-determinism to CI jobs, increasing the chance of failure (the problem Cargo ran into)
• There are times when "latest dependencies" can't be calculated
  • e.g. There are corner cases with multi-major-version version requirements where the wrong version gets selected

[obi1kenobi]

Thanks!

What would be an alternative to what we're doing today?

Could c-s-c somehow ask cargo to reuse the dependency versions of another lockfile while generating the baseline rustdoc JSON? While what we're doing today could be argued to be a hack, any alternative seems like an even worse hack.

There are corner cases with multi-major-version version requirements where the wrong version gets selected

Could you say more about this, or point me to an issue where I can learn about it? We defer dependency version selection to cargo so of course if cargo does the wrong thing then c-s-c will as well.

[obi1kenobi]

Updated the title to avoid potential confusion. cargo-semver-checks would never delete your lockfile, it merely ignores it.

[epage]

I disagree with changing the title and reverted it back. It doesn't just ignore it, it changes my repo. My experience with a lot of developers is that they see a change in the repo and assume its intentional and commit it, making this another source of problems. A general rule of well-formed cargo commands is they should only modify the lockfile when there is a mismatch with Cargo.toml.

[obi1kenobi]

It doesn't just ignore it, it changes my repo.

Could you provide a repro where running cargo semver-checks in a clean checkout makes a change in the repo? If this is accurate, it's a serious bug that I'd like to look into immediately.

The current implementation is designed to create its own lockfile in a separate directory inside the target dir, and should never ever mess with the repo's Cargo.lock or any other file in the project. If that isn't the case, something is seriously wrong.

[epage]

Thanks for the clarification that it only ignores and doesn't modify my repo. I've reverted the title back to you version.

[obi1kenobi]

Thanks! If you have a sec, I'd love your thoughts on this earlier comment as well 👇

What would be an alternative to what we're doing today?

Could c-s-c somehow ask cargo to reuse the dependency versions of another lockfile while generating the baseline rustdoc JSON? While what we're doing today could be argued to be a hack, any alternative seems like an even worse hack.

There are corner cases with multi-major-version version requirements where the wrong version gets selected

Could you say more about this, or point me to an issue where I can learn about it? We defer dependency version selection to cargo so of course if cargo does the wrong thing then c-s-c will as well.

[epage]

I'll re-phrase my earlier comment: well-formed cargo commands should respect the existing lockfile.

When running with a git baseline, there is no problem as a lockfile is present if its intended to be.

Yes, there are questions on what to do with registry baselines. I don't have a magical end-all answer. This is something that will likely take exploration. Possible ideas

• Use the lockfile in the .crate, if present (this would be limited because of Heuristics to include Cargo.lock or not in a package are suboptimal rust-lang/cargo#13447)
• If this is intended with lockfiles, copy over the existing lockfile and let Cargo fill in the pieces.

There are corner cases with multi-major-version version requirements where the wrong version gets selected

Could you say more about this, or point me to an issue where I can learn about it? We defer dependency version selection to cargo so of course if cargo does the wrong thing then c-s-c will as well.

I was vague because I haven't been touching those projects in a well. The rough idea is that cargo resolves dependencies maximally so if you have two paths to a dependency with links (or its used in public APIs, or both) and one has a multi-major version version requirement, the latest will be picked while a more normal version requirement will pick a lower version. rust-lang/cargo#9029 is the resolver issue for this.

[obi1kenobi]

Thanks, that makes sense! I subscribed to the issue you linked as well — I didn't know cargo heuristically decided whether to include the lockfile in the crate.

copy over the existing lockfile and let Cargo fill in the pieces

To generate rustdoc for the current code, we create a new project and declare a path dependency on the crate being checked. As pseudocode, we do more or less the equivalent of:

cargo new --lib placeholder && \
    cd placeholder && \
    cargo add --path /path/to/crate/being/checked && \
    cargo doc -p crate_being_checked --output-format json

Would copying over the existing project's lockfile and letting Cargo fill in the pieces work here? That sounds ideal, but my prior assumption was that it wouldn't work since the lockfile is for a completely different project. I'd love to be pleasantly surprised about that.