Repository navigation
ci: download Docker release binaries with the workflow token - #7962
PastaPastaPasta merged 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Final review complete — no blockers (commit cf1f162) · triage: low |
|
No actionable comments were generated in the recent review.
|
…ial #59 0b33fee Merge bitcoin-core/bitcoin-maintainer-tools#92: github-merge: Always include repo_from in the merge commit message (pasta) fe057ee partial bitcoin-core/bitcoin-maintainer-tools#59: github-merge: Format pull_reference in one place (pasta) Pull request description: ## Issue being fixed or feature implemented `contrib/devtools/github-merge.py` writes merge subjects as `Merge #N: ...` no matter which repo the PR came from. The v23.1.9 merge-back (#7961) brings in merges made in dashpay/dash-internal, such as `Merge #2: fix(consensus): reject negative transaction nVersion after DIP3`. Next to public merges like `Merge #7486: ...` in the same history, there is no way to tell that `#2` is a dash-internal PR rather than #2. Bitcoin Core fixed this after `github-merge.py` moved to bitcoin-core/bitcoin-maintainer-tools (bitcoin#16625, which Dash never took). That is why their history reads `Merge bitcoin/bitcoin#N` / `Merge bitcoin-core/gui#N`. Dash's copy of the script never picked up the change. ## What was done? Backports two bitcoin-core/bitcoin-maintainer-tools commits to Dash's `github-merge.py`: - partial bitcoin-core/bitcoin-maintainer-tools#59 (commit 6e05dc8 only): format the PR reference in one place (`pull_reference`) and use it in the merge subject, the merge-details header and the error messages. The rest of #59 (`push_mirrors`) is unrelated and is omitted. Dash's `print_merge_details()` has no `message` argument, so only its first parameter changes. - bitcoin-core/bitcoin-maintainer-tools#92 (commit 3811ce1): always include the repo in the reference. Upstream uses `repo_from`, which comes from bitcoin-core/bitcoin-maintainer-tools#57 (`--repo-from` for monotree fetches). Dash doesn't have #57, so this uses `repo` (`githubmerge.repository`), which is what `repo_from` defaults to upstream. No Dash-specific adaptation is needed for dash-internal. When run in a checkout with `githubmerge.repository = dashpay/dash-internal`, merges come out as `Merge dashpay/dash-internal#N`. Before / after: ``` Merge #2: fix(consensus): reject negative transaction nVersion after DIP3 Merge dashpay/dash-internal#2: fix(consensus): reject negative transaction nVersion after DIP3 ``` ## How Has This Been Tested? Ran the patched script in a throwaway clone against open PRs and answered `x` at the sign prompt, so nothing was committed or pushed. The `git commit` call was wrapped to print the generated subject: - `githubmerge.repository = dashpay/dash`, PR 7962 → `Merge #7962: ci: download Docker release binaries with the workflow token` - `githubmerge.repository = dashpay/dash-internal`, PR 13 onto `v23.1.x` → `Merge dashpay/dash-internal#13: ci: let Docker release builds download binaries from a private repo` The merge-details header printed `#7962 ... into develop` and `dashpay/dash-internal#13 ... into v23.1.x`. ## Breaking Changes None. Only the merge commit subject format changes, to `Merge <owner>/<repo>#N`, which matches the `merge-script` merges already on develop. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: d014624d1b0d47dcf0040d2e76b4bfaadb3f878840acb11ec1af0c476cc97e97463be016c0937aa2229a7b45e4cc35b718681115c643c55a198afa3cbaf09cb0
The release image fetched dashcore-<tag>-<arch>.tar.gz with an anonymous wget inside the image build. Download both Linux archives in the workflow with `gh release download` and the workflow token instead, then bind-mount them into the image build so they add no layer and the image no longer needs wget or ca-certificates installed and purged. Select the runner from a dedicated RUNNER_DOCKER_RELEASE repository variable, falling back to ubuntu-22.04 when it is unset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2dff471 to
cf1f162
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The workflow and Dockerfile changes correctly move release archive downloads into the authenticated workflow, make both architecture archives available in the BuildKit context, and select the matching archive using TARGETARCH. The unsupported-architecture path fails explicitly, the runner fallback preserves existing behavior, and no in-scope correctness or security issues were found. Lint and related completed checks passed; source-build checks were still queued, and the release workflow has no release-event result yet.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — This is a small, self-contained CI and Docker build change with straightforward behavior and no critical code-surface impact. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 89% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
Issue being fixed or feature implemented
The Docker Hub release image downloads
dashcore-<tag>-<arch>.tar.gzwith an anonymouswgetfrom inside the image build. To do that, the image installswgetandca-certificatesand then purges them, and the download only works if the release assets can be fetched without credentials. The download step also had small bugs: it echoed a URL built from an unset$arch, and itscasedidn't fail on an unsupported architecture.The job also always runs on
ubuntu-22.04, with no way to choose its runner the way other workflows here can, through a repository variable.What was done?
gh release downloadand the workflow token (contents: read). The Dockerfile bind-mounts them and picks one byTARGETARCH, so the archives add no image layer and thewget/ca-certificatesinstall and purge steps are gone. An unsupported architecture now fails the build.RELEASE_TAGenv var, used by the raw-tag, download and suffix steps.runs-onis now${{ vars.RUNNER_DOCKER_RELEASE || 'ubuntu-22.04' }}. It gets a variable of its own, rather than sharingRUNNER_AMD64with the CI build jobs, so the release runner can be changed without moving them. When the variable is unset, the job runs onubuntu-22.04as before.The Docker tags and build platforms are unchanged.
How Has This Been Tested?
Dockerfile.GitHubActions.Releaselocally with buildx forlinux/amd64, using thev23.1.9archives fetched withgh release download.dashd --versionprintsDash Core version v23.1.9, and/usr/local/binholds the same binaries as before (dash-cli,dash-tx,dash-util,dash-wallet,dashd,test_dash). The same Dockerfile was built earlier for bothlinux/amd64andlinux/arm64.actionlintis clean on the new workflow (rerun after the runner-variable change); ondevelopit reports one SC2086 warning, which this change fixes.Breaking Changes
None. Repository setup: to run the release job on Blacksmith, set the repository variable
RUNNER_DOCKER_RELEASEtoblacksmith-4vcpu-ubuntu-2204(Settings → Secrets and variables → Actions → Variables). Leave it unset to keepubuntu-22.04.Checklist:
🤖 Generated with Claude Code