Skip to content

Commit

Permalink
fix(package): show dirty filepaths relative to git workdir (#14968)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

Dirty file paths in the original message were stripped relative to
package root.
User is not able to know the full path to find dirty files.

This PR makes it relative to git workdir.

### How should we test and review this PR?

This was found during #14962
  • Loading branch information
epage authored Dec 19, 2024
2 parents 9c17646 + 982eb2d commit 71678a4
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 4 deletions.
9 changes: 5 additions & 4 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ fn check_repo_state(
.and_then(|p| p.to_str())
.unwrap_or("")
.replace("\\", "/");
let Some(git) = git(p, src_files, &repo, &opts)? else {
let Some(git) = git(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);
Expand All @@ -805,7 +805,6 @@ fn check_repo_state(
return Ok(Some(VcsInfo { git, path_in_vcs }));

fn git(
pkg: &Package,
src_files: &[PathBuf],
repo: &git2::Repository,
opts: &PackageOpts<'_>,
Expand All @@ -824,11 +823,12 @@ fn check_repo_state(
// Find the intersection of dirty in git, and the src_files that would
// be packaged. This is a lazy n^2 check, but seems fine with
// thousands of files.
let dirty_src_files: Vec<String> = src_files
let workdir = repo.workdir().unwrap();
let mut dirty_src_files: Vec<_> = src_files
.iter()
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
.map(|path| {
path.strip_prefix(pkg.root())
path.strip_prefix(workdir)
.unwrap_or(path)
.display()
.to_string()
Expand All @@ -847,6 +847,7 @@ fn check_repo_state(
dirty,
}))
} else {
dirty_src_files.sort_unstable();
anyhow::bail!(
"{} files in the working directory contain changes that were \
not yet committed into git:\n\n{}\n\n\
Expand Down
138 changes: 138 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,144 @@ src/lib.rs
.run();
}

#[cargo_test]
fn vcs_status_check_for_each_workspace_member() {
// Cargo checks VCS status separately for each workspace member.
// This ensure one file changed in a package won't affect the other.
// Since the dirty bit in .cargo_vcs_info.json is just for advisory purpose,
// We may change the meaning of it in the future.
let (p, repo) = git::new_repo("foo", |p| {
p.file(
"Cargo.toml",
r#"
[workspace]
members = ["isengard", "mordor"]
"#,
)
.file("hobbit", "...")
.file(
"isengard/Cargo.toml",
r#"
[package]
name = "isengard"
edition = "2015"
homepage = "saruman"
description = "saruman"
license = "MIT"
"#,
)
.file("isengard/src/lib.rs", "")
.file(
"mordor/Cargo.toml",
r#"
[package]
name = "mordor"
edition = "2015"
homepage = "sauron"
description = "sauron"
license = "MIT"
"#,
)
.file("mordor/src/lib.rs", "")
});
git::commit(&repo);

p.change_file(
"Cargo.toml",
r#"
[workspace]
members = ["isengard", "mordor"]
[workspace.package]
edition = "2021"
"#,
);
// Dirty file outside won't affect packaging.
p.change_file("hobbit", "changed!");
p.change_file("mordor/src/lib.rs", "changed!");
p.change_file("mordor/src/main.rs", "fn main() {}");

// Ensure dirty files be reported only for one affected package.
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
[ERROR] 2 files in the working directory contain changes that were not yet committed into git:
mordor/src/lib.rs
mordor/src/main.rs
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
"#]])
.run();

// Ensure only dirty package be recorded as dirty.
p.cargo("package --workspace --no-verify --allow-dirty")
.with_stderr_data(str![[r#"
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
[PACKAGING] mordor v0.0.0 ([ROOT]/foo/mordor)
[PACKAGED] 6 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
"#]])
.run();

let f = File::open(&p.root().join("target/package/isengard-0.0.0.crate")).unwrap();
validate_crate_contents(
f,
"isengard-0.0.0.crate",
&[
".cargo_vcs_info.json",
"Cargo.toml",
"Cargo.toml.orig",
"src/lib.rs",
"Cargo.lock",
],
[(
".cargo_vcs_info.json",
// No change within `isengard/`, so not dirty at all.
str![[r#"
{
"git": {
"sha1": "[..]"
},
"path_in_vcs": "isengard"
}
"#]]
.is_json(),
)],
);

let f = File::open(&p.root().join("target/package/mordor-0.0.0.crate")).unwrap();
validate_crate_contents(
f,
"mordor-0.0.0.crate",
&[
".cargo_vcs_info.json",
"Cargo.toml",
"Cargo.toml.orig",
"src/lib.rs",
"src/main.rs",
"Cargo.lock",
],
[(
".cargo_vcs_info.json",
// Dirty bit is recorded.
str![[r#"
{
"git": {
"dirty": true,
"sha1": "[..]"
},
"path_in_vcs": "mordor"
}
"#]]
.is_json(),
)],
);
}

#[cargo_test]
fn issue_13695_allow_dirty_vcs_info() {
let p = project()
Expand Down

0 comments on commit 71678a4

Please sign in to comment.