Skip to content

Failed to cargo package --allow-dirty for repo with no commit #14354

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

Closed
weihanglo opened this issue Aug 5, 2024 · 9 comments · Fixed by #14359
Closed

Failed to cargo package --allow-dirty for repo with no commit #14354

weihanglo opened this issue Aug 5, 2024 · 9 comments · Fixed by #14359
Assignees
Labels
A-git Area: anything dealing with git C-bug Category: bug Command-package regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage.

Comments

@weihanglo
Copy link
Member

weihanglo commented Aug 5, 2024

Problem

#13960 unconditionally checks VCS status when doing cargo package.
However, a VCS (Git specifically) repository could have no commit history, and then it fails cargo package with such an error:

cargo +beta package --allow-dirty
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: revspec 'HEAD' not found; class=Reference (4); code=NotFound (-3)

Steps

  1. cargo new foo
  2. cd foo
  3. cargo package --allow-dirty

Possible Solution(s)

The current .cargo_vcs_info.json format looks like:

{
 "git": {
   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302",
   "dirty": true
 },
 "path_in_vcs": ""
}

If the HEAD hash cannot be determined, should we instead emit a JSON like

{
 "git": {
   "dirty": true
 },
 "path_in_vcs": ""
}

or just don't generate .cargo_vcs_info.json at all?

Notes

No response

Version

cargo 1.81.0-beta.2 (a2b58c3da 2024-07-16)
release: 1.81.0-beta.2
commit-hash: a2b58c3dad4d554ba01ed6c45c41ff85390560f2
commit-date: 2024-07-16
@weihanglo weihanglo added C-bug Category: bug A-git Area: anything dealing with git Command-package regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage. labels Aug 5, 2024
@linyihai
Copy link
Contributor

linyihai commented Aug 6, 2024

@rustbot claim

@weihanglo
Copy link
Member Author

Here are some possible solutions:

  1. Don't include .cargo_vcs_info.json at all
    • This maintains the old behavior without blocking cargo package.
  2. Don't include the git.sha key.
    • The git.dirty is still here to tell it is a dirty Git repo without a commit.
  3. Don't include the entire git object.
    • A Git repository without any commit is kinda useless so stripping it off.
  4. Treat it as a non-Git package.
    • Non-git package is assumed clean and will not include .cargo_vcs_info.json.

Any other solutions?

Mind that this is a bit time-bound as it breaks the stable usage of Cargo.

@epage
Copy link
Contributor

epage commented Aug 8, 2024

Don't include .cargo_vcs_info.json at all

I lean towards this seems the least controversial starting point, especially if this is time bound.

Mind that this is a bit time-bound as it breaks the stable usage of Cargo.

While this is a stable-to-beta regression, I'm assuming the end-user impact is pretty minimal. I can't see it being terribly common (and most likely a mistake) to publish from an empty repo.

@weihanglo
Copy link
Member Author

Agree on least controversial and minimal impact parts. I'll go ahead and merge #14359.

@bors bors closed this as completed in b66cad8 Aug 8, 2024
weihanglo pushed a commit to weihanglo/cargo that referenced this issue Aug 9, 2024
Fix: `cargo package` failed on bare commit git repo.

### What does this PR try to resolve?
Fixes rust-lang#14354

This approach chose to not generate a `.cargo_vcs_info.json` for bare commit git repo.

### How should we test and review this PR?
Compare the test changes before and after the two commits

### Additional information
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Sep 6, 2024
@Byron
Copy link
Member

Byron commented May 18, 2025

And even though the respective test is succeeding, I still seem to be able to reproduce (something like?) it:

❯ gst
On branch master

No commits yet

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        Cargo.toml
        src/

nothing added to commit but untracked files present (use "git add" to track)

foo ( master) [?]
❯ cargo --version
cargo 1.86.0 (adf9b6ad1 2025-02-28)

foo ( master) [?]
❯ cargo package
error: 2 files in the working directory contain changes that were not yet committed into git:

Cargo.toml
src/lib.rs

to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag

foo ( master) [?]
❯ cargo package --allow-dirty
error: revspec 'HEAD' not found; class=Reference (4); code=NotFound (-3)

And the same is happening with a more recent version of cargo:

❯ cargo +nightly --version
cargo 1.88.0-nightly (7918c7eb5 2025-04-27)

foo ( master) [?]
❯ cargo +nightly package --allow-dirty
error: revspec 'HEAD' not found; class=Reference (4); code=NotFound (-3)
❯ cargo run --manifest-path ../../../../../Cargo.toml -- package --allow-dirty
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
     Running `/Users/byron/dev/github.com/rust-lang/cargo/target/debug/cargo package --allow-dirty`
error: revspec 'HEAD' not found; class=Reference (4); code=NotFound (-3)

I noticed this while researching for #15534, and I am wondering why the respective test succeeds.
Interestingly, it does catch this error in gitoxide, making it even more puzzling.

@weihanglo
Copy link
Member Author

I can't reproduce it on either latest nightly, beta, or 1.87.0 stable.

Unless the git_repository_is_empty function from libgit has some bugs, otherwise I don't really know what happened 🤔.

An empty repository has just been initialized and contains no references apart from HEAD, which must be pointing to the unborn master branch, or the branch specified for the repository in the init.defaultBranch configuration variable.

@Byron
Copy link
Member

Byron commented May 18, 2025

Ah, right, I have set my default branch to main via init.defaultBranch. The code that I think is responsible for that has been adjusted (in the gitoxide version), so I don't think this will be an issue anymore - after all it just wants to check for an unborn branch and not if the repo is pristine.

@Byron
Copy link
Member

Byron commented May 18, 2025

With the current version there are results immediately and it takes a little less than 12m to compile everything. The greatest expense seems to be the fetching of latest packages now.

aws-sdk-rust ( main) via 🐍
❯ /Users/byron/dev/github.com/rust-lang/cargo/target/release/cargo package --allow-dirty --no-verify --no-metadata
[..]
   Packaging aws-smithy-observability-otel v0.1.0 (/Users/byron/dev/github.com/awslabs/aws-sdk-rust/sdk/aws-smithy-observability-otel)
    Updating crates.io index
    Packaged 12 files, 83.4KiB (20.6KiB compressed)
   Packaging aws-smithy-types-convert v0.60.9 (/Users/byron/dev/github.com/awslabs/aws-sdk-rust/sdk/aws-smithy-types-convert)
    Updating crates.io index
    Packaged 10 files, 36.9KiB (11.0KiB compressed)
   Packaging aws-smithy-wasm v0.1.4 (/Users/byron/dev/github.com/awslabs/aws-sdk-rust/sdk/aws-smithy-wasm)
    Updating crates.io index
    Packaged 9 files, 40.2KiB (12.7KiB compressed)

aws-sdk-rust ( main) via 🐍 took 11m49s

@weihanglo
Copy link
Member Author

Yeah duplicate index update is definitely a thing we could avoid (in a separate PR though).

BTW we're commenting on the wrong issue 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-git Area: anything dealing with git C-bug Category: bug Command-package regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants