Skip to content

Commit

Permalink
Auto merge of #9186 - weihanglo:issue-9054, r=alexcrichton
Browse files Browse the repository at this point in the history
Respect Cargo.toml `[package.exclude]` even not in a git repo.

May resolves #9054

This bug (or feature?) has been lingering for a while. #7680 fixed the `cargo package` part but `cargo vendor` is still affected by the heuristic rule of ignoring dotfiles. ~~I propose to drop the rule and include dotfiles by default even if the package is not under git-controlled~~. See below.

## Updated: Changes Summary

`cargo vendor` vendors dependencies without git-controlled but `cargo package` often runs under a VCS like git. [These lines](https://github.com/rust-lang/cargo/blob/1ca930b/src/cargo/sources/path.rs#L161-L168) are where they diverges: `fn list_files_walk_except_dot_files_and_dirs` builds [its own ignore instance], which cannot merge with other filter rules from `[package.exclude]`. This causes some patterns to not work as expected, such as re-including file after ignoring dotfiles `[.*, !negated_file]`.

To make re-include (negate) rule works, this patch adds the excluding dotfiles rule directly into the `package.exclude` ignore instance if **_no include option nor git repo exists_**. Other old behaviors should not change in this patch.

[its own ignore instance]: https://github.com/rust-lang/cargo/blob/1ca930b6/src/cargo/sources/path.rs#L364-L366
  • Loading branch information
bors committed May 1, 2021
2 parents 468314f + 153146e commit db741ac
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 34 deletions.
49 changes: 15 additions & 34 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,17 @@ impl<'cfg> PathSource<'cfg> {
fn _list_files(&self, pkg: &Package) -> CargoResult<Vec<PathBuf>> {
let root = pkg.root();
let no_include_option = pkg.manifest().include().is_empty();
let git_repo = if no_include_option {
self.discover_git_repo(root)?
} else {
None
};

let mut exclude_builder = GitignoreBuilder::new(root);
if no_include_option && git_repo.is_none() {
// no include option and not git repo discovered (see rust-lang/cargo#7183).
exclude_builder.add_line(None, ".*")?;
}
for rule in pkg.manifest().exclude() {
exclude_builder.add_line(None, rule)?;
}
Expand Down Expand Up @@ -161,23 +170,16 @@ impl<'cfg> PathSource<'cfg> {

// Attempt Git-prepopulate only if no `include` (see rust-lang/cargo#4135).
if no_include_option {
if let Some(result) = self.discover_git_and_list_files(pkg, root, &mut filter)? {
return Ok(result);
if let Some(repo) = git_repo {
return self.list_files_git(pkg, &repo, &mut filter);
}
// no include option and not git repo discovered (see rust-lang/cargo#7183).
return self.list_files_walk_except_dot_files_and_dirs(pkg, &mut filter);
}
self.list_files_walk(pkg, &mut filter)
}

// Returns `Some(_)` if found sibling `Cargo.toml` and `.git` directory;
// otherwise, caller should fall back on full file list.
fn discover_git_and_list_files(
&self,
pkg: &Package,
root: &Path,
filter: &mut dyn FnMut(&Path, bool) -> CargoResult<bool>,
) -> CargoResult<Option<Vec<PathBuf>>> {
/// Returns `Some(git2::Repository)` if found sibling `Cargo.toml` and `.git`
/// directory; otherwise, caller should fall back on full file list.
fn discover_git_repo(&self, root: &Path) -> CargoResult<Option<git2::Repository>> {
let repo = match git2::Repository::discover(root) {
Ok(repo) => repo,
Err(e) => {
Expand Down Expand Up @@ -212,7 +214,7 @@ impl<'cfg> PathSource<'cfg> {
};
let manifest_path = repo_relative_path.join("Cargo.toml");
if index.get_path(&manifest_path, 0).is_some() {
return Ok(Some(self.list_files_git(pkg, &repo, filter)?));
return Ok(Some(repo));
}
// Package Cargo.toml is not in git, don't use git to guide our selection.
Ok(None)
Expand Down Expand Up @@ -356,27 +358,6 @@ impl<'cfg> PathSource<'cfg> {
}
}

fn list_files_walk_except_dot_files_and_dirs(
&self,
pkg: &Package,
filter: &mut dyn FnMut(&Path, bool) -> CargoResult<bool>,
) -> CargoResult<Vec<PathBuf>> {
let root = pkg.root();
let mut exclude_dot_files_dir_builder = GitignoreBuilder::new(root);
exclude_dot_files_dir_builder.add_line(None, ".*")?;
let ignore_dot_files_and_dirs = exclude_dot_files_dir_builder.build()?;

let mut filter_ignore_dot_files_and_dirs =
|path: &Path, is_dir: bool| -> CargoResult<bool> {
let relative_path = path.strip_prefix(root)?;
match ignore_dot_files_and_dirs.matched_path_or_any_parents(relative_path, is_dir) {
Match::Ignore(_) => Ok(false),
_ => filter(path, is_dir),
}
};
self.list_files_walk(pkg, &mut filter_ignore_dot_files_and_dirs)
}

fn list_files_walk(
&self,
pkg: &Package,
Expand Down
44 changes: 44 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,50 @@ fn add_vendor_config(p: &Project) {
);
}

#[cargo_test]
fn package_exclude() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = "0.1.0"
"#,
)
.file("src/lib.rs", "")
.build();

Package::new("bar", "0.1.0")
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.1.0"
exclude = [".*", "!.include", "!.dotdir/include"]
"#,
)
.file("src/lib.rs", "")
.file(".exclude", "")
.file(".include", "")
.file(".dotdir/exclude", "")
.file(".dotdir/include", "")
.publish();

p.cargo("vendor --respect-source-config").run();
let csum = dbg!(p.read_file("vendor/bar/.cargo-checksum.json"));
assert!(csum.contains(".include"));
assert!(!csum.contains(".exclude"));
assert!(!csum.contains(".dotdir/exclude"));
// Gitignore doesn't re-include a file in an excluded parent directory,
// even if negating it explicitly.
assert!(!csum.contains(".dotdir/include"));
}

#[cargo_test]
fn two_versions() {
let p = project()
Expand Down

0 comments on commit db741ac

Please sign in to comment.