Skip to content

Commit 4ee7193

Browse files
committed
fix(package): check dirtiness of symlink 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 567c7d7 commit 4ee7193

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

src/cargo/ops/cargo_package/vcs.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Helpers to gather the VCS information for `cargo package`.
22
3+
use std::collections::HashSet;
34
use std::path::Path;
45
use std::path::PathBuf;
56

@@ -139,6 +140,7 @@ fn git(
139140
.filter(|src_file| dirty_files.iter().any(|path| src_file.starts_with(path)))
140141
.map(|p| p.as_ref())
141142
.chain(dirty_metadata_paths(pkg, repo)?.iter())
143+
.chain(dirty_symlinks(pkg, repo, src_files)?.iter())
142144
.map(|path| {
143145
pathdiff::diff_paths(path, cwd)
144146
.as_ref()
@@ -201,6 +203,34 @@ fn dirty_metadata_paths(pkg: &Package, repo: &git2::Repository) -> CargoResult<V
201203
Ok(dirty_files)
202204
}
203205

206+
/// Checks whether source files are symlinks and have been modified.
207+
///
208+
/// This is required because those paths may link to a file outside the
209+
/// current package root, but still under the git workdir, affecting the
210+
/// final packaged `.crate` file.
211+
fn dirty_symlinks(
212+
pkg: &Package,
213+
repo: &git2::Repository,
214+
src_files: &[PathEntry],
215+
) -> CargoResult<HashSet<PathBuf>> {
216+
let workdir = repo.workdir().unwrap();
217+
let mut dirty_symlinks = HashSet::new();
218+
for rel_path in src_files
219+
.iter()
220+
.filter(|p| p.is_symlink_or_under_symlink())
221+
.map(|p| p.as_ref().as_path())
222+
// If inside package root. Don't bother checking git status.
223+
.filter(|p| paths::strip_prefix_canonical(p, pkg.root()).is_err())
224+
// Handle files outside package root but under git workdir,
225+
.filter_map(|p| paths::strip_prefix_canonical(p, workdir).ok())
226+
{
227+
if repo.status_file(&rel_path)? != git2::Status::CURRENT {
228+
dirty_symlinks.insert(workdir.join(rel_path));
229+
}
230+
}
231+
Ok(dirty_symlinks)
232+
}
233+
204234
/// Helper to collect dirty statuses for a single repo.
205235
fn collect_statuses(repo: &git2::Repository, dirty_files: &mut Vec<PathBuf>) -> CargoResult<()> {
206236
let mut status_opts = git2::StatusOptions::new();

tests/testsuite/package.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1378,10 +1378,13 @@ 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] 2 files in the working directory contain changes that were not yet committed into git:
1381+
[ERROR] 5 files in the working directory contain changes that were not yet committed into git:
13821382
13831383
LICENSE
13841384
README.md
1385+
README.md
1386+
lib.rs
1387+
original-dir/file
13851388
13861389
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
13871390

0 commit comments

Comments
 (0)