Skip to content

Commit 767408a

Browse files
committed
fix(package): check dirtiness of symlinks source files
This adds a special case for checking source files are symlinks and have been modified when under a VCS control 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. This may have potential performance issue. If a package contains thousands of symlinks, Cargo will fire `git status` for each of them.
1 parent 0c93708 commit 767408a

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

src/cargo/ops/cargo_package.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::collections::HashSet;
12
use std::collections::{BTreeSet, HashMap};
23
use std::fs::{self, File};
34
use std::io::prelude::*;
@@ -827,11 +828,14 @@ fn check_repo_state(
827828
// be packaged. This is a lazy n^2 check, but seems fine with
828829
// thousands of files.
829830
let cwd = gctx.cwd();
831+
832+
let mut dirty_files_outside_pkg_root = dirty_symlinks(pkg, repo, src_files)?;
833+
dirty_files_outside_pkg_root.extend(dirty_metadata_paths(pkg, repo)?);
830834
let mut dirty_src_files: Vec<_> = src_files
831835
.iter()
832836
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
833837
.map(|p| p.as_path_buf())
834-
.chain(dirty_metadata_paths(pkg, repo)?.iter())
838+
.chain(dirty_files_outside_pkg_root.iter())
835839
.map(|path| {
836840
pathdiff::diff_paths(path, cwd)
837841
.as_ref()
@@ -899,6 +903,36 @@ fn check_repo_state(
899903
Ok(dirty_files)
900904
}
901905

906+
/// Checks whether source files are symlinks and have been modified.
907+
///
908+
/// This is required because those paths may link to a file outside the
909+
/// current package root, but still under the git workdir, affecting the
910+
/// final packaged `.crate` file.
911+
fn dirty_symlinks(
912+
pkg: &Package,
913+
repo: &git2::Repository,
914+
src_files: &[PathEntry],
915+
) -> CargoResult<HashSet<PathBuf>> {
916+
let workdir = repo.workdir().unwrap();
917+
let mut dirty_symlinks = HashSet::new();
918+
for rel_path in src_files
919+
.iter()
920+
.filter(|p| p.is_symlink())
921+
.map(|p| p.as_path())
922+
// If inside package root. Don't bother checking git status.
923+
.filter(|p| paths::strip_prefix_canonical(*p, pkg.root()).is_err())
924+
// Handle files outside package root but under git workdir,
925+
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
926+
{
927+
// TODO: Should we warn users if there are like thousands of symlinks,
928+
// which may hurt the performance?
929+
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
930+
dirty_symlinks.insert(workdir.join(rel_path));
931+
}
932+
}
933+
Ok(dirty_symlinks)
934+
}
935+
902936
// Helper to collect dirty statuses for a single repo.
903937
fn collect_statuses(
904938
repo: &git2::Repository,

tests/testsuite/package.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1375,10 +1375,11 @@ fn dirty_file_outside_pkg_root_considered_dirty() {
13751375
p.cargo("package --workspace --no-verify")
13761376
.with_status(101)
13771377
.with_stderr_data(str![[r#"
1378-
[ERROR] 2 files in the working directory contain changes that were not yet committed into git:
1378+
[ERROR] 3 files in the working directory contain changes that were not yet committed into git:
13791379
13801380
LICENSE
13811381
README.md
1382+
lib.rs
13821383
13831384
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
13841385

0 commit comments

Comments
 (0)