Skip to content

More cargo-deny maintenance #1927

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

Merged
merged 4 commits into from
Apr 5, 2025
Merged

Conversation

EliahKagan
Copy link
Member

@EliahKagan EliahKagan commented Apr 5, 2025

Changes:

  • Remove the old ring license from the allowlist, since we no longer depend on any versions of ring that use it.

  • Convert the comment explanation for why we ignore RUSTSEC-2024-0436 into reified form as the value of a reason key, so tools that process the file don't lose it (even if only to make debugging slightly easier when using tomlq).

  • Split the cargo-deny-advisories scan into two scans (still in that one job) that cover what we covered before, except ignoring RUSTSEC-2025-0021 when the affected version is only reachable as a dependency of gix-testtools. This is as discussed in #1924 (comment), and it makes cargo-deny-advisories pass again.

See the commit messages for further details.


Regarding the license allowlist, disused licenses do seem to accumulate gradually; see also 9c708db (#1863). cargo deny supports configuring this as an error. But I have refrained from doing so, because:

  • We don't list that many unusual licenses anymore.
  • Which licenses produce the warning differs based on how cargo deny check licenses is run, showing another occurrence when --all-features is omitted, which might occasionally make sense to do when running it locally.

Before this change, we get a warning:

    $ cargo deny --workspace --all-features check licenses
    warning[license-not-encountered]: license was not encountered
       ┌─ /home/ek/source/repos/gitoxide/deny.toml:32:6
       │
    32 │     "LicenseRef-ring",
       │      ━━━━━━━━━━━━━━━ unmatched license allowance

    licenses ok

The same warning is shown in the `cargo-deny` job check on CI.

This happens because `ring` no longer uses a custom/nonstandard
license, instead using `Apache-2.0 AND ISC` since version 0.17.10.
(See briansmith/ring#2402 and
https://crates.io/crates/ring/versions for details.)

Nothing in this workspace depends directly or directly on old
versions of `ring` that use that license, so this removes it from
the list of licenses in `deny.toml`.
This turns the comment in `cargo.deny` explaining why we currently
ignore the informational advisory RUSTSEC-2024-0436 in `paste` into
data that could be parsed and displayed by tools, by using the form
with `id` and `reason` fields, where the old comment text is the
value of the `reason` field.

This is one of the forms documented in:
https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html#the-ignore-field-optional
This is only a minor improvement in clarity now, but the benefit
will be greater with the immediately forthcoming change, and doing
this separately makes it so the next commit can be reverted by
itself once it is no longer needed.
This splits the `EmbarkStudios/cargo-deny-action` step in
`cargo-deny-advisories` into two such steps:

- Scan the workspace except prune `gix-testtools` and everything
  reachable through it (following it neither as a root, nor when it
  is found as dev dependency of another crate). This doesn't get to
  its obsolete dependencies, while still ensuring that nothing in
  the workspace *except* what we reach through `gix-testtools` is
  affected by RUSTSEC-2025-0021.

- Scan the whole workspace, including `gix-testtools` and all its
  dependencies, including the obsolete version of `gix-features`
  that is affected by RUSTSEC-2025-0021. But ignore that advisory.

To support this, steps are added to install the `yq`-associated
`tomlq` command and use it to produce the modified configuration
file for the second scan in a way that shouldn't break under any
changes to comments, spacing, style, or ordering in `deny.toml`.
Copy link
Member Author

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since cargo-deny-advisories is modified here and that job does not block auto-merge even when it fails, I want to double-check that it still works the way I have described here. I plan to enable auto-merge after verifying that.

Edit: It is working (as it did when run in my fork).

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! I can't wait for not getting these failure emails anymore 😅.

@EliahKagan EliahKagan merged commit 24f40b9 into GitoxideLabs:main Apr 5, 2025
21 checks passed
@EliahKagan EliahKagan deleted the run-ci/deny branch April 5, 2025 06:50
EliahKagan added a commit to EliahKagan/gitoxide that referenced this pull request Apr 14, 2025
3cfd7fa (GitoxideLabs#1927) removed `LicenseRef-ring` from the `cargo deny`
license allowlist, because we no longer used any `ring` version old
enough to involve the old custom license. But the associated entry
in the `license.clarify` array that definded `LicenseRef-ring` was
not removed, even though it's not needed either given that the
license it clarifies is no longer referenced. This cleans that up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants