Skip to content

Commit

Permalink
Auto merge of #14359 - linyihai:issue-14354, r=weihanglo
Browse files Browse the repository at this point in the history
Fix: `cargo package` failed on bare commit git repo.

### What does this PR try to resolve?
Fixes #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
  • Loading branch information
bors committed Aug 8, 2024
2 parents 0d8d22f + 37cda2d commit b66cad8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 7 deletions.
21 changes: 14 additions & 7 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,12 @@ fn check_repo_state(
.and_then(|p| p.to_str())
.unwrap_or("")
.replace("\\", "/");
return Ok(Some(VcsInfo {
git: git(p, src_files, &repo, &opts)?,
path_in_vcs,
}));
let Some(git) = git(p, src_files, &repo, &opts)? else {
// If the git repo lacks essensial field like `sha1`, and since this field exists from the beginning,
// then don't generate the corresponding file in order to maintain consistency with past behavior.
return Ok(None);
};
return Ok(Some(VcsInfo { git, path_in_vcs }));
}
}
gctx.shell().verbose(|shell| {
Expand All @@ -804,7 +806,7 @@ fn check_repo_state(
src_files: &[PathBuf],
repo: &git2::Repository,
opts: &PackageOpts<'_>,
) -> CargoResult<GitVcsInfo> {
) -> CargoResult<Option<GitVcsInfo>> {
// This is a collection of any dirty or untracked files. This covers:
// - new/modified/deleted/renamed/type change (index or worktree)
// - untracked files (which are "new" worktree files)
Expand All @@ -831,11 +833,16 @@ fn check_repo_state(
.collect();
let dirty = !dirty_src_files.is_empty();
if !dirty || opts.allow_dirty {
// Must check whetherthe repo has no commit firstly, otherwise `revparse_single` would fail on bare commit repo.
// Due to lacking the `sha1` field, it's better not record the `GitVcsInfo` for consistency.
if repo.is_empty()? {
return Ok(None);
}
let rev_obj = repo.revparse_single("HEAD")?;
Ok(GitVcsInfo {
Ok(Some(GitVcsInfo {
sha1: rev_obj.id().to_string(),
dirty,
})
}))
} else {
anyhow::bail!(
"{} files in the working directory contain changes that were \
Expand Down
30 changes: 30 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,36 @@ fn issue_13695_allowing_dirty_vcs_info_but_clean() {
);
}

#[cargo_test]
fn issue_14354_allowing_dirty_bare_commit() {
let p = project().build();
// Init a bare commit git repo
let _ = git::repo(&paths::root().join("foo"))
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
description = "foo"
license = "foo"
documentation = "foo"
"#,
)
.file("src/lib.rs", "");

p.cargo("package --allow-dirty").run();

let f = File::open(&p.root().join("target/package/foo-0.1.0.crate")).unwrap();
validate_crate_contents(
f,
"foo-0.1.0.crate",
&["Cargo.toml", "Cargo.toml.orig", "src/lib.rs"],
&[],
);
}

#[cargo_test]
fn generated_manifest() {
let registry = registry::alt_init();
Expand Down

0 comments on commit b66cad8

Please sign in to comment.