Repository navigation
fix(ci): the license inventory guard reads two of the three workspace roots - #1114
Merged
nihalpasham merged 1 commit intoAug 21, 2026
Merged
Conversation
… roots
`check-dependency-licenses.sh` builds its required set from two manifests:
required="$(
{
declared_crates Cargo.toml
declared_crates crates/rustc-codegen-cuda/Cargo.toml
} | LC_ALL=C sort -u
)"
and the comment above it says "Both first-party workspaces, not just the root
one". There are three. `cargo metadata` stops at a `[workspace]` boundary, so
one pass per root is the only way to reach them all, and
`crates/cuda-macros/tests/device-only` declares its own `[workspace]` on
purpose: the fixture exists to prove a graph that does *not* contain
`cuda-host` (NVIDIA#701/NVIDIA#702), so it must resolve independently rather than share the
root lock. That boundary puts it outside both passes.
The gap is live, not theoretical. Its member `device-only-kernels` has no row
in `dependency-licenses.csv`, and adding the third pass says so immediately:
error: dependency-licenses.csv is missing a row for:
device-only-kernels
`publish = false` is not what excluded it -- `cuda-intrinsics-gen` and
`reserved-oxide-symbols` are both unpublished root members and both have rows.
The only reason it was absent is that no pass ever looked at that workspace.
This is NVIDIA#1043's gap in the sibling guard. That fix taught `cargo deny check`
the third root, which governs the license *policy* over the resolved graph.
This guard covers the other half, stated in its own header: "that the
human-readable inventory does not silently fall behind what the workspace
declares." Both halves need all three roots, and until now only one had them.
What it protects going forward: the fixture currently declares nothing but two
first-party path dependencies, so the one missing row is the whole of today's
drift. But a third-party dependency added there would have been recorded
nowhere -- cargo-deny would judge its policy and the inventory would stay
silent, which is exactly the asymmetry this pair of guards exists to prevent.
Changes:
* the third `declared_crates` pass, plus a rewritten comment that enumerates
all three roots and says why each is separate, so the next reader does not
have to rediscover that `cargo metadata` stops at the boundary;
* the `device-only-kernels` row, matching the column layout of the other
first-party rows and inserted in sort position.
Verified, both directions:
* drop the CSV row, keep the third pass -> fails naming `device-only-kernels`;
* keep the row, drop the third pass -> reports "OK: ... all 58 declared
crates" while blind to that workspace, which is the state being fixed;
* restored -> "OK: dependency-licenses.csv records all 59 declared crates" and
the example half unchanged.
`scripts/check-example-license-policy.sh`, the other consumer of the CSV, still
reports "OK: deny.toml holds over every example workspace". `bash -n` clean.
Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Collaborator
|
nihalpasham
approved these changes
Aug 21, 2026
nihalpasham
added a commit
that referenced
this pull request
Aug 23, 2026
…checks (#1132) * ci(licenses): the guard's own workspace-root list is a claim nothing checks `check-dependency-licenses.sh` reaches each first-party workspace with one `declared_crates` pass per root, because `cargo metadata` stops at a `[workspace]` boundary. Which roots exist was hard-coded, and nothing compared that list against the tree -- which is exactly how the guard has failed twice: * #662 found `crates/rustc-codegen-cuda` unread. The backend, the largest first-party crate in the tree, had no CSV row while every other member did. * #1114 found `crates/cuda-macros/tests/device-only` unread, and its member `device-only-kernels` likewise unrecorded. Both times the guard printed OK. That is the failure mode this repo's guards are built to refuse, and it is the one this guard had: a short list is indistinguishable from a complete one, a count cannot tell them apart, and the comment saying "Both first-party workspaces" was itself the wrong number for months. #1114's review asked for the self-check; this is it. The list becomes two named arrays and is compared against `git ls-files -- '*Cargo.toml'` filtered to manifests declaring `[workspace]`, both directions: * a manifest on disk that no pass reads fails, naming it, and says to add it as first-party or as vendored; * a name in the arrays that is no longer such a manifest fails as stale, since a renamed root left behind is a permanent hole. `crates/fuzzer/rustlantis` is the one vendored root, listed separately with the reason: third-party code attributed in THIRD_PARTY_NOTICES whose dependencies are not ours to record, excluded by `check-spdx-headers.sh` on the same grounds. Roots under `crates/rustc-codegen-cuda/examples` stay out of scope here because the second half of the script already resolves those, including the two nested sub-workspaces, from their committed lock files. Verified, four ways: | mutation | result | |---|---| | drop the device-only root from the list | fails, naming that manifest | | add a name that is not a manifest | fails as stale, naming it | | a new tracked `[workspace]` manifest appears | fails, naming it | | the `[workspace]` scan pattern rots | refuses: "the scan broke, so a clean result means nothing" | Unmutated the guard reports "OK: dependency-licenses.csv records all 59 declared crates" and the example half is unchanged. `bash -n` clean. `git` is added to the `require_tool` list it already uses for `cargo` and `python3`. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> * ci(licenses): derive workspace roots with Cargo Resolve every tracked manifest through Cargo, compare named non-example roots in both directions, and require each example workspace to have an adjacent tracked lock. Signed-off-by: nihalp <nihalp@nvidia.com> --------- Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Signed-off-by: nihalp <nihalp@nvidia.com> Co-authored-by: nihalp <nihalp@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is wrong
check-dependency-licenses.shbuilds its required set from two manifests:and the comment above it says "Both first-party workspaces, not just the root one." There are three.
cargo metadatastops at a[workspace]boundary, so one pass per root is the only way to reach them all, andcrates/cuda-macros/tests/device-onlydeclares its own[workspace]on purpose — the fixture exists to prove a graph that does not containcuda-host(#701/#702), so it must resolve independently rather than share the root lock. That boundary puts it outside both passes.The gap is live
Its member
device-only-kernelshas no row independency-licenses.csv, and adding the third pass says so immediately:publish = falseis not what excluded it —cuda-intrinsics-genandreserved-oxide-symbolsare both unpublished root members and both have rows. The only reason it was absent is that no pass ever looked at that workspace.Why this guard specifically
This is #1043's gap in the sibling guard. That fix taught
cargo deny checkthe third root, which governs the license policy over the resolved graph. This guard covers the other half, stated in its own header: "that the human-readable inventory does not silently fall behind what the workspace declares." Both halves need all three roots, and until now only one had them.The fixture is treated as a peer root elsewhere in the tree too — #1050's
FIXED_FMT_SCOPESlists exactly these three directories forcargo oxide fmt. This guard was the outlier.What it protects going forward: the fixture currently declares nothing but two first-party path dependencies, so the one missing row is the whole of today's drift. But a third-party dependency added there would have been recorded nowhere — cargo-deny would judge its policy and the inventory would stay silent, which is exactly the asymmetry this pair of guards exists to prevent.
Changes
declared_cratespass, plus a rewritten comment enumerating all three roots and why each is separate — so the next reader does not have to rediscover thatcargo metadatastops at the boundary.device-only-kernelsrow, matching the column layout of the other first-party rows and inserted in sort position.Verification
All local, no GPU needed. Both directions:
device-only-kernelsOK: ... all 58 declared crates— blind to that workspaceOK: dependency-licenses.csv records all 59 declared cratesscripts/check-example-license-policy.sh, the other consumer of the CSV, still reportsOK: deny.toml holds over every example workspace.bash -nclean.Two files. Checked against every open PR's file list via
gh api --paginate .../pulls/N/files— no open PR touches either, including #1102, which edits many manifests but not the CSV.