ci: run multiprocess / tan on arm runners #7047
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughSplits container build into architecture-specific jobs (build-amd64 and build-arm64) that push arch-suffixed images and emit per-arch digests, then runs a create-manifest job to publish a multi-arch manifest (hash-tag, tag, latest) combining both digests. Adds Sequence DiagramsequenceDiagram
participant GH as GitHub Event
participant BuildA as Build (linux/amd64)
participant BuildB as Build (linux/arm64)
participant Registry as Container Registry
participant Manifest as Create-Manifest Job
GH->>BuildA: trigger build-amd64 (ubuntu-24.04)
BuildA->>Registry: build & push image: repo:HASH_TAG-amd64
BuildA-->>BuildA: output digest (amd64)
GH->>BuildB: trigger build-arm64 (ubuntu-24.04-arm)
BuildB->>Registry: build & push image: repo:HASH_TAG-arm64
BuildB-->>BuildB: output digest (arm64)
BuildA-->>Manifest: provide amd64 digest
BuildB-->>Manifest: provide arm64 digest
Manifest->>Registry: create/push multi-arch manifest tags (HASH_TAG, TAG, latest)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1){guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-02-19T00:03:39.001ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
.github/workflows/build-container.yml (1)
24-68: Consider extracting common job steps to reduce duplication.The
build-amd64(lines 24–68) andbuild-arm64(lines 70–112) jobs are nearly identical, differing only in platform, runner label, and image tag suffixes. While GitHub Actions does not support direct YAML anchors in workflows, consider extracting the repeated steps into a reusable composite action if this pattern extends to other workflows. For now, the duplication is maintainable, but watch for future similar patterns.Also applies to: 70-112
test/sanitizer_suppressions/tsan (1)
22-26: BerkeleyDB deadlock suppressions look consistent; optional trace referenceThe new
deadlock:BerkeleyBatch,deadlock:DatabaseBatch, anddeadlock:__db_pthread_mutexentries fit the existing TSAN suppression style and the rationale in the preceding comment. If these are based on specific TSAN reports or upstream issues, consider adding a short link or reference next to the comment to help future debugging, but the changes themselves look fine.ci/test/00_setup_env_native_tsan.sh (1)
10-27: Arch‑aware HOST detection and timeout tweak look good; consider failing fast on unknown arch
- The
uname -m/dpkg --print-architecturemapping toaarch64-linux-gnuandx86_64-pc-linux-gnuis correct and should behave as expected on the GitHub ARM64 runners we use (aarch64/arm64) as well as amd64. Based on learnings, this aligns with our ARM64 runner environments.- The extra
--timeout-factor=4on TSAN runs is a reasonable way to compensate for sanitizer + ARM slowness.One optional hardening tweak: in the
*)branch, ifdpkgis missing or returns an unhandled arch,HOSTwill stay unset and we may fail later in a less obvious way. You could instead print an error andexit 1when we can’t map the architecture, so misconfigured runners are caught early.Also applies to: 31-31
contrib/containers/ci/ci-slim.Dockerfile (1)
22-22: TARGETARCH plumb‑through and ShellCheck arch selection look correctWiring
ARG TARGETARCHinto the stage and then mappingTARGETARCH/dpkg --print-architecturetoSC_ARCHfor amd64 and arm64 is correct and matches buildx expectations. Explicitly failing on unsupported architectures is also the right default for this CI image. If you want to be extra clear for future maintainers, you could mention in a comment that only amd64/arm64 are supported here, but behavior‑wise this looks solid.Also applies to: 109-121
ci/test/00_setup_env_native_multiprocess.sh (1)
10-27: HOST selection mirrors TSAN setup; consider fail‑fast and avoiding future driftThe uname/dpkg‑based HOST detection here is consistent with the TSAN setup script and correctly handles amd64/arm64. That’s exactly what we need for multi‑arch multiprocess runs.
Same optional points as for TSAN:
- If
dpkgis unavailable or returns an unexpected arch,HOSTwill be left unset; consider emitting an explicit error and exiting so misconfigured environments don’t fail later in a confusing way.- Since this logic is duplicated in the TSAN script, you might eventually want a small shared helper (or a sourced common script) to keep the mappings in sync, though that’s not strictly necessary for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/build-container.yml(2 hunks).github/workflows/build-depends.yml(4 hunks).github/workflows/build-src.yml(2 hunks).github/workflows/build.yml(6 hunks).github/workflows/test-src.yml(2 hunks)ci/test/00_setup_env_native_multiprocess.sh(1 hunks)ci/test/00_setup_env_native_tsan.sh(1 hunks)contrib/containers/ci/ci-slim.Dockerfile(2 hunks)test/sanitizer_suppressions/tsan(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted
Files:
ci/test/00_setup_env_native_tsan.sh.github/workflows/build-container.yml.github/workflows/build-src.yml.github/workflows/test-src.ymlci/test/00_setup_env_native_multiprocess.sh.github/workflows/build-depends.ymlcontrib/containers/ci/ci-slim.Dockerfile.github/workflows/build.yml
🧠 Learnings (6)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
📚 Learning: 2025-02-19T00:03:39.002Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Applied to files:
.github/workflows/build-container.yml.github/workflows/build-src.yml.github/workflows/test-src.yml.github/workflows/build.yml
📚 Learning: 2025-02-19T00:03:39.001Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
Applied to files:
.github/workflows/build-container.yml.github/workflows/build-src.yml.github/workflows/test-src.yml.github/workflows/build-depends.yml.github/workflows/build.yml
📚 Learning: 2025-11-20T20:03:59.701Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6987
File: contrib/containers/ci/ci-slim.Dockerfile:126-126
Timestamp: 2025-11-20T20:03:59.701Z
Learning: In contrib/containers/ci/ci-slim.Dockerfile, the group handling logic should follow this pattern: if the target GROUP_ID exists, change the ubuntu user's primary group to that GID using `usermod -g ${GROUP_ID} ubuntu`; if the GID doesn't exist, change the existing ubuntu group's GID to the target and rename it to "dash" using `groupmod -g ${GROUP_ID} -n dash ubuntu`. The complete command is: `(getent group ${GROUP_ID} && usermod -g ${GROUP_ID} ubuntu) || groupmod -g ${GROUP_ID} -n dash ubuntu`.
Applied to files:
contrib/containers/ci/ci-slim.Dockerfile
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite
Applied to files:
test/sanitizer_suppressions/tsan
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
test/sanitizer_suppressions/tsan
🪛 actionlint (1.7.9)
.github/workflows/build-container.yml
83-83: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
83-83: shellcheck reported issue in this script: SC2086:info:4:29: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint / Run linters
- GitHub Check: Build container / Build container
🔇 Additional comments (6)
.github/workflows/build.yml (2)
88-99: No issues found. Theubuntu-24.04-armrunner label is a valid, officially supported GitHub Actions runner for Linux arm64, and the reusable workflows have been extended to accept theruns-onparameter as indicated in the PR context.
172-172: Verify that reusable workflows accept and propagate theruns-oninput parameter.The
ubuntu-24.04-armrunner label is valid and supported for public repositories. However, the critical concern remains:build-src.ymlandtest-src.ymlmust defineruns-onas an input parameter and correctly propagate it to their job configurations. Without confirming this architectural detail, the consistent application of ARM runners across the multiprocess and tsan jobs (lines 172, 202, 249, 277) may not work as intended..github/workflows/test-src.yml (1)
18-31: Good pattern for dynamic runner selection.The addition of the configurable
runs-oninput with a sensible default maintains backward compatibility while enabling ARM runner usage for callers..github/workflows/build-src.yml (1)
18-31: Consistent with test-src.yml pattern.Dynamic runner selection is properly integrated here as well.
.github/workflows/build-depends.yml (2)
14-27: Consistent runs-on input pattern.Maintains alignment with other workflow files.
46-46: Excellent architecture-aware caching strategy.The use of
RUNNER_ARCHviauname -mensures separate caches for amd64 and arm64 builds, preventing incompatible cache artifacts across architectures. The restore-key fallback chain maintains good cache hit rates.Also applies to: 78-81
|
|
||
| create-manifest: | ||
| name: Create multi-arch manifest | ||
| runs-on: ubuntu-24.04-arm |
There was a problem hiding this comment.
Create-manifest job should run on standard amd64 runner, not ARM.
The create-manifest job performs pure orchestration via docker buildx imagetools to assemble multi-arch manifests—a task that does not require ARM hardware and is not compute-intensive. Running it on ubuntu-24.04-arm unnecessarily consumes ARM runner capacity that would be better reserved for actual build work.
Change line 116 from runs-on: ubuntu-24.04-arm to runs-on: ubuntu-24.04 to use standard amd64 infrastructure for manifest orchestration.
create-manifest:
name: Create multi-arch manifest
- runs-on: ubuntu-24.04-arm
+ runs-on: ubuntu-24.04
needs: [build-amd64, build-arm64]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runs-on: ubuntu-24.04-arm | |
| create-manifest: | |
| name: Create multi-arch manifest | |
| runs-on: ubuntu-24.04 | |
| needs: [build-amd64, build-arm64] |
🤖 Prompt for AI Agents
.github/workflows/build-container.yml around line 116: the create-manifest job
is configured to run on an ARM runner but only does manifest orchestration, so
change the runner from ubuntu-24.04-arm to the standard amd64 runner by updating
runs-on to ubuntu-24.04 to avoid consuming ARM runner capacity.
There was a problem hiding this comment.
IMO; we should default to running on arm, unless there's a good reason to run on x86
There was a problem hiding this comment.
@PastaPastaPasta, understood! I'll defer to your preference of defaulting to ARM runners. If that's the strategic direction for the project's CI infrastructure, then keeping create-manifest on ubuntu-24.04-arm makes sense for consistency.
Feel free to disregard my suggestion about moving it to amd64. Thanks for clarifying the approach!
🧠 Learnings used
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
| docker buildx imagetools create -t "${REPO}:latest" \ | ||
| "${REPO}:${HASH_TAG}-amd64" \ | ||
| "${REPO}:${HASH_TAG}-arm64" |
There was a problem hiding this comment.
I never realized this but doesn't this mean our latest points to whatever branch ran CI most recently? Is it intentional? Feels like it makes it kind of unpredictable/unusable - could jump from develop to master to any PR head sha. We should probably only do this when develop is updated.
| docker buildx imagetools create -t "${REPO}:latest" \ | |
| "${REPO}:${HASH_TAG}-amd64" \ | |
| "${REPO}:${HASH_TAG}-arm64" | |
| if [ "${TAG}" = "develop" ]; then | |
| docker buildx imagetools create -t "${REPO}:latest" \ | |
| "${REPO}:${HASH_TAG}-amd64" \ | |
| "${REPO}:${HASH_TAG}-arm64" | |
| fi |
There was a problem hiding this comment.
@coderabbitai pls review my comment above. Does it make sense or am I missing smth?
There was a problem hiding this comment.
@UdjinM6 Your analysis is absolutely correct! The current implementation does indeed update the latest tag every time this workflow runs, regardless of which branch triggered it. This means latest could unpredictably point to:
- A
developbuild - A
masterbuild - Any PR branch build
- Any other branch that runs CI
Your suggested fix makes perfect sense:
docker buildx imagetools create -t "${REPO}:${HASH_TAG}" \
"${REPO}:${HASH_TAG}-amd64" \
"${REPO}:${HASH_TAG}-arm64"
docker buildx imagetools create -t "${REPO}:${TAG}" \
"${REPO}:${HASH_TAG}-amd64" \
"${REPO}:${HASH_TAG}-arm64"
if [ "${TAG}" = "develop" ]; then
docker buildx imagetools create -t "${REPO}:latest" \
"${REPO}:${HASH_TAG}-amd64" \
"${REPO}:${HASH_TAG}-arm64"
fiThis ensures that latest consistently points to the most recent develop build, which is the expected and conventional behavior for Docker's latest tag. This is actually a pre-existing issue that existed before this PR's multi-arch changes, but it's good that you caught it now!
🧠 Learnings used
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
|
This pull request has conflicts, please rebase. |
Add support for building CI container images for both linux/amd64 and linux/arm64 architectures: - Add QEMU setup and multi-platform support to build-container workflow - Add runs-on input parameter for runner flexibility - Update ci-slim Dockerfile to dynamically select ShellCheck binary based on target architecture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add runs-on input parameter to reusable workflow files (build-depends, build-src, test-src) with ubuntu-24.04 as default. Configure only the multiprocess job chain (depends, build, test) to use ubuntu-24.04-arm runners. Include RUNNER_ARCH in depends cache keys to prevent mixing x86 and ARM cached artifacts. Make HOST detection dynamic in 00_setup_env_native_multiprocess.sh to build natively for ARM (aarch64-linux-gnu) when running on ARM runners. Refactor build-container.yml to build each architecture natively: - Build amd64 image on ubuntu-24.04 runner - Build arm64 image on ubuntu-24.04-arm runner - Create multi-arch manifest combining both images This eliminates slow QEMU emulation for cross-arch builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
BerkeleyDB uses internal pthread rwlocks with its own locking protocol that TSAN cannot properly reason about, causing false positive lock-order-inversion warnings in wallet tests (e.g., ZapSelectTx). The existing deadlock:libdb suppression doesn't match because BDB is statically linked into test_dash. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add suppression for DatabaseBatch alongside BerkeleyBatch to cover BerkeleyDB lock-order-inversion warnings occurring through the DatabaseBatch::Write code path in wallet benchmarks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add suppression for __db_pthread_mutex to catch all BerkeleyDB internal pthread rwlock operations. This covers MakeBerkeleyDatabase and any other BDB code paths that trigger lock-order-inversion warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
PastaPastaPasta
force-pushed
the
ci/move-multiprocess-arm
branch
from
ba6de41 to
7ef6f3d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/build-container.yml (1)
149-151: Resolve the unresolvedlatesttag behavior (flagged in prior review).The
latesttag continues to be created unconditionally on every workflow run (lines 149-151), regardless of branch. As noted by UdjinM6 in the prior review, this meanslatestunpredictably points to whichever branch ran CI most recently—develop,master, a PR, etc.—rather than consistently tracking the most recentdevelopbuild.To fix, restrict
latesttag creation to only thedevelopbranch:docker buildx imagetools create -t "${REPO}:${TAG}" \ "${REPO}:${HASH_TAG}-amd64" \ "${REPO}:${HASH_TAG}-arm64" + + if [ "${TAG}" = "develop" ]; then + docker buildx imagetools create -t "${REPO}:latest" \ + "${REPO}:${HASH_TAG}-amd64" \ + "${REPO}:${HASH_TAG}-arm64" + fiThen remove lines 149–151 entirely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/build-container.yml(2 hunks).github/workflows/build-depends.yml(3 hunks).github/workflows/build-src.yml(2 hunks).github/workflows/build.yml(6 hunks).github/workflows/test-src.yml(2 hunks)ci/test/00_setup_env_native_multiprocess.sh(1 hunks)ci/test/00_setup_env_native_tsan.sh(1 hunks)contrib/containers/ci/ci-slim.Dockerfile(2 hunks)test/sanitizer_suppressions/tsan(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- contrib/containers/ci/ci-slim.Dockerfile
- .github/workflows/build-depends.yml
- .github/workflows/build-src.yml
- ci/test/00_setup_env_native_multiprocess.sh
- ci/test/00_setup_env_native_tsan.sh
- .github/workflows/test-src.yml
- .github/workflows/build.yml
🧰 Additional context used
📓 Path-based instructions (1)
{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted
Files:
.github/workflows/build-container.yml
🧠 Learnings (4)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
📚 Learning: 2025-02-19T00:03:39.002Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.002Z
Learning: GitHub Actions supports ARM64 runners with the labels `ubuntu-latest-arm64` and `linux-arm64` for public repositories, introduced in January 2025. These runners are specifically designed for ARM64 architecture builds.
Applied to files:
.github/workflows/build-container.yml
📚 Learning: 2025-02-19T00:03:39.001Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6588
File: .github/workflows/release_docker_hub.yml:79-81
Timestamp: 2025-02-19T00:03:39.001Z
Learning: GitHub Actions supports ARM64 runners with the label `ubuntu-22.04-arm` for public repositories, introduced in January 2025. This runner is specifically designed for ARM64 architecture builds.
Applied to files:
.github/workflows/build-container.yml
📚 Learning: 2025-08-01T07:46:37.840Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.
Applied to files:
.github/workflows/build-container.yml
🪛 actionlint (1.7.9)
.github/workflows/build-container.yml
83-83: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
83-83: shellcheck reported issue in this script: SC2086:info:4:29: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint / Run linters
- GitHub Check: Build container / Build container
🔇 Additional comments (1)
test/sanitizer_suppressions/tsan (1)
44-48: TSAN suppressions for BerkeleyDB look well‑scoped and documentedThe added comment clearly explains why these BerkeleyDB deadlock reports are false positives, and the dedicated suppressions for
BerkeleyBatch,DatabaseBatch, and__db_pthread_mutexkeep TSAN noise down without hiding unrelated issues. Placement alongside the other non-actionable/3rd‑party suppressions is reasonable; I don’t see further changes needed here.
| run: | | ||
| BRANCH_NAME=$(echo "${GITHUB_REF##*/}" | tr '[:upper:]' '[:lower:]') | ||
| REPO_NAME=$(echo "${{ github.repository }}" | tr '[:upper:]' '[:lower:]') | ||
| echo "tag=${BRANCH_NAME}" >> $GITHUB_OUTPUT | ||
| echo "repo=${REPO_NAME}" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the actual file content at the specified lines
fd "build-container.yml" --type fRepository: dashpay/dash
Length of output: 168
🌐 Web query:
GitHub Actions GITHUB_OUTPUT variable quoting best practices
💡 Result:
Quick summary — best practices for writing safe, correctly quoted outputs to GITHUB_OUTPUT:
-
Use the heredoc-style delimiter to preserve newlines:
echo "name<<DELIM" >> "$GITHUB_OUTPUT"
echo "" >> "$GITHUB_OUTPUT"
echo "DELIM" >> "$GITHUB_OUTPUT". [1][2] ((stackoverflow.com)) -
Use a unique/random delimiter if the value may contain the delimiter text (avoid accidental termination). [2] ((stackoverflow.com))
-
Protect against shell word-splitting and preserve literal quotes/newlines by assigning the expanded value to an env var or local var, then echo that var (e.g., printf or echo "$var") into GITHUB_OUTPUT. Quote $GITHUB_OUTPUT when redirecting. [3][4] ((stackoverflow.com))
-
For complex data or to guarantee fidelity (including quotes, special chars, very long or binary data), serialize (JSON) or base64-encode the value before writing it, then decode/parse where consumed. This avoids parsing surprises. [1][5][6] ((jstrieb.github.io))
-
If you only need single-line simple strings, do:
echo "name=$value" >> "$GITHUB_OUTPUT" — but be mindful that unquoted expansions can drop literal quotes. [4] ((stackoverflow.com))
Sources: GitHub Actions docs/discussions and practical guides — examples & rationale. [1][2][3][4][5][6] ((stackoverflow.com))
Citations:
- 1: https://stackoverflow.com/questions/74137120/how-to-fix-or-avoid-error-unable-to-process-file-command-output-successfully/74256214?utm_source=openai
- 2: https://stackoverflow.com/questions/74137120/how-to-fix-or-avoid-error-unable-to-process-file-command-output-successfully/74256214?utm_source=openai
- 3: https://stackoverflow.com/questions/77924041/github-action-outputting-a-string-passing-string-with-quotes-to-next-job?utm_source=openai
- 4: https://jstrieb.github.io/posts/github-actions-multiline-outputs/?utm_source=openai
- 5: https://stackoverflow.com/questions/77924041/github-action-outputting-a-string-passing-string-with-quotes-to-next-job?utm_source=openai
- 6: https://stackoverflow.com/questions/74137120/how-to-fix-or-avoid-error-unable-to-process-file-command-output-successfully/74256214?utm_source=openai
Add missing quotes around $GITHUB_OUTPUT in shell redirects.
The prepare steps in both build-amd64 (lines 39-43) and build-arm64 (lines 83-87) redirect to $GITHUB_OUTPUT without quoting the variable. GitHub Actions best practices require quoting the variable to safely handle file redirects.
Apply this diff to both jobs:
- echo "tag=${BRANCH_NAME}" >> $GITHUB_OUTPUT
- echo "repo=${REPO_NAME}" >> $GITHUB_OUTPUT
+ echo "tag=${BRANCH_NAME}" >> "$GITHUB_OUTPUT"
+ echo "repo=${REPO_NAME}" >> "$GITHUB_OUTPUT"Also applies to: 39-43
🧰 Tools
🪛 actionlint (1.7.9)
83-83: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
83-83: shellcheck reported issue in this script: SC2086:info:4:29: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In .github/workflows/build-container.yml around lines 39-43 and 83-87, the shell
redirects use $GITHUB_OUTPUT unquoted which can break when the variable contains
spaces or special chars; update both occurrences to quote the variable in the
redirect (i.e., use "$GITHUB_OUTPUT") so the echo lines safely write to the
file, ensuring both build-amd64 and build-arm64 prepare steps are changed.
The check-cache job was hardcoded to run on ubuntu-latest (x86_64), but it computes HOST using uname -m. When the build job runs on ARM (ubuntu-24.04-arm), it received HOST=x86_64-pc-linux-gnu from the cache-check, causing the ARM build to fail trying to compile for x86_64. Fix by using inputs.runs-on for check-cache so HOST detection matches the actual build runner architecture. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
2 participants
Issue being fixed or feature implemented
run long running tsan / multiprocess builds on arm
What was done?
before tsan took 11:09 https://github.com/dashpay/dash/actions/runs/19945280778/job/57193769222
after tsan took 9:52 https://github.com/PastaPastaPasta/dash/actions/runs/19967178415/job/57305624043
before multiprocess took 48:45 https://github.com/dashpay/dash/actions/runs/19945280778/job/57193769225
after multiprocess took 42:04 https://github.com/PastaPastaPasta/dash/actions/runs/19967178415/job/57305624116
(still waiting on the functional test runs, I expect these to also improve, but not sure)
How Has This Been Tested?
CI looks ~ good here: https://github.com/PastaPastaPasta/dash/actions/runs/19967178415/job/57305624118
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.