chore: Add breaking-changes label automation to verify dependent crates - #2232
chore: Add breaking-changes label automation to verify dependent crates#2232scouten-adobe wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2232 +/- ##
==========================================
- Coverage 78.56% 78.56% -0.01%
==========================================
Files 176 176
Lines 46407 46407
==========================================
- Hits 36460 36459 -1
- Misses 9947 9948 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ed receivers Reclassifies c2pa-js (builds wasm from a Cargo c2pa dep -> Variant A) and c2pa-cpp (has a build-from-source path) as source-buildable; only c2pa-ios and c2pa-android still need the artifact bridge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the verify_sha correlation input + run-name to library-release.yml, the orchestrator's one-build-per-labeled-PR trigger (gated on BINARY_DEPENDENTS), and the iOS/Android receivers that download and stage the branch artifacts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follows each repo's documented build-from-local flow: drops the PYTHON= override for c2pa-python; for c2pa-cpp checks out c2pa-rs as a sibling dir and sets LD_LIBRARY_PATH for make test-release (per README). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ok-nick
left a comment
There was a problem hiding this comment.
Excited to see this, much needed!
| When a c2pa-rs change breaks API/ABI compatibility, the downstream binding repos | ||
| may need code changes before the c2pa-rs PR can land. This automation surfaces | ||
| that *before* merge: apply the **`breaking-changes`** label to a c2pa-rs PR and | ||
| every dependent repo is built against the PR's branch, with the results gating | ||
| the PR as a required status check. |
There was a problem hiding this comment.
We should also consider running this suite on every release. That may be planned already, though it would be good to call out. Sometimes things don't break API compatibility but they do break test cases/functionality in downstream SDKs that we can't predict. Same idea when the build system changes.
It may also be worth considering requiring this as as status check before finally merging. If we know it doesn't affect anything (such as not a feat: , fix: , build: PR, or just in general) it can be explicitly overridden.
|
|
||
| The orchestrator runs on **`pull_request_target`** rather than `pull_request`. | ||
| That's required so the gate status can be posted on fork PRs too — under | ||
| `pull_request` the `GITHUB_TOKEN` is read-only for forks, which would leave the | ||
| required gate context unset and block every external-contributor PR. The | ||
| fan-out job is safe under `pull_request_target` because it never checks out or | ||
| executes PR code; it only reads trusted event metadata and posts statuses / | ||
| dispatches. **Do not add a checkout of the PR head to that workflow.** | ||
|
|
There was a problem hiding this comment.
I think we just need to be careful not to run CI on PRs that edit this workflow. I've seen recommendations to use workflow_run to ensure any functionality accessing secrets is gated on the main branch rather than the fork.
| repos consume c2pa-rs differently, the receivers differ — use the ready-made | ||
| ones where they exist: | ||
|
|
||
| | Dependent | Receiver to install | Notes | |
There was a problem hiding this comment.
I wonder if we can move the build logic to the other repo, maybe as a reusable workflow we call or a makefile that way we don't need custom logic here for each SDK.
| **The artifact bridge is in place.** That workflow already uploads per-target | ||
| workflow artifacts (`release-artifacts-<os>-<target>`, containing the same | ||
| `c2pa-v*-<target>.zip` files as the release assets) on every run, and its | ||
| `release` job is gated to `c2pa-v*` tag pushes — so dispatching it against a | ||
| branch produces downloadable artifacts without cutting a release: |
There was a problem hiding this comment.
This sounds cool, so we will have all the artifacts available directly in c2pa-rs?
|
Withdrawing for now pending some conversations about our release process. |
What
Adds a
breaking-changeslabel that, when applied to a c2pa-rs PR, fans out to the org's downstream binding repos and verifies each still builds against the PR's branch — gating the PR on the results as a required status check.How it works
.github/workflows/breaking-changes.yml(orchestrator + aggregator):breaking-changes-gate(success when the label is absent, so unlabeled PRs never hang).repository_dispatch(c2pa-rs-breaking-change) fans out to each dependent inDEPENDENTS. IfBINARY_DEPENDENTSis non-empty, it also triggers one branch build oflibrary-release.yml(viaCROSS_ORG_PR_TOKEN, sinceGITHUB_TOKENcan't start a run).status-triggered job aggregates the per-dependentbreaking-changes / <repo>results back into the gate.pull_request_target(never checks out PR code) so the gate can be posted on fork PRs too; fork PRs pass-through with a note.Receivers under
docs/breaking-change-automation/receivers/— one per dependent, tailored to how each consumes c2pa-rs:c2pa-node-v2,c2pa-js— rewrite the Cargoc2padep to a git+branch source, build, open a draft PR (Variant A).c2pa-python,c2pa-cpp— build from a local c2pa-rs checkout, status-only.c2pa-ios,c2pa-android— download the branch'slibrary-release.ymlartifacts, stage them, build, status-only. Unvalidated (need a real run + runners).library-release.yml:releasejob gated toc2pa-v*tags (so branch dispatch produces artifacts without a release), plus averify_shainput +run-nameso receivers can locate the branch build.Required follow-up before this is live
CROSS_ORG_PR_TOKENis live (Contents + Pull requests write on the dependents; Commit statuses + Actions write on c2pa-rs).breaking-changes-gateas a required status check onmain/v1_api. Do not require the per-dependent contexts.DEPENDENTS(andBINARY_DEPENDENTSfor ios/android). See the README's "To activate iOS/Android".Receiver PRs
🤖 Generated with Claude Code