rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.73k stars 2.42k forks source link

cargo publish should refuse an out-of-date lockfile #13986

Open lolbinarycat opened 5 months ago

lolbinarycat commented 5 months ago

Problem

updating a package's version in Cargo.toml, running cargo publish, then creating a tagged commit will cause a desync between the commit and the version on crates.io in the lockfile.

the crates.io version will have the appropriate version in Cargo.lock, but the tagged commit will have the wrong version info in the lockfile, causing problems for anyone who tries to build that version with cargo build --locked

Steps

  1. make a package in a git repo
  2. make an init commit with everything checked in
  3. change the package.version field in Cargo.toml
  4. run cargo publish
  5. make a new tagged commit
  6. run cargo build --locked

Possible Solution(s)

make cargo publish refuse to publish if the lockfile is out of date

Notes

this is particularly annoying when trying to package a rust program with nix, often the only solution is to vendor a fixed lockfile

Version

release: 1.80.0-nightly
commit-hash: a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
commit-date: 2024-05-24
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: NixOS 23.11.0 [64-bit]
epage commented 5 months ago

To double check my understand, cargo publish is updating the lockfile on disk as part of its operation. Does cargo publish fail, saying that the filesystem is dirty and you need to pass --allow-dirty?

lolbinarycat commented 5 months ago

no, cargo publish succeeds, and publishes a different version of the lockfile than the one that exists on disk.

epage commented 5 months ago

Ah, I was looking at the wrong layer of abstraction. cargo package calls into package which updates the lockfile but cargo publish calls into package_one which is after that lockfile update.

$ cargo publish -n
   Compiling cargo v0.81.0 (/home/epage/src/personal/cargo)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 10.03s
     Running `/home/epage/src/personal/cargo/target/debug/cargo -Zscript publish -n`
    Updating crates.io index
warning: manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
   Packaging cargo-13986 v0.1.1 (/home/epage/src/personal/dump/cargo-13986)
   Verifying cargo-13986 v0.1.1 (/home/epage/src/personal/dump/cargo-13986)
   Compiling cargo-13986 v0.1.1 (/home/epage/src/personal/dump/cargo-13986/target/package/cargo-13986-0.1.1)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.26s
    Packaged 6 files, 1.1KiB (857.0B compressed)
   Uploading cargo-13986 v0.1.1 (/home/epage/src/personal/dump/cargo-13986)
warning: aborting upload due to dry run

$ cargo package
warning: manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
error: 1 files in the working directory contain changes that were not yet committed into git:

Cargo.lock

to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
epage commented 5 months ago

I worry that there are shenanigans that people do during publish such that having cargo publish also generate the lockfile would be a breaking change.

I think changing the behavior in the presence of --locked would be reasonable. If people are relying on that, then they are going counter to the documentation. Still not a great answer because you have to know to use it.

For myself, after doing enough releases, I feel like cargo publish is best left to plumbing and people should be using a higher level release workflow as talked about at https://doc.rust-lang.org/cargo/reference/publishing.html#publishing-a-new-version-of-an-existing-crate