Skip to content

Commit

Permalink
fix(package): deduplicate dirty symlink detection
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
weihanglo committed Dec 31, 2024
1 parent de39f59 commit 24dd205
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 38 deletions.
55 changes: 19 additions & 36 deletions src/cargo/ops/cargo_package/vcs.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Helpers to gather the VCS information for `cargo package`.
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;

use anyhow::Context as _;
Expand Down Expand Up @@ -187,8 +186,7 @@ fn git(
.iter()
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
.map(|p| p.as_ref())
.chain(dirty_metadata_paths(pkg, repo)?.iter())
.chain(dirty_symlinks(pkg, repo, src_files)?.iter())
.chain(dirty_files_outside_pkg_root(pkg, repo, src_files)?.iter())
.map(|path| {
pathdiff::diff_paths(path, cwd)
.as_ref()
Expand Down Expand Up @@ -221,54 +219,39 @@ fn git(
}
}

/// Checks whether files at paths specified in `package.readme` and
/// `package.license-file` have been modified.
/// Checks whether "included" source files outside package root have been modified.
///
/// This is required because those paths may link to a file outside the
/// current package root, but still under the git workdir, affecting the
/// final packaged `.crate` file.
fn dirty_metadata_paths(pkg: &Package, repo: &git2::Repository) -> CargoResult<Vec<PathBuf>> {
let mut dirty_files = Vec::new();
let workdir = repo.workdir().unwrap();
let root = pkg.root();
let meta = pkg.manifest().metadata();
for path in [&meta.license_file, &meta.readme] {
let Some(path) = path.as_deref().map(Path::new) else {
continue;
};
let abs_path = paths::normalize_path(&root.join(path));
if paths::strip_prefix_canonical(&abs_path, root).is_ok() {
// Inside package root. Don't bother checking git status.
continue;
}
if let Ok(rel_path) = paths::strip_prefix_canonical(&abs_path, workdir) {
// Outside package root but under git workdir,
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
dirty_files.push(workdir.join(rel_path))
}
}
}
Ok(dirty_files)
}

/// Checks whether source files are symlinks and have been modified.
/// This currently looks at
///
/// * `package.readme` and `package.license-file` pointing to paths outside package root
/// * symlinks targets reside outside package root
///
/// This is required because those paths may link to a file outside the
/// current package root, but still under the git workdir, affecting the
/// final packaged `.crate` file.
fn dirty_symlinks(
fn dirty_files_outside_pkg_root(
pkg: &Package,
repo: &git2::Repository,
src_files: &[PathEntry],
) -> CargoResult<HashSet<PathBuf>> {
let pkg_root = pkg.root();
let workdir = repo.workdir().unwrap();

let meta = pkg.manifest().metadata();
let metadata_paths: Vec<_> = [&meta.license_file, &meta.readme]
.into_iter()
.filter_map(|p| p.as_deref())
.map(|path| paths::normalize_path(&pkg_root.join(path)))
.collect();

let mut dirty_symlinks = HashSet::new();
for rel_path in src_files
.iter()
.filter(|p| p.is_symlink_or_under_symlink())
.map(|p| p.as_ref().as_path())
.map(|p| p.as_ref())
.chain(metadata_paths.iter())
// If inside package root. Don't bother checking git status.
.filter(|p| paths::strip_prefix_canonical(p, pkg.root()).is_err())
.filter(|p| paths::strip_prefix_canonical(p, pkg_root).is_err())
// Handle files outside package root but under git workdir,
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
{
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1378,11 +1378,10 @@ fn dirty_file_outside_pkg_root_considered_dirty() {
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] 5 files in the working directory contain changes that were not yet committed into git:
[ERROR] 4 files in the working directory contain changes that were not yet committed into git:
LICENSE
README.md
README.md
lib.rs
original-dir/file
Expand Down

0 comments on commit 24dd205

Please sign in to comment.