Skip to content

Commit 71678a4

Browse files
authored
fix(package): show dirty filepaths relative to git workdir (#14968)
### 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
2 parents 9c17646 + 982eb2d commit 71678a4

File tree

2 files changed

+143
-4
lines changed

2 files changed

+143
-4
lines changed

src/cargo/ops/cargo_package.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ fn check_repo_state(
796796
.and_then(|p| p.to_str())
797797
.unwrap_or("")
798798
.replace("\\", "/");
799-
let Some(git) = git(p, src_files, &repo, &opts)? else {
799+
let Some(git) = git(src_files, &repo, &opts)? else {
800800
// If the git repo lacks essensial field like `sha1`, and since this field exists from the beginning,
801801
// then don't generate the corresponding file in order to maintain consistency with past behavior.
802802
return Ok(None);
@@ -805,7 +805,6 @@ fn check_repo_state(
805805
return Ok(Some(VcsInfo { git, path_in_vcs }));
806806

807807
fn git(
808-
pkg: &Package,
809808
src_files: &[PathBuf],
810809
repo: &git2::Repository,
811810
opts: &PackageOpts<'_>,
@@ -824,11 +823,12 @@ fn check_repo_state(
824823
// Find the intersection of dirty in git, and the src_files that would
825824
// be packaged. This is a lazy n^2 check, but seems fine with
826825
// thousands of files.
827-
let dirty_src_files: Vec<String> = src_files
826+
let workdir = repo.workdir().unwrap();
827+
let mut dirty_src_files: Vec<_> = src_files
828828
.iter()
829829
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
830830
.map(|path| {
831-
path.strip_prefix(pkg.root())
831+
path.strip_prefix(workdir)
832832
.unwrap_or(path)
833833
.display()
834834
.to_string()
@@ -847,6 +847,7 @@ fn check_repo_state(
847847
dirty,
848848
}))
849849
} else {
850+
dirty_src_files.sort_unstable();
850851
anyhow::bail!(
851852
"{} files in the working directory contain changes that were \
852853
not yet committed into git:\n\n{}\n\n\

tests/testsuite/package.rs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,144 @@ src/lib.rs
11561156
.run();
11571157
}
11581158

1159+
#[cargo_test]
1160+
fn vcs_status_check_for_each_workspace_member() {
1161+
// Cargo checks VCS status separately for each workspace member.
1162+
// This ensure one file changed in a package won't affect the other.
1163+
// Since the dirty bit in .cargo_vcs_info.json is just for advisory purpose,
1164+
// We may change the meaning of it in the future.
1165+
let (p, repo) = git::new_repo("foo", |p| {
1166+
p.file(
1167+
"Cargo.toml",
1168+
r#"
1169+
[workspace]
1170+
members = ["isengard", "mordor"]
1171+
"#,
1172+
)
1173+
.file("hobbit", "...")
1174+
.file(
1175+
"isengard/Cargo.toml",
1176+
r#"
1177+
[package]
1178+
name = "isengard"
1179+
edition = "2015"
1180+
homepage = "saruman"
1181+
description = "saruman"
1182+
license = "MIT"
1183+
"#,
1184+
)
1185+
.file("isengard/src/lib.rs", "")
1186+
.file(
1187+
"mordor/Cargo.toml",
1188+
r#"
1189+
[package]
1190+
name = "mordor"
1191+
edition = "2015"
1192+
homepage = "sauron"
1193+
description = "sauron"
1194+
license = "MIT"
1195+
"#,
1196+
)
1197+
.file("mordor/src/lib.rs", "")
1198+
});
1199+
git::commit(&repo);
1200+
1201+
p.change_file(
1202+
"Cargo.toml",
1203+
r#"
1204+
[workspace]
1205+
members = ["isengard", "mordor"]
1206+
[workspace.package]
1207+
edition = "2021"
1208+
"#,
1209+
);
1210+
// Dirty file outside won't affect packaging.
1211+
p.change_file("hobbit", "changed!");
1212+
p.change_file("mordor/src/lib.rs", "changed!");
1213+
p.change_file("mordor/src/main.rs", "fn main() {}");
1214+
1215+
// Ensure dirty files be reported only for one affected package.
1216+
p.cargo("package --workspace --no-verify")
1217+
.with_status(101)
1218+
.with_stderr_data(str![[r#"
1219+
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
1220+
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
1221+
[ERROR] 2 files in the working directory contain changes that were not yet committed into git:
1222+
1223+
mordor/src/lib.rs
1224+
mordor/src/main.rs
1225+
1226+
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
1227+
1228+
"#]])
1229+
.run();
1230+
1231+
// Ensure only dirty package be recorded as dirty.
1232+
p.cargo("package --workspace --no-verify --allow-dirty")
1233+
.with_stderr_data(str![[r#"
1234+
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
1235+
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
1236+
[PACKAGING] mordor v0.0.0 ([ROOT]/foo/mordor)
1237+
[PACKAGED] 6 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
1238+
1239+
"#]])
1240+
.run();
1241+
1242+
let f = File::open(&p.root().join("target/package/isengard-0.0.0.crate")).unwrap();
1243+
validate_crate_contents(
1244+
f,
1245+
"isengard-0.0.0.crate",
1246+
&[
1247+
".cargo_vcs_info.json",
1248+
"Cargo.toml",
1249+
"Cargo.toml.orig",
1250+
"src/lib.rs",
1251+
"Cargo.lock",
1252+
],
1253+
[(
1254+
".cargo_vcs_info.json",
1255+
// No change within `isengard/`, so not dirty at all.
1256+
str![[r#"
1257+
{
1258+
"git": {
1259+
"sha1": "[..]"
1260+
},
1261+
"path_in_vcs": "isengard"
1262+
}
1263+
"#]]
1264+
.is_json(),
1265+
)],
1266+
);
1267+
1268+
let f = File::open(&p.root().join("target/package/mordor-0.0.0.crate")).unwrap();
1269+
validate_crate_contents(
1270+
f,
1271+
"mordor-0.0.0.crate",
1272+
&[
1273+
".cargo_vcs_info.json",
1274+
"Cargo.toml",
1275+
"Cargo.toml.orig",
1276+
"src/lib.rs",
1277+
"src/main.rs",
1278+
"Cargo.lock",
1279+
],
1280+
[(
1281+
".cargo_vcs_info.json",
1282+
// Dirty bit is recorded.
1283+
str![[r#"
1284+
{
1285+
"git": {
1286+
"dirty": true,
1287+
"sha1": "[..]"
1288+
},
1289+
"path_in_vcs": "mordor"
1290+
}
1291+
"#]]
1292+
.is_json(),
1293+
)],
1294+
);
1295+
}
1296+
11591297
#[cargo_test]
11601298
fn issue_13695_allow_dirty_vcs_info() {
11611299
let p = project()

0 commit comments

Comments
 (0)