Skip to content

Commit 95bb4ed

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 4ee7193 commit 95bb4ed

File tree

2 files changed

+20
-38
lines changed

2 files changed

+20
-38
lines changed

src/cargo/ops/cargo_package/vcs.rs

Lines changed: 19 additions & 36 deletions
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 _;
@@ -139,8 +138,7 @@ fn git(
139138
.iter()
140139
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
141140
.map(|p| p.as_ref())
142-
.chain(dirty_metadata_paths(pkg, repo)?.iter())
143-
.chain(dirty_symlinks(pkg, repo, src_files)?.iter())
141+
.chain(dirty_files_outside_pkg_root(pkg, repo, src_files)?.iter())
144142
.map(|path| {
145143
pathdiff::diff_paths(path, cwd)
146144
.as_ref()
@@ -173,54 +171,39 @@ fn git(
173171
}
174172
}
175173

176-
/// Checks whether files at paths specified in `package.readme` and
177-
/// `package.license-file` have been modified.
174+
/// Checks whether "included" source files outside package root have been modified.
178175
///
179-
/// This is required because those paths may link to a file outside the
180-
/// current package root, but still under the git workdir, affecting the
181-
/// final packaged `.crate` file.
182-
fn dirty_metadata_paths(pkg: &Package, repo: &git2::Repository) -> CargoResult<Vec<PathBuf>> {
183-
let mut dirty_files = Vec::new();
184-
let workdir = repo.workdir().unwrap();
185-
let root = pkg.root();
186-
let meta = pkg.manifest().metadata();
187-
for path in [&meta.license_file, &meta.readme] {
188-
let Some(path) = path.as_deref().map(Path::new) else {
189-
continue;
190-
};
191-
let abs_path = paths::normalize_path(&root.join(path));
192-
if paths::strip_prefix_canonical(&abs_path, root).is_ok() {
193-
// Inside package root. Don't bother checking git status.
194-
continue;
195-
}
196-
if let Ok(rel_path) = paths::strip_prefix_canonical(&abs_path, workdir) {
197-
// Outside package root but under git workdir,
198-
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
199-
dirty_files.push(workdir.join(rel_path))
200-
}
201-
}
202-
}
203-
Ok(dirty_files)
204-
}
205-
206-
/// Checks whether source files are symlinks and have been modified.
176+
/// This currently looks at
177+
///
178+
/// * `package.readme` and `package.license-file` pointing to paths outside package root
179+
/// * symlinks targets reside outside package root
207180
///
208181
/// This is required because those paths may link to a file outside the
209182
/// current package root, but still under the git workdir, affecting the
210183
/// final packaged `.crate` file.
211-
fn dirty_symlinks(
184+
fn dirty_files_outside_pkg_root(
212185
pkg: &Package,
213186
repo: &git2::Repository,
214187
src_files: &[PathEntry],
215188
) -> CargoResult<HashSet<PathBuf>> {
189+
let pkg_root = pkg.root();
216190
let workdir = repo.workdir().unwrap();
191+
192+
let meta = pkg.manifest().metadata();
193+
let metadata_paths: Vec<_> = [&meta.license_file, &meta.readme]
194+
.into_iter()
195+
.filter_map(|p| p.as_deref())
196+
.map(|path| paths::normalize_path(&pkg_root.join(path)))
197+
.collect();
198+
217199
let mut dirty_symlinks = HashSet::new();
218200
for rel_path in src_files
219201
.iter()
220202
.filter(|p| p.is_symlink_or_under_symlink())
221-
.map(|p| p.as_ref().as_path())
203+
.map(|p| p.as_ref())
204+
.chain(metadata_paths.iter())
222205
// If inside package root. Don't bother checking git status.
223-
.filter(|p| paths::strip_prefix_canonical(p, pkg.root()).is_err())
206+
.filter(|p| paths::strip_prefix_canonical(p, pkg_root).is_err())
224207
// Handle files outside package root but under git workdir,
225208
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
226209
{

tests/testsuite/package.rs

Lines changed: 1 addition & 2 deletions
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)