ci: Watch cargo deps with dependabot - #355
Conversation
Sorry, something went wrong.
Dependabot watches `pip` and `github-actions` but not `cargo`, so a new `openjd-expr` / `openjd-model` / `openjd-sessions` release is only noticed when somebody goes looking. OpenJobDescription#335 is the worked example: the crates published, and picking them up was a manual chase for pins, `Cargo.lock`, and `THIRD-PARTY-LICENSES.txt`. Two groups, ordered: cargo-patch all patch bumps cargo-minor all minor bumps except openjd-* An `openjd-*` minor therefore matches no group and gets its own PR, which is the point: those crates are 0.x, where cargo treats a minor as breaking, and they carry the API surface these bindings wrap. That is not theoretical — openjd-expr 0.4.0 carried a breaking coercion change, and openjd-model 0.5.2 changed the job-side `StepScript` wire format. Everything else is grouped. `tokio`, `uuid` and `serde_json` are 1.x, where a minor is additive by cargo's own rules, so isolating those buys nothing; the same goes for the 0.x crates this package does not wrap (`pyo3`, `log`, `windows`) and for transitive `Cargo.lock` bumps, which `dependency-type: indirect` support means cargo dependabot will raise. Ordering matters and is load-bearing: dependabot places a dependency in the first group it matches, so an `openjd-*` patch still rides in `cargo-patch` and only minors reach the exclusion. Loosening the version requirements would not have helped. They are already permissive — `openjd-model = "0.6.0"` is `^0.6.0`, so `>=0.6.0, <0.7.0`, and a 0.6.1 satisfies it without an edit. `Cargo.lock` is what actually pins the build, and it is committed and is what CI resolves from. So a patch pickup needs a `cargo update` and a commit no matter how loose the requirement string is, and a `*` requirement would give up the reproducibility the lock exists for. The one manual step this leaves: `scripts/check_third_party_licenses.sh` renders crate versions out of `Cargo.lock` and hard-fails on any diff, so the `third_party_licenses` check is red on every cargo PR until someone pushes `scripts/check_third_party_licenses.sh --update` onto the branch. Dependabot cannot do that itself. The note is in `dependabot.yml` rather than only here so it is findable after this commit scrolls away. Testing: config parses as YAML and declares all three ecosystems. Routing was checked by simulating dependabot's documented rules (first matching group wins; unmatched dependencies get their own PR) over 19 cases covering openjd-* minors and patches, the 1.x and 0.x direct deps, two transitive crates, and majors — all 19 route as intended. Against the previous single-group config the same check fails 10 cases, including `tokio`/`uuid`/`serde_json` minors, which confirms the second group is load-bearing. Dependabot itself cannot be run locally, so the schedule and the real grouping behaviour are unverified until it runs on a Monday. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
| patterns: | ||
| - "*" | ||
| exclude-patterns: | ||
| - "openjd-*" |
There was a problem hiding this comment.
The "0.x minor = breaking" rationale used to exclude openjd-* applies to most of the other direct dependencies too, but they are not excluded.
From rust-bindings/Cargo.toml, the direct deps are:
- 1.x (minor is additive): tokio, uuid, serde_json, log
- 0.x (cargo treats minor as breaking): pyo3 = "0.29", pyo3-log = "0.13", pyo3-stub-gen = "0.23", windows = "0.62"
The comment above justifies grouping with "tokio, uuid and serde_json are 1.x, where a minor is additive", which is true for those three but silently mis-describes the four 0.x crates. As written, cargo-minor will bundle a pyo3 0.29 → 0.30 bump — which is a breaking API change and also moves the abi3 / extension-module surface and the pyo3-stub-gen compatibility pairing — into one PR alongside unrelated bumps like windows 0.62 → 0.63. That is the exact situation the openjd-* exclusion exists to avoid, and it makes the PR harder to review and to revert in isolation.
Consider either widening the exclusion, e.g.
exclude-patterns:
- "openjd-*"
- "pyo3*"
- "windows"(pyo3 and pyo3-stub-gen are version-coupled, so if they should move together they are better in their own small group than in the catch-all), or updating the comment to state explicitly that 0.x breaking bumps other than openjd-* are intentionally grouped.
Sorry, something went wrong.
| # | ||
| # Every cargo PR needs `scripts/check_third_party_licenses.sh --update` | ||
| # committed onto its branch: that check renders crate versions from | ||
| # Cargo.lock and dependabot cannot regenerate it. |
There was a problem hiding this comment.
This comment documents a step Dependabot cannot perform, which means every cargo PM this config opens will land with a failing required check and, per .github/workflows/auto_approve.yml, an automatic approval on top of it.
The chain:
- A cargo bump (including a transitive-only Cargo.lock bump) changes Cargo.lock.
- third_party_licenses in .github/workflows/rust_quality.yml runs scripts/check_third_party_licenses.sh in verify mode, which renders crate versions from Cargo.lock and diffs against the committed THIRD-PARTY-LICENSES.txt — exit 1 on any drift.
- Dependabot never runs that script, so the job fails on essentially every cargo PR.
- auto_approve.yml triggers on any pull_request where github.actor == dependabot[bot] with no gate on check status or update type, so the PR gets approved regardless.
An approved PR sitting on a red check is a bad steady state for a repo where THIRD-PARTY-LICENSES.txt ships with every release: the failure becomes routine noise, and routine noise is what gets clicked past. Since the Python pip ecosystem block above also feeds the same script (via pyproject.toml [project.dependencies]), this is pre-existing for pip bumps, but adding cargo makes it the common case rather than the rare one.
Worth resolving before enabling this, rather than relying on the comment being read. Options:
- A workflow on pull_request restricted to github.actor == dependabot[bot] that runs scripts/check_third_party_licenses.sh --update and pushes the result onto the Dependabot branch, so the check goes green on its own.
- Or, if manual regeneration is genuinely the intended workflow, note that expectation in CONTRIBUTING.md/DEVELOPMENT.md where a maintainer will actually see it, and consider gating auto_approve.yml so it does not approve PRs whose checks have not passed.
Sorry, something went wrong.
Supersedes #336, which is rebased onto a two-week-old mainline and carries a factually wrong
justification. Close that one in favour of this.
Dependabot watches pip and github-actions but not cargo, so a new openjd-expr /
openjd-model / openjd-sessions release is only noticed when somebody goes looking.
#335 is the worked
example: the crates published, and picking them up was a manual chase for pins, Cargo.lock, and
THIRD-PARTY-LICENSES.txt. This makes that arrive as a PR on its own.
What lands where
Two groups, ordered. Dependabot places a dependency in the first group it matches, and anything
matching no group gets its own PR.
The isolation is for the openjd-* crates specifically. They are 0.x, where cargo treats a minor
as breaking, and they carry the API surface these bindings wrap: openjd-expr 0.4.0 carried a
breaking coercion change, and openjd-model 0.5.2 changed the job-side StepScript wire format.
Those deserve their own CI run and their own review rather than riding along with a patch.
Everything else groups. tokio, uuid and serde_json are 1.x, where a minor is additive by
cargo's own rules, so isolating them buys nothing; the same goes for the 0.x crates this package
does not wrap (pyo3, log, windows) and for transitive Cargo.lock bumps, which cargo's
dependency-type: indirect support means dependabot will raise.
Loosening the version requirements would not have helped
Worth stating because it is the obvious alternative. The requirements are already permissive
enough — openjd-model = "0.6.0" is ^0.6.0, so >=0.6.0, <0.7.0, and a 0.6.1 satisfies it
without an edit. Cargo.lock is what actually pins the build, and it is committed and is what CI
resolves from. So a patch pickup needs a cargo update and a commit no matter how loose the
requirement string is, and a * requirement would buy nothing while giving up the reproducibility
the lock exists for.
The one manual step this leaves
scripts/check_third_party_licenses.sh renders crate versions out of Cargo.lock and hard-fails
on any diff, so the third_party_licenses check will be red on every cargo PR until the file is
regenerated [measured — a lock-only diff of the three openjd versions failed the check]. The
follow-up is one command pushed to the dependabot branch:
That note now lives in dependabot.yml rather than only in this description, so it is findable
after the PR is merged.
This check is already non-hermetic, independently of cargo. The Python section is regenerated
by resolving pyproject.toml into a fresh venv, and pyproject.toml declares
pydantic >= 2.10, < 3 — unpinned. The committed file records pydantic; version 2.13.5, which is
what PyPI serves today, so the check is green by coincidence of timing. The next pydantic patch
release turns third_party_licenses red on every PR from every author. Fixing that properly (a
regenerate-and-commit job that works for any author, or making the check advisory) is worth its own
PR and would subsume the cargo case; gating the check on dependabot[bot] would not, because it
leaves the Python side untouched.
Note the zero-touch option is not as cheap as it looks: a dependabot pull_request gets a
read-only token, so committing back needs pull_request_target or a PAT, and the script builds a
wheel from PR head — that is a write-capable token running PR-authored build code.
Review findings from #336
On the grouping fix specifically: the review suggested a cargo-stable-minor group listing
["tokio", "uuid", "serde_json"]. That does not reach the transitive 1.x crates in Cargo.lock
(memchr 2.x and friends), which is where most of the churn it objects to comes from. Inverting it
— group all minors, exclude openjd-* — covers those and states the intent directly.
Testing
Config parses as YAML and declares all three ecosystems; the cargo entry's directory: "/" is
where Cargo.toml and Cargo.lock actually live, with rust-bindings picked up as a workspace
member.
Routing was checked by simulating dependabot's documented rules (first matching group wins;
unmatched dependencies get their own PR) over 19 cases: openjd-* minors and patches, every 1.x
and 0.x direct dep, two transitive crates, and majors. All 19 route as intended. Run against the
single-group config from #336, the same check fails 10 cases — including tokio, uuid and
serde_json minors — which confirms the second group is load-bearing rather than decorative.
Dependabot itself cannot be run locally, so the schedule and the real grouping behaviour are
unverified until it runs on a Monday.