Skip to content

Commit

Permalink
Treat git dependencies the same as path dependencies for `allow-w…
Browse files Browse the repository at this point in the history
…ildcard-paths`. (#599)

Fixes #488, making it possible to ban wildcards without also banning
git-only dependencies.

This may not be a perfect fit for some use cases — arguably `git`
dependencies are less implicitly-versioned than `path` dependencies
since `path` dependencies are typically always the same revision of the
same repo, but `git` dependencies might be `cargo update`d to totally
different code. But I can't think of an alternative that's
equal-or-better in correctness short of introducing even more
configuration.

(I suspect that the whole idea of counting path-only or git-only deps as
wildcard versions *ever* is wrong, because [the Cargo documentation
says](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies)
that “…the version key always implies that the package is available in a
registry. version, git, and path keys are considered [separate
locations](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations)
for resolving the dependency” — which implies that a dep *without*
`version` is different from a dep with a wildcard version. However,
figuring out Cargo's behavior there and how cargo-deny should treat it
feels like a rabbit hole I don't want to go down just to fix #488. I
left a TODO comment suggesting further consideration.)
  • Loading branch information
kpreid authored Feb 5, 2024
1 parent 3afa5e5 commit c529e66
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 7 deletions.
8 changes: 4 additions & 4 deletions docs/src/checks/bans/cfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ Determines what happens when a dependency is specified with the `*` (wildcard) v

If specified, alters how the `wildcard` field behaves:

* [path](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies) `dependencies` in **private** crates will no longer emit a warning or error.
* path `dev-dependencies` in both public and private crates will no longer emit a warning or error.
* path `dependencies` and `build-dependencies` in **public** crates will continue to produce warnings and errors.
* [path](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies) or [git](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories) `dependencies` in **private** crates will no longer emit a warning or error.
* path or git `dev-dependencies` in both public and private crates will no longer emit a warning or error.
* path or git `dependencies` and `build-dependencies` in **public** crates will continue to produce warnings and errors.

Being limited to private crates is due to crates.io not allowing packages to be published with `path` dependencies except for `dev-dependencies`.
Being limited to private crates is due to crates.io not allowing packages to be published with `path` or `git` dependencies except for `dev-dependencies`.

### The `highlight` field (optional)

Expand Down
19 changes: 16 additions & 3 deletions src/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,12 +786,13 @@ pub fn check(
let is_private = krate.is_private(&[]);

wildcards.retain(|dep| {
let is_path_or_git = is_path_or_git_dependency(dep);
if is_private {
dep.path.is_none()
!is_path_or_git
} else {
let is_path_dev_dependency = dep.path.is_some()
let is_path_non_dev_dependency = is_path_or_git
&& dep.kind != DependencyKind::Development;
is_path_dev_dependency || dep.path.is_none()
is_path_non_dev_dependency || !is_path_or_git
}
});
}
Expand Down Expand Up @@ -1415,3 +1416,15 @@ fn validate_file_checksum(path: &crate::Path, expected: &cfg::Checksum) -> anyho
validate_checksum(std::io::BufReader::new(file), expected)?;
Ok(())
}

/// Returns true if the dependency has a `path` or `git` source.
///
/// TODO: Possibly what we actually care about, where this is used in the wildcard check, is
/// “is not using any registry source”.
fn is_path_or_git_dependency(dep: &krates::cm::Dependency) -> bool {
dep.path.is_some()
|| dep
.source
.as_ref()
.is_some_and(|url| url.starts_with("git+"))
}
16 changes: 16 additions & 0 deletions tests/bans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,22 @@ allow-wildcard-paths = true
insta::assert_json_snapshot!(diags);
}

/// Ensures that dependencies with wildcard and git are allowed for private packages
#[test]
fn allow_git_wildcards_private_package() {
let diags = gather_bans(
func_name!(),
KrateGather::new("wildcards/allow-git"),
r#"
multiple-versions = 'allow'
wildcards = 'deny'
allow-wildcard-paths = true
"#,
);

insta::assert_json_snapshot!(diags);
}

/// Ensures that multiple versions are always deterministically sorted by
/// version number
/// See <https://github.com/EmbarkStudios/cargo-deny/issues/384>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: tests/bans.rs
expression: diags
---
[]
213 changes: 213 additions & 0 deletions tests/test_data/wildcards/allow-git/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions tests/test_data/wildcards/allow-git/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "wildcards-test-allow-git"
version = "0.1.0"
authors = []
edition = "2018"
license = "MIT"

publish = false

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# An arbitrary choice of actually existant Git repository
wildcards-test-allow-git = { package = "krates", git = "https://github.com/EmbarkStudios/krates", rev = "b03ecd6f3204a1b1ec04fbaead2d0d122a3a4494" }
1 change: 1 addition & 0 deletions tests/test_data/wildcards/allow-git/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}

0 comments on commit c529e66

Please sign in to comment.