Skip to content

Commit 24dd205

Browse files
committed
fix(package): deduplicate dirty symlink detection
metdata path fields may point to a dirty symlilnk and cause duplicate report. This commit combines `dirty_metadata_paths` and `dirty_symlinks` into one so is able to de-duplicate them.
1 parent de39f59 commit 24dd205

File tree

2 files changed

+20
-38
lines changed

2 files changed

+20
-38
lines changed

src/cargo/ops/cargo_package/vcs.rs

+19-36
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Helpers to gather the VCS information for `cargo package`.
22
33
use std::collections::HashSet;
4-
use std::path::Path;
54
use std::path::PathBuf;
65

76
use anyhow::Context as _;
@@ -187,8 +186,7 @@ fn git(
187186
.iter()
188187
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
189188
.map(|p| p.as_ref())
190-
.chain(dirty_metadata_paths(pkg, repo)?.iter())
191-
.chain(dirty_symlinks(pkg, repo, src_files)?.iter())
189+
.chain(dirty_files_outside_pkg_root(pkg, repo, src_files)?.iter())
192190
.map(|path| {
193191
pathdiff::diff_paths(path, cwd)
194192
.as_ref()
@@ -221,54 +219,39 @@ fn git(
221219
}
222220
}
223221

224-
/// Checks whether files at paths specified in `package.readme` and
225-
/// `package.license-file` have been modified.
222+
/// Checks whether "included" source files outside package root have been modified.
226223
///
227-
/// This is required because those paths may link to a file outside the
228-
/// current package root, but still under the git workdir, affecting the
229-
/// final packaged `.crate` file.
230-
fn dirty_metadata_paths(pkg: &Package, repo: &git2::Repository) -> CargoResult<Vec<PathBuf>> {
231-
let mut dirty_files = Vec::new();
232-
let workdir = repo.workdir().unwrap();
233-
let root = pkg.root();
234-
let meta = pkg.manifest().metadata();
235-
for path in [&meta.license_file, &meta.readme] {
236-
let Some(path) = path.as_deref().map(Path::new) else {
237-
continue;
238-
};
239-
let abs_path = paths::normalize_path(&root.join(path));
240-
if paths::strip_prefix_canonical(&abs_path, root).is_ok() {
241-
// Inside package root. Don't bother checking git status.
242-
continue;
243-
}
244-
if let Ok(rel_path) = paths::strip_prefix_canonical(&abs_path, workdir) {
245-
// Outside package root but under git workdir,
246-
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
247-
dirty_files.push(workdir.join(rel_path))
248-
}
249-
}
250-
}
251-
Ok(dirty_files)
252-
}
253-
254-
/// Checks whether source files are symlinks and have been modified.
224+
/// This currently looks at
225+
///
226+
/// * `package.readme` and `package.license-file` pointing to paths outside package root
227+
/// * symlinks targets reside outside package root
255228
///
256229
/// This is required because those paths may link to a file outside the
257230
/// current package root, but still under the git workdir, affecting the
258231
/// final packaged `.crate` file.
259-
fn dirty_symlinks(
232+
fn dirty_files_outside_pkg_root(
260233
pkg: &Package,
261234
repo: &git2::Repository,
262235
src_files: &[PathEntry],
263236
) -> CargoResult<HashSet<PathBuf>> {
237+
let pkg_root = pkg.root();
264238
let workdir = repo.workdir().unwrap();
239+
240+
let meta = pkg.manifest().metadata();
241+
let metadata_paths: Vec<_> = [&meta.license_file, &meta.readme]
242+
.into_iter()
243+
.filter_map(|p| p.as_deref())
244+
.map(|path| paths::normalize_path(&pkg_root.join(path)))
245+
.collect();
246+
265247
let mut dirty_symlinks = HashSet::new();
266248
for rel_path in src_files
267249
.iter()
268250
.filter(|p| p.is_symlink_or_under_symlink())
269-
.map(|p| p.as_ref().as_path())
251+
.map(|p| p.as_ref())
252+
.chain(metadata_paths.iter())
270253
// If inside package root. Don't bother checking git status.
271-
.filter(|p| paths::strip_prefix_canonical(p, pkg.root()).is_err())
254+
.filter(|p| paths::strip_prefix_canonical(p, pkg_root).is_err())
272255
// Handle files outside package root but under git workdir,
273256
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
274257
{

tests/testsuite/package.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1378,11 +1378,10 @@ fn dirty_file_outside_pkg_root_considered_dirty() {
13781378
p.cargo("package --workspace --no-verify")
13791379
.with_status(101)
13801380
.with_stderr_data(str![[r#"
1381-
[ERROR] 5 files in the working directory contain changes that were not yet committed into git:
1381+
[ERROR] 4 files in the working directory contain changes that were not yet committed into git:
13821382
13831383
LICENSE
13841384
README.md
1385-
README.md
13861385
lib.rs
13871386
original-dir/file
13881387

0 commit comments

Comments
 (0)