Add support for ignoring dev dependencies (#557)

Resolves: #322 Resolves: #329 Resolves: #413 Resolves: #497
EmbarkStudios · Sep 4, 2023 · e815a51 · e815a51
1 parent 30fb0d0
commit e815a51
Show file tree

Hide file tree

Showing 27 changed files with 382 additions and 81 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Added
+- [PR#545](https://github.com/EmbarkStudios/cargo-deny/pull/545) added the ability to specify additional license exceptions via [additional configuration files](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#additional-exceptions-configuration-file).
+- [PR#549](https://github.com/EmbarkStudios/cargo-deny/pull/549) added the [`bans.build`](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-build-field-optional) configuration option, opting in to checking for [file extensions](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-script-extensions-field-optional), [native executables](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-executables-field-optional), and [interpreted scripts](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-interpreted-field-optional). This resolved [#43](https://github.com/EmbarkStudios/cargo-deny/issues/43).
+
+### Changed
+- [PR#557](https://github.com/EmbarkStudios/cargo-deny/pull/557) introduced changes to how [`dev-dependencies`](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#development-dependencies) are handled. By default, crates that are only used as dev-dependencies (ie, there are no normal nor build dependency edges linking them to other crates) will no longer be considered when checking for [`multiple-versions`](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-multiple-versions-field-optional) violations. This can be re-enabled via the [`bans.multiple-versions-include-dev`](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-multiple-versions-include-dev-field-optional) config field. Additionally, licenses are no longer checked for `dev-dependencies`, but can be re-enabled via [`licenses.include-dev`](https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html#the-include-dev-field-optional) the config field. `dev-dependencies` can also be completely disabled altogether, but this applies to all checks, including `advisories` and `sources`, so is not enabled by default. This behavior can be enabled by using the [`exclude-dev`](https://embarkstudios.github.io/cargo-deny/checks/cfg.html#the-exclude-dev-field-optional) field, or the `--exclude-dev` command line flag. This change resolved [#322](https://github.com/EmbarkStudios/cargo-deny/issues/322), [#329](https://github.com/EmbarkStudios/cargo-deny/issues/329), [#413](https://github.com/EmbarkStudios/cargo-deny/issues/413) and [#497](https://github.com/EmbarkStudios/cargo-deny/issues/497).
+
+### Fixed
+- [PR#549](https://github.com/EmbarkStudios/cargo-deny/pull/549) fixed [#548](https://github.com/EmbarkStudios/cargo-deny/issues/548) by correctly locating cargo registry indices from an git ssh url.
+- [PR#549](https://github.com/EmbarkStudios/cargo-deny/pull/549) fixed [#552](https://github.com/EmbarkStudios/cargo-deny/issues/552) by correctly handling signal interrupts and removing the advisory-dbs lock file.
+- [PR#549](https://github.com/EmbarkStudios/cargo-deny/pull/549) fixed [#553](https://github.com/EmbarkStudios/cargo-deny/issues/553) by adding the `native-certs` feature flag that can enable the OS native certificate store.
+
+### Deprecated
+- [PR#549](https://github.com/EmbarkStudios/cargo-deny/pull/549) moved `bans.allow-build-scripts` to [`bans.build.allow-build-scripts`](https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html#the-allow-build-scripts-field-optional). `bans.allow-build-scripts` is still supported, but emits a warning.
+
 ## [0.14.1] - 2023-08-02
 ### Fixed
 - [PR#544](https://github.com/EmbarkStudios/cargo-deny/pull/544) updated dependencies, notably `tame-index 0.2.5` which fixed [this issue](https://github.com/EmbarkStudios/tame-index/issues/8)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/docs/src/checks/bans/cfg.md b/docs/src/checks/bans/cfg.md
@@ -16,6 +16,10 @@ Determines what happens when multiple versions of the same crate are encountered
 * `warn` (default) - Prints a warning for each crate with duplicates, but does not fail the check.
 * `allow` - Ignores duplicate versions of the same crate.
 
+### The `multiple-versions-include-dev` field (optional)
+
+If `true`, `dev-dependencies` are included when checking for multiple versions of crates. By default this is false, and any crates that are only reached via dev dependency edges are ignored when checking for multiple versions. Note that this also means that `skip` and `skip` tree are not used, which may lead to warnings about unused configuration.
+
 ### The `wildcards` field (optional)
 
 Determines what happens when a dependency is specified with the `*` (wildcard) version.

diff --git a/docs/src/checks/cfg.md b/docs/src/checks/cfg.md
@@ -28,15 +28,15 @@ The `targets` field allows you to specify one or more targets which you **actual
 
 The [target triple](https://forge.rust-lang.org/release/platform-support.html) for the target you wish to filter target specific dependencies with. If the target triple specified is **not** one of the targets builtin to `rustc`, the configuration check for that target will be limited to only the raw `[target.<target-triple>.dependencies]` style of target configuration, as `cfg()` expressions require us to know the details about the target.
 
-#### The `exclude` field (optional)
+#### The `targets.features` field (optional)
 
-Just as with the [`--exclude`](../cli/common.md#--exclude) command line option, this field allows you to specify one or more [Package ID specifications](https://doc.rust-lang.org/cargo/commands/cargo-pkgid.html) that will cause the crate(s) in question to be excluded from the crate graph that is used for the operation you are performing.
+Rust `cfg()` expressions support the [`target_feature = "feature-name"`](https://doc.rust-lang.org/reference/attributes/codegen.html#the-target_feature-attribute) predicate, but at the moment, the only way to actually pass them when compiling is to use the `RUSTFLAGS` environment variable. The `features` field allows you to specify 1 or more `target_feature`s you plan to build with, for a particular target triple. At the time of this writing, cargo-deny does not attempt to validate that the features you specify are actually valid for the target triple, but this is [planned](https://github.com/EmbarkStudios/cfg-expr/issues/1).
 
-Note that excluding a crate is recursive, if any of its transitive dependencies are only referenced via the excluded crate, they will also be excluded from the crate graph.
+### The `exclude` field (optional)
 
-#### The `features` field (optional)
+Just as with the [`--exclude`](../cli/common.md#--exclude) command line option, this field allows you to specify one or more [Package ID specifications](https://doc.rust-lang.org/cargo/commands/cargo-pkgid.html) that will cause the crate(s) in question to be excluded from the crate graph that is used for the operation you are performing.
 
-Rust `cfg()` expressions support the [`target_feature = "feature-name"`](https://doc.rust-lang.org/reference/attributes/codegen.html#the-target_feature-attribute) predicate, but at the moment, the only way to actually pass them when compiling is to use the `RUSTFLAGS` environment variable. The `features` field allows you to specify 1 or more `target_feature`s you plan to build with, for a particular target triple. At the time of this writing, cargo-deny does not attempt to validate that the features you specify are actually valid for the target triple, but this is [planned](https://github.com/EmbarkStudios/cfg-expr/issues/1).
+Note that excluding a crate is recursive, if any of its transitive dependencies are only referenced via the excluded crate, they will also be excluded from the crate graph.
 
 ### The `all-features` field (optional)
 
@@ -54,6 +54,10 @@ If set, and `--features` is not specified on the cmd line, these features will b
 
 The maximum depth that features will be displayed when inclusion graphs are included in diagnostics, unless specified via `--feature-depth` on the command line. Only applies to diagnostics that actually print features. If not specified defaults to `1`.
 
+### The `exclude-dev` field (optional)
+
+If set to `true`, all `dev-dependencies`, even one for workspace crates, are not included in the crate graph used for any of the checks. This option can also be enabled on cmd line with `--exclude-dev` either [before](../cli/common.md#--exclude-dev) or [after](../cli/check.md#--exclude-dev) the `check` subcommand.
+
 ### The `[licenses]` section
 
 See the [licenses config](licenses/cfg.html) for more info.

diff --git a/docs/src/checks/licenses/cfg.md b/docs/src/checks/licenses/cfg.md
@@ -41,6 +41,10 @@ allow = [
 ]
 ```
 
+### The `include-dev` field (optional)
+
+If `true`, licenses are checked even for `dev-dependencies`. By default this is false as `dev-dependencies` are not used by downstream crates, nor part of binary artifacts.
+
 ### The `unlicensed` field (optional)
 
 Determines what happens when a crate has not explicitly specified its license terms, and no license information could be confidently detected via `LICENSE*` files in the crate's source.

diff --git a/docs/src/cli/check.md b/docs/src/cli/check.md
@@ -36,6 +36,10 @@ When running the `advisories` check, the configured advisory database will be fe
 
 This option is also set if the `--offline` flag is used in the global options.
 
+### `--exclude-dev`
+
+If set to `true`, all `dev-dependencies`, even one for workspace crates, are not included in the crate graph used for any of the checks.
+
 ### `-D, --deny <DENY>`
 
 Set lint denied

diff --git a/docs/src/cli/common.md b/docs/src/cli/common.md
@@ -24,6 +24,10 @@ Space-separated list of features to enable when determining which crates to cons
 
 Forces all workspace crates to be used as roots in the crate graph that we operate on, unless they are excluded by other means. By default, if you specify a [virtual manifest](https://doc.rust-lang.org/cargo/reference/manifest.html#virtual-manifest), all crates in the workspace will be used as roots. However, if you specify a normal package manifest somewhere inside a workspace, only that crate will be used as a graph root, and only other workspaces crates it depends on will be included in the graph. If you want to specify a sub-crate in a workspace, but still include all other crates in the workspace, you can use this flag.
 
+### `--exclude-dev`
+
+If set to `true`, all `dev-dependencies`, even one for workspace crates, are not included in the crate graph used for any of the checks.
+
 ### `--exclude`
 
 Exclude the specified package(s) from the crate graph. Unlike other cargo subcommands, it doesn't have to be used in conjunction with the `--workspace` flag. This flag may be specified multiple times.

diff --git a/src/bans.rs b/src/bans.rs
@@ -200,6 +200,7 @@ pub fn check(
         external_default_features,
         skipped,
         multiple_versions,
+        multiple_versions_include_dev,
         highlight,
         tree_skipped,
         wildcards,
@@ -236,6 +237,24 @@ pub fn check(
         dupes: smallvec::SmallVec::new(),
     };
 
+    let filtered_krates = if !multiple_versions_include_dev {
+        ctx.krates.krates_filtered(krates::DepKind::Dev)
+    } else {
+        Vec::new()
+    };
+
+    // If we're not counting dev dependencies as duplicates, create a separate
+    // set of krates with dev dependencies filtered out
+    let should_add_dupe = move |kid| {
+        if multiple_versions_include_dev {
+            true
+        } else {
+            filtered_krates
+                .binary_search_by(|krate| krate.id.cmp(kid))
+                .is_ok()
+        }
+    };
+
     let report_duplicates = |multi_detector: &MultiDetector<'_>, sink: &mut diag::ErrorSink| {
         if multi_detector.dupes.len() <= 1 {
             return;
@@ -669,66 +688,68 @@ pub fn check(
                     }
                 }
 
-                if let Some(matches) = matches(&skipped, krate) {
-                    for rm in matches {
-                        pack.push(diags::Skipped {
-                            krate,
-                            skip_cfg: CfgCoord {
-                                file: file_id,
-                                span: rm.id.span.clone(),
-                            },
-                        });
+                if should_add_dupe(&krate.id) {
+                    if let Some(matches) = matches(&skipped, krate) {
+                        for rm in matches {
+                            pack.push(diags::Skipped {
+                                krate,
+                                skip_cfg: CfgCoord {
+                                    file: file_id,
+                                    span: rm.id.span.clone(),
+                                },
+                            });
 
-                        // Mark each skip filter that is hit so that we can report unused
-                        // filters to the user so that they can cleanup their configs as
-                        // their dependency graph changes over time
-                        skip_hit.as_mut_bitslice().set(rm.index, true);
-                    }
-                } else if !tree_skipper.matches(krate, &mut pack) {
-                    if multi_detector.name != krate.name {
-                        report_duplicates(&multi_detector, &mut sink);
+                            // Mark each skip filter that is hit so that we can report unused
+                            // filters to the user so that they can cleanup their configs as
+                            // their dependency graph changes over time
+                            skip_hit.as_mut_bitslice().set(rm.index, true);
+                        }
+                    } else if !tree_skipper.matches(krate, &mut pack) {
+                        if multi_detector.name != krate.name {
+                            report_duplicates(&multi_detector, &mut sink);
 
-                        multi_detector.name = &krate.name;
-                        multi_detector.dupes.clear();
-                    }
+                            multi_detector.name = &krate.name;
+                            multi_detector.dupes.clear();
+                        }
 
-                    multi_detector.dupes.push(i);
+                        multi_detector.dupes.push(i);
 
-                    if wildcards != LintLevel::Allow && !krate.is_git_source() {
-                        let severity = match wildcards {
-                            LintLevel::Warn => Severity::Warning,
-                            LintLevel::Deny => Severity::Error,
-                            LintLevel::Allow => unreachable!(),
-                        };
+                        if wildcards != LintLevel::Allow && !krate.is_git_source() {
+                            let severity = match wildcards {
+                                LintLevel::Warn => Severity::Warning,
+                                LintLevel::Deny => Severity::Error,
+                                LintLevel::Allow => unreachable!(),
+                            };
 
-                        let mut wildcards: Vec<_> = krate
-                            .deps
-                            .iter()
-                            .filter(|dep| dep.req == VersionReq::STAR)
-                            .collect();
+                            let mut wildcards: Vec<_> = krate
+                                .deps
+                                .iter()
+                                .filter(|dep| dep.req == VersionReq::STAR)
+                                .collect();
 
-                        if allow_wildcard_paths {
-                            let is_private = krate.is_private(&[]);
+                            if allow_wildcard_paths {
+                                let is_private = krate.is_private(&[]);
 
-                            wildcards.retain(|dep| {
-                                if is_private {
-                                    dep.path.is_none()
-                                } else {
-                                    let is_path_dev_dependency = dep.path.is_some()
-                                        && dep.kind != DependencyKind::Development;
-                                    is_path_dev_dependency || dep.path.is_none()
-                                }
-                            });
-                        }
+                                wildcards.retain(|dep| {
+                                    if is_private {
+                                        dep.path.is_none()
+                                    } else {
+                                        let is_path_dev_dependency = dep.path.is_some()
+                                            && dep.kind != DependencyKind::Development;
+                                        is_path_dev_dependency || dep.path.is_none()
+                                    }
+                                });
+                            }
 
-                        if !wildcards.is_empty() {
-                            sink.push(diags::Wildcards {
-                                krate,
-                                severity,
-                                wildcards,
-                                allow_wildcard_paths,
-                                cargo_spans: &cargo_spans,
-                            });
+                            if !wildcards.is_empty() {
+                                sink.push(diags::Wildcards {
+                                    krate,
+                                    severity,
+                                    wildcards,
+                                    allow_wildcard_paths,
+                                    cargo_spans: &cargo_spans,
+                                });
+                            }
                         }
                     }
                 }

diff --git a/src/bans/cfg.rs b/src/bans/cfg.rs
@@ -239,6 +239,8 @@ pub struct Config {
     /// How to handle multiple versions of the same crate
     #[serde(default = "crate::lint_warn")]
     pub multiple_versions: LintLevel,
+    #[serde(default)]
+    pub multiple_versions_include_dev: bool,
     /// How the duplicate graphs are highlighted
     #[serde(default = "highlight")]
     pub highlight: GraphHighlight,
@@ -288,6 +290,7 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             multiple_versions: LintLevel::Warn,
+            multiple_versions_include_dev: false,
             highlight: GraphHighlight::All,
             deny: Vec::new(),
             allow: Vec::new(),
@@ -618,6 +621,7 @@ impl crate::cfg::UnvalidatedConfig for Config {
         ValidConfig {
             file_id: cfg_file,
             multiple_versions: self.multiple_versions,
+            multiple_versions_include_dev: self.multiple_versions_include_dev,
             highlight: self.highlight,
             denied,
             denied_multiple_versions,
@@ -767,6 +771,7 @@ pub struct ValidBuildConfig {
 pub struct ValidConfig {
     pub file_id: FileId,
     pub multiple_versions: LintLevel,
+    pub multiple_versions_include_dev: bool,
     pub highlight: GraphHighlight,
     pub(crate) denied: Vec<KrateBan>,
     pub(crate) denied_multiple_versions: Vec<Skrate>,