itsscb/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo.git synced 2025-09-28 11:20:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "fix(package): detect dirtiness for symlinks to submodule"

Browse Source 
This reverts commit 71ea2e5c5fa285e8e0336d51fd03ba4a427154bf.

`Repository::discover` and `Repository::status_file` are too expenstive
to run inside a loop. And `cargo package` are doing a lot of duplicate
works for checking submodule VCS status.

The possible fix might look like

* Let `status_submodules` function returns a path entry set, so
  Cargo can check whether a source file is dirty based on that.
* When listing files in `PathSource`, attach the VCS status of a
  path entry assoicated with. Then subsequent operations can skip
  status check entirely.

The above solutions are not trivial, and the dirtiness check is
informational only based on T-cargo conclusion, so we should be
good just reverting the change now.

Again, the caveat of this is that we can't really detect
dirty symlinks that links into a Git submodule.
This commit is contained in:
Weihang Lo 2025-04-11 13:45:16 -04:00
parent 27366fdea4
commit 314df11c74
No known key found for this signature in database
GPG Key ID: D7DBF189825E82E7
2 changed files with 5 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/cargo/ops/cargo_package/vcs.rs
View File

@ -274,28 +274,9 @@ fn dirty_files_outside_pkg_root(
dirty_files.insert(workdir.join(rel_path));
}
Err(e) => {
if e.code() == git2::ErrorCode::NotFound {
// Object not found means this file might be inside a subrepo/submodule.
// Let's check its status from that repo.
let abs_path = workdir.join(&rel_path);
if let Ok(repo) = git2::Repository::discover(&abs_path) {
let is_dirty = if repo.workdir() == Some(workdir) {
false
} else if let Ok(path) =
paths::strip_prefix_canonical(&abs_path, repo.workdir().unwrap())
{
repo.status_file(&path) != Ok(git2::Status::CURRENT)
} else {
false
};
if is_dirty {
dirty_files.insert(abs_path);
}
}
}
// Dirtiness check for symlinks is mostly informational.
// To avoid adding more complicated logic,
// And changes in submodule would fail git-status as well (see #15384).
// To avoid adding complicated logic to handle that,
// for now we ignore the status check failure.
debug!(
"failed to get status from file `{}` in git repo at `{}`: {e}",

9
tests/testsuite/package.rs
View File

@ -1463,16 +1463,13 @@ fn dirty_file_outside_pkg_root_inside_submodule() {
p.symlink("submodule/file.txt", "isengard/src/file.txt");
git::add(&repo);
git::commit(&repo);
// This dirtyness should be detected in the future.
p.change_file("submodule/file.txt", "changed");
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] 1 files in the working directory contain changes that were not yet committed into git:
submodule/file.txt
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
[PACKAGED] 6 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
"#]])
.run();