Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add justification/comment fields for [bans] to be shown on warning/error #578

Closed
CinchBlue opened this issue Dec 6, 2023 · 1 comment · Fixed by #605
Closed

Add justification/comment fields for [bans] to be shown on warning/error #578

CinchBlue opened this issue Dec 6, 2023 · 1 comment · Fixed by #605
Labels
enhancement New feature or request

Comments

@CinchBlue
Copy link

Is your feature request related to a problem? Please describe.
When denying usage of a crate, it is often useful to give a justification for the ban (and how to workaround it or redirect usage to more suitable crates).

Describe the solution you'd like
Add a comment or justification field to item entries under ban.deny. Print this as a part of a warning/error.

Describe alternatives you've considered
You could add it as a comment, but it's better if it's in the tool.

Additional context

  • Useful for denying access to anyhow or eyre in-general, and redirecting users to use a thiserror-based crate.
@CinchBlue CinchBlue added the enhancement New feature or request label Dec 6, 2023
@repi
Copy link
Contributor

repi commented Dec 6, 2023

agree this would be good to have, we've talked about it a long time ago about having a reason field on all bans. we really should implement it wouldn't be hard.

all of our bans do have a comment today so would be a way to encode that and as you say give a clear reason/justification/recommendation directly on failures.

    # denied crates
    { name = "openssl" }, # we use rustls instead
    { name = "openssl-sys" }, # we use rustls instead
    { name = "RustyXml" }, # we don't want to use any XML and some of these are 4 year old dependencies
    { name = "serde-xml-rs" }, # we don't want to use any XML and some of these are 4 year old dependencies
    { name = "color-backtrace" }, # color-backtrace is nice but brings in too many dependencies and that are often outdated, so not worth it for us.
    { name = "typetag" }, # disallow these crates that rely on static initialization order which we've had issues with
    { name = "inventory" }, # disallow these crates that rely on static initialization order which we've had issues with
    { name = "ctor" }, # disallow these crates that rely on static initialization order which we've had issues with
    { name = "bzip2" }, # disallow C dependency, we just Rust native versions instead
    { name = "smart-default", wrappers = [
        "minidump-common",
    ] }, # smart-default should not be used
    { name = "actix-web" }, # repeatedly unsound, too many dependencies, and not needed for our use cases
    { name = "bzip2-sys" }, # disallow C dependency, we just Rust native versions instead
    { name = "nfd" }, # unmaintined, we use `rfd` instead
    { name = "nfd2" }, # we use `rfd` instead
    { name = "msgbox" }, # we use `rfd` instead
    { name = "backtrace-sys" }, # disallow C dependency, use gimli Rust crate instead
    { name = "keyring" }, # too many and too old dependencies
    { name = "secret-service" }, # too many and too old dependencies
    { name = "wasmtime-cache" }, # we do our own manual caching

Jake-Shadle added a commit that referenced this issue Feb 20, 2024
This PR completely refactors the deny configuration, notably:

### `toml-span`

`toml-span` is now used for parsing toml files (currently only
deny.toml, but eventually cargo manifests as well), replacing `toml`.
This was done so that span information is _always_ available for keys
and values if we want to use it, as well as just reducing external
dependencies and build times, as serde is no longer used.

### `PackageSpec`

Specifying a package spec via the name + version combo occurs in many
locations in the config, but this has verbose in both the simple case,
where you just want to specify a crate name (eg. `[bans.deny]`), as well
as needing an entire extra key if you _do_ want to specify the version
requirement.

In addition, it was not possible to specify just a string previously due
to toml + serde making us decide between supporting plain strings for
package specs, and span information.

In all cases (and a few new ones) where the package name + version could
be used, now a simple string can be used instead, or, if you want/need
to supply additional values as a table, the `crate` key can be used
instead which follows a simple format as a single string, instead of the
separate name/version keys. `name` and `version` are still supported,
but are deprecated and will be removed in a future release.

#### Format

The string format of `PackageSpec` is quite simple:

1. No version - `<crate_name>` = `*`
1. Exact - `<crate_name>@<semver>` = `=<semver>`
2. Requirements - `<crate_name>:<semver_requirements>` =
`<semver_requirements>`

### Add `reason`, `use-instead`

Many items can now be tagged with a `reason = "<reason>"` and/or
`use-instead = "<crate_name>/<url>"` to add explanatory/helpful messages
that are surfaced with diagnostic messages. This was added to fix #578
instead of the typical fallback of relying on toml comments that might
not be part of the diagnostic span.

### `[advisories.ignore]`

Yanked crates can now be ignored by specifying a crate spec + optional
reason as a string or table, while still supporting advisory ids.

### Root table improved

The `targets`, `all-features`, `features`, `no-default-features`,
`exclude`, and `exclude-dev` keys have been moved to the new `graph`
table, as they all affect the dependency graph that cargo-deny performs
checks against. The `feature-depth` key has been moved to the `output`
table. The old locations are still respected, but are deprecated. Note
also that `targets` can now just take a string instead of `triple =
<target_triple>`.

Resolves: #264
Resolves: #539
Resolves: #578
Resolves: #579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants