Skip to content

Commit f2c4849

Browse files
authored
fix(package): detect dirtiness for symlinks to submodule (#15416)
### What does this PR try to resolve? If a there is a symlink into a git repository/submodule, when checking its git status with the wrong outer repo, we'll get an NotFound error, as the object doesn't belong to the outer repository. This kind of error blocked the entire `cargo package` operation. This fix additionally discovers the nearest Git repository, and then checks status with that, assuming the repo is the parent of the source file of the symlink. This is a best effort solution, so if the check fails we ignore. ### How should we test and review this PR? If we don't want the complication, we could drop the last commit, ignore the error, and forget about handling submodules fixes #15384 fixes #15413
2 parents 0acc1db + 71ea2e5 commit f2c4849

File tree

2 files changed

+88
-2
lines changed

2 files changed

+88
-2
lines changed

src/cargo/ops/cargo_package/vcs.rs

+35-2
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,41 @@ fn dirty_files_outside_pkg_root(
268268
// Handle files outside package root but under git workdir,
269269
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
270270
{
271-
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
272-
dirty_files.insert(workdir.join(rel_path));
271+
match repo.status_file(&rel_path) {
272+
Ok(git2::Status::CURRENT) => {}
273+
Ok(_) => {
274+
dirty_files.insert(workdir.join(rel_path));
275+
}
276+
Err(e) => {
277+
if e.code() == git2::ErrorCode::NotFound {
278+
// Object not found means this file might be inside a subrepo/submodule.
279+
// Let's check its status from that repo.
280+
let abs_path = workdir.join(&rel_path);
281+
if let Ok(repo) = git2::Repository::discover(&abs_path) {
282+
let is_dirty = if repo.workdir() == Some(workdir) {
283+
false
284+
} else if let Ok(path) =
285+
paths::strip_prefix_canonical(&abs_path, repo.workdir().unwrap())
286+
{
287+
repo.status_file(&path) != Ok(git2::Status::CURRENT)
288+
} else {
289+
false
290+
};
291+
if is_dirty {
292+
dirty_files.insert(abs_path);
293+
}
294+
}
295+
}
296+
297+
// Dirtiness check for symlinks is mostly informational.
298+
// To avoid adding more complicated logic,
299+
// for now we ignore the status check failure.
300+
debug!(
301+
"failed to get status from file `{}` in git repo at `{}`: {e}",
302+
rel_path.display(),
303+
workdir.display()
304+
);
305+
}
273306
}
274307
}
275308
Ok(dirty_files)

tests/testsuite/package.rs

+53
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,59 @@ edition = "2021"
14251425
);
14261426
}
14271427

1428+
#[cargo_test]
1429+
fn dirty_file_outside_pkg_root_inside_submodule() {
1430+
if !symlink_supported() {
1431+
return;
1432+
}
1433+
let (p, repo) = git::new_repo("foo", |p| {
1434+
p.file(
1435+
"Cargo.toml",
1436+
r#"
1437+
[workspace]
1438+
members = ["isengard"]
1439+
resolver = "2"
1440+
"#,
1441+
)
1442+
.file(
1443+
"isengard/Cargo.toml",
1444+
r#"
1445+
[package]
1446+
name = "isengard"
1447+
edition = "2015"
1448+
homepage = "saruman"
1449+
description = "saruman"
1450+
license = "ISC"
1451+
"#,
1452+
)
1453+
.file("isengard/src/lib.rs", "")
1454+
});
1455+
let submodule = git::new("submodule", |p| {
1456+
p.no_manifest().file("file.txt", "from-submodule")
1457+
});
1458+
git::add_submodule(
1459+
&repo,
1460+
&submodule.root().to_url().to_string(),
1461+
Path::new("submodule"),
1462+
);
1463+
p.symlink("submodule/file.txt", "isengard/src/file.txt");
1464+
git::add(&repo);
1465+
git::commit(&repo);
1466+
p.change_file("submodule/file.txt", "changed");
1467+
1468+
p.cargo("package --workspace --no-verify")
1469+
.with_status(101)
1470+
.with_stderr_data(str![[r#"
1471+
[ERROR] 1 files in the working directory contain changes that were not yet committed into git:
1472+
1473+
submodule/file.txt
1474+
1475+
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
1476+
1477+
"#]])
1478+
.run();
1479+
}
1480+
14281481
#[cargo_test]
14291482
fn issue_13695_allow_dirty_vcs_info() {
14301483
let p = project()

0 commit comments

Comments
 (0)