Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ updates:
update-types:
- "minor"
- "patch"
- package-ecosystem: "cargo"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Enabling the cargo ecosystem will make every Dependabot Rust PR fail CI, because Dependabot updates Cargo.lock but cannot regenerate THIRD-PARTY-LICENSES.txt.

The third_party_licenses job in .github/workflows/rust_quality.yml runs scripts/check_third_party_licenses.sh, which renders the Rust section from Cargo.lock via cargo about generate and then hard-fails on any diff:

if ! diff -u "$OUTPUT_FILE" "$generated"; then
  ...
  exit 1
fi

The committed file embeds exact crate versions (** tokio; version 1.53.1, ** memchr; version 2.8.3, ...), so a bump of any crate — including a transitive patch bump inside the cargo-patch group — changes the rendered output and trips the check. There is no path for a bot-authored PR to fix this on its own, so cargo Dependabot PRs will land permanently red and need a manual scripts/check_third_party_licenses.sh --update commit pushed onto each one.

Worth deciding up front which way to go, e.g.:

  • add a job (or a Dependabot-triggered workflow with contents: write) that runs --update and commits back to the Dependabot branch, or
  • make the license check advisory / non-blocking for dependabot[bot]-authored PRs, or
  • accept the manual step and document it.

Note this is specific to cargo: the existing pip group only bumps requirements-*.txt (dev/test), which the script does not include since it resolves Python deps from the installed wheel's runtime closure.

directory: "/" # Workspace root; rust-bindings is discovered as a member
schedule:
interval: "weekly"
day: "monday"
commit-message:
prefix: "chore(deps):"
# Patch bumps are combined into a single PR. Minor bumps do not match this
# group and so each get their own individual PR, because every crate this
# package depends on is pre-1.0: for a 0.x crate cargo treats a minor bump

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"every crate this package depends on is pre-1.0" is not accurate, and it is the premise the grouping choice rests on. Per rust-bindings/Cargo.toml:

crate req pre-1.0?
openjd-expr 0.3.0 yes
openjd-model 0.5.0 yes
openjd-sessions 0.5.0 yes
pyo3 0.29 yes
pyo3-log 0.13 yes
pyo3-stub-gen 0.22 yes
log 0.4 yes
windows 0.62 yes
tokio 1 no
uuid 1 no
serde_json 1 no

For the three 1.x crates a minor bump is explicitly non-breaking under cargo's semver rules, so the "0.x minor == breaking, so isolate it" rationale does not apply to them — yet they are excluded from cargo-patch and will each open a standalone PR (tokio 1.53 -> 1.54, etc.) for a compatible update. Combined with transitive deps in Cargo.lock, that is a fair amount of avoidable PR churn.

If the intent is "isolate only the genuinely-breaking 0.x minors", consider adding a second group that sweeps up the 1.x minors, e.g.:

groups:
  cargo-patch:
    patterns: ["*"]
    update-types: ["patch"]
  cargo-stable-minor:
    patterns: ["tokio", "uuid", "serde_json"]
    update-types: ["minor"]

Otherwise the comment should just be corrected to say most deps are pre-1.0 and the per-PR split is being accepted for all minors for simplicity.

# as breaking, so openjd-expr 0.3 -> 0.4 deserves its own review rather
# than riding along with a patch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The stated goal — "openjd-expr 0.3 -> 0.4 deserves its own review rather than riding along with a patch" — is defeated by .github/workflows/auto_approve.yml, which approves any PR authored by dependabot[bot] with no filtering on update type:

if: ${{ github.actor == 'dependabot[bot]' }}
steps:
  - uses: dependabot/fetch-metadata@v3
    id: metadata
  - run: gh pr review --approve "$PR_URL"

So the isolated-PR-per-minor-bump split buys separation but not review: a 0.3 -> 0.4 bump of openjd-expr/openjd-model/openjd-sessions — the crates that actually carry the API surface this package binds to — arrives pre-approved. That is the highest-risk update class this config can produce, and it is exactly the one that gets waved through.

fetch-metadata is already wired up (id: metadata) but its outputs are unused, so gating is cheap, e.g. restrict auto-approve to patch bumps:

if: steps.metadata.outputs.update-type == 'version-update:semver-patch'

(That is a change to auto_approve.yml, not this file, but it is this PR that makes it load-bearing for Rust deps.)

groups:
cargo-patch:
patterns:
- "*"
update-types:
- "patch"

- package-ecosystem: "github-actions"
directory: "/" # Location of package manifests
schedule:
Expand Down
Loading