Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lolbinarycat opened this issue May 30, 2024 · 4 comments
Open

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

lolbinarycat opened this issue May 30, 2024 · 4 comments
Labels
C-bug Category: bug Command-publish S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@lolbinarycat
Copy link

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]
@lolbinarycat lolbinarycat added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 30, 2024
@epage
Copy link
Contributor

epage commented May 30, 2024

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?

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels May 30, 2024
@lolbinarycat
Copy link
Author

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

@epage
Copy link
Contributor

epage commented May 30, 2024

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 epage added S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels May 30, 2024
@epage
Copy link
Contributor

epage commented May 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-publish S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

No branches or pull requests

3 participants