Include cargo_vcs_info.json even for dirty working directories

[kornelski]

Problem

Currently, Cargo includes a .cargo_vcs_info.json file when publishing, but only if the working copy is clean and --allow-dirty is not used.

However, this metadata is needed regardless whether the directory was modified or not. It's unlikely the path in repo would change, and it is still needed for resolving relative paths in README, creating links to the crate, and still helpful for quickly finding the crate in its repository.

Even having the commit hash is still useful, because it provides a base commit to diff from.

There are situations where it's still necessary to edit the Cargo.toml file, e.g. to publish crates with circular dev dependencies (#4242), or explicitly specified tests/benchmarks that are excluded from the tarball (#13456). In this case the commit hash is basically still accurate.

Note that Cargo omitting the commit hash when the directory is dirty is not a guarantee that packages with a commit hash are matching their commit. Dishonest users can always modify Cargo to make it lie or upload a manipulated tarball themselves, so this file is informational, not an integrity guarantee.

Proposed Solution

Cargo could always include the commit hash when available. Perhaps add "dirty":true if reporting that state is deemed important. Alternatively, provide an option of --allow-dirty-but-still-include-the-commit-hash.

Notes

No response

[epage]

Is there a reason to be mutating Cargo.toml for dev-dependencies these days with the stripping of non-registry dev-dependencies?

For #13456, I am actively working on a fix. I posted the last major refactor today (#13693) before I can make the relevant change.

That said, I agree with the idea and it makes me wonder why this wasn't considered before. #5886 and its predecessor don't really say much on motivation. Same with #9866.

[epage]

The biggest risk with this is if someone assumes the presence of cargo_vcs_info.json means the .crate is clean.

I talked to @joshtriplett to get some historical perspective and they don't see an issue and don't think that above use case should be large enough for us to work around (e.g. adding a cargo_vcs_info-2.json file)

[kornelski]

https://github.com/tokio-rs/async-stream/blob/v0.3.5/async-stream/Cargo.toml#L23
https://docs.rs/crate/async-stream/0.3.5/source/Cargo.toml.orig

is an example of a crate that removed non-workspace crates-io dev-dep to avoid a cycle (tokio-test)

[epage]

I'm not seeing tokio-test in the source Cargo.toml.

I'm also not too sure the point you are trying to get across. In theory, there isn't a reason to do that manually anymore. However, that is independent of whether we go forward with this and I've marked it as Accepted though I'm assuming we'd hold an FCP for it but I expect that to be relatively smooth.

[VorpalBlade]

The biggest risk with this is if someone assumes the presence of cargo_vcs_info.json means the .crate is clean.

Adding a "dirty" field would help here, unless they are just looking for the file (not the contents of the file. In which case there really isn't much you can do about it.

Another use case here is that of briansmith/ring#1460 (comment). TLDR: ring has a perl script(!) to generate assembly files, and also pre-compiles these for Windows so users don't need nasm to build ring. I can only assume that this predates stable support for inline assembly / global assembly in Rust. But I can imagine other similar use cases, where you want to pre-generate some data that requires exotic tools / takes an excessive amount of time to build. It's not ideal of course, but the real world rarely is.