Skip to content

ci: migrate primary CI jobs to ARM runners #7232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thepastaclaw wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

ci: migrate primary CI jobs to ARM runners #7232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions .github/workflows/build-depends.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ on:
description: "Runner label to use (e.g., ubuntu-24.04 or ubuntu-24.04-arm)"
required: true
type: string
effective-runs-on:
description: "Optional runner label override used to execute the workflow jobs"
required: false
type: string
Comment on lines +22 to +25
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw May 5, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: effective-runs-on input is no longer used by any caller

After this PR's cleanup, every call site of build-depends.yml in build.yml passes only runs-on: and never effective-runs-on: (depends-linux64 was the last user and was switched to runner-arm64 directly). The effective-runs-on input here, plus the three ${{ inputs.effective-runs-on || inputs.runs-on }} resolutions at lines 40, 74, and 106, are now dead code and just leave room for future drift between the runner label and the cache-key prefix. Since the indirection no longer serves a purpose, the input and the three effective-runs-on || runs-on resolutions can be collapsed back to plain inputs.runs-on. Not a correctness issue at this head — flagging only as cleanup follow-up.

source: ['claude']

outputs:
key:
description: "Key needed for restoring depends cache"
Expand All @@ -33,7 +37,7 @@ on:
jobs:
check-cache:
name: Check cache
runs-on: ${{ inputs.runs-on }}
runs-on: ${{ inputs.effective-runs-on || inputs.runs-on }}
outputs:
cache-hit: ${{ steps.cache-check.outputs.cache-hit }}
cache-key: ${{ steps.setup.outputs.cache-key }}
Expand Down Expand Up @@ -67,7 +71,7 @@ jobs:
echo "DEP_HASH=${DEP_HASH}" >> "${GITHUB_OUTPUT}"
DOCKERFILE_HASH="${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/containers/ci/ci-slim.Dockerfile') }}"
PACKAGES_HASH="${{ hashFiles('depends/packages/*', 'depends/Makefile') }}"
CACHE_KEY_PREFIX="depends-${DOCKERFILE_HASH}-${{ inputs.base-image-digest }}-${{ inputs.runs-on }}-${{ inputs.build-target }}"
CACHE_KEY_PREFIX="depends-${DOCKERFILE_HASH}-${{ inputs.base-image-digest }}-${{ inputs.effective-runs-on || inputs.runs-on }}-${{ inputs.build-target }}"
CACHE_KEY="${CACHE_KEY_PREFIX}-${DEP_HASH}-${PACKAGES_HASH}"
echo "cache-key-prefix=${CACHE_KEY_PREFIX}" >> "${GITHUB_OUTPUT}"
echo "cache-key=${CACHE_KEY}" >> "${GITHUB_OUTPUT}"
Expand Down Expand Up @@ -99,7 +103,7 @@ jobs:
name: Build depends
needs: [check-cache]
if: needs.check-cache.outputs.cache-hit != 'true'
runs-on: ${{ inputs.runs-on }}
runs-on: ${{ inputs.effective-runs-on || inputs.runs-on }}
container:
image: ${{ inputs.container-path }}
options: --user root
Expand Down Expand Up @@ -136,7 +140,7 @@ jobs:
- name: Build depends
run: |
export HOST="${{ needs.check-cache.outputs.host }}"
env ${{ needs.check-cache.outputs.dep-opts }} make -j$(nproc) -C depends
env ${{ needs.check-cache.outputs.dep-opts }} make -j"$(nproc)" -C depends

- name: Save depends cache
uses: actions/cache/save@v5
Expand Down
112 changes: 67 additions & 45 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,26 +112,27 @@ jobs:
runs-on-amd64: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on-arm64: ${{ needs.check-skip.outputs['runner-arm64'] }}

depends-aarch64-linux:
name: aarch64-linux-gnu
depends-linux64:
name: linux64 (native)
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: ${{ vars.SKIP_ARM_LINUX == '' }}
if: |
vars.SKIP_LINUX64 == '' ||
vars.SKIP_LINUX64_FUZZ == '' ||
vars.SKIP_LINUX64_SQLITE == ''
with:
build-target: aarch64-linux
build-target: linux64
container-path: ${{ needs.container.outputs.path }}
base-image-digest: ${{ needs.check-skip.outputs.base-image-digest }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

depends-linux64:
name: x86_64-pc-linux-gnu
depends-linux64-x86:
name: linux64 (x86 canary)
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: |
vars.SKIP_LINUX64 == '' ||
vars.SKIP_LINUX64_FUZZ == '' ||
vars.SKIP_LINUX64_SQLITE == '' ||
vars.SKIP_LINUX64_UBSAN == ''
vars.SKIP_LINUX64_UBSAN == '' ||
vars.SKIP_LINUX64_X86CANARY == ''
with:
build-target: linux64
container-path: ${{ needs.container.outputs.path }}
Expand All @@ -142,25 +143,34 @@ jobs:
name: linux64_multiprocess
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: |
vars.SKIP_LINUX64_MULTIPROCESS == '' ||
vars.SKIP_LINUX64_TSAN == ''
if: ${{ vars.SKIP_LINUX64_TSAN == '' }}
with:
build-target: linux64_multiprocess
container-path: ${{ needs.container.outputs.path }}
base-image-digest: ${{ needs.check-skip.outputs.base-image-digest }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

depends-linux64_multiprocess-x86:
name: linux64_multiprocess (x86)
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: ${{ vars.SKIP_LINUX64_MULTIPROCESS == '' }}
with:
build-target: linux64_multiprocess
container-path: ${{ needs.container.outputs.path }}
base-image-digest: ${{ needs.check-skip.outputs.base-image-digest }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

depends-linux64_nowallet:
name: x86_64-pc-linux-gnu_nowallet
name: linux64_nowallet (native)
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: ${{ vars.SKIP_LINUX64_NOWALLET == '' }}
with:
build-target: linux64_nowallet
container-path: ${{ needs.container.outputs.path }}
base-image-digest: ${{ needs.check-skip.outputs.base-image-digest }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

depends-mac:
name: x86_64-apple-darwin
Expand Down Expand Up @@ -192,18 +202,6 @@ jobs:
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

src-aarch64-linux:
name: aarch64-linux-build
uses: ./.github/workflows/build-src.yml
needs: [check-skip, container, depends-aarch64-linux]
with:
build-target: aarch64-linux
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-aarch64-linux.outputs.key }}
depends-host: ${{ needs.depends-aarch64-linux.outputs.host }}
depends-dep-opts: ${{ needs.depends-aarch64-linux.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

src-linux64:
name: linux64-build
uses: ./.github/workflows/build-src.yml
Expand All @@ -215,7 +213,7 @@ jobs:
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

src-linux64_fuzz:
name: linux64_fuzz-build
Expand All @@ -228,20 +226,20 @@ jobs:
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

src-linux64_multiprocess:
name: linux64_multiprocess-build
uses: ./.github/workflows/build-src.yml
needs: [check-skip, container, depends-linux64_multiprocess, lint]
needs: [check-skip, container, depends-linux64_multiprocess-x86, lint]
Comment thread
thepastaclaw marked this conversation as resolved.
if: ${{ vars.SKIP_LINUX64_MULTIPROCESS == '' }}
with:
build-target: linux64_multiprocess
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64_multiprocess.outputs.key }}
depends-host: ${{ needs.depends-linux64_multiprocess.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_multiprocess.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}
depends-key: ${{ needs.depends-linux64_multiprocess-x86.outputs.key }}
depends-host: ${{ needs.depends-linux64_multiprocess-x86.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_multiprocess-x86.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

src-linux64_nowallet:
name: linux64_nowallet-build
Expand All @@ -253,7 +251,7 @@ jobs:
depends-key: ${{ needs.depends-linux64_nowallet.outputs.key }}
depends-host: ${{ needs.depends-linux64_nowallet.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64_nowallet.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

src-linux64_sqlite:
name: linux64_sqlite-build
Expand All @@ -266,7 +264,7 @@ jobs:
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

src-linux64_tsan:
name: linux64_tsan-build
Expand All @@ -284,14 +282,27 @@ jobs:
src-linux64_ubsan:
name: linux64_ubsan-build
uses: ./.github/workflows/build-src.yml
needs: [check-skip, container, depends-linux64]
needs: [check-skip, container, depends-linux64-x86]
if: ${{ vars.SKIP_LINUX64_UBSAN == '' }}
with:
build-target: linux64_ubsan
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64.outputs.key }}
depends-host: ${{ needs.depends-linux64.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64.outputs.dep-opts }}
depends-key: ${{ needs.depends-linux64-x86.outputs.key }}
depends-host: ${{ needs.depends-linux64-x86.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64-x86.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

src-linux64_x86canary:
name: linux64_x86canary-build
uses: ./.github/workflows/build-src.yml
needs: [check-skip, container, depends-linux64-x86]
if: ${{ vars.SKIP_LINUX64_X86CANARY == '' }}
with:
build-target: linux64_x86canary
container-path: ${{ needs.container.outputs.path }}
depends-key: ${{ needs.depends-linux64-x86.outputs.key }}
depends-host: ${{ needs.depends-linux64-x86.outputs.host }}
depends-dep-opts: ${{ needs.depends-linux64-x86.outputs.dep-opts }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

src-mac:
Expand Down Expand Up @@ -326,7 +337,7 @@ jobs:
bundle-key: ${{ needs.src-linux64.outputs.key }}
build-target: linux64
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

test-linux64_multiprocess:
name: linux64_multiprocess-test
Expand All @@ -336,7 +347,7 @@ jobs:
bundle-key: ${{ needs.src-linux64_multiprocess.outputs.key }}
build-target: linux64_multiprocess
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
Comment on lines 231 to +350
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: multiprocess lane moved from ARM to x86 — contradicts PR description and drops ARM coverage

src-linux64_multiprocess (build.yml:232-243) now depends on the new x86-only depends-linux64_multiprocess-x86 (build.yml:154-163) and runs on runner-amd64, and test-linux64_multiprocess (build.yml:343-351) likewise runs on runner-amd64. Only the TSAN lane still consumes the ARM-side depends-linux64_multiprocess. This contradicts the PR description's claim that "multiprocess + tsan were already on ARM — no change" and removes ARM coverage of the one lane that exercises the multiprocess/v2transport configuration, so any ARM-only regression there will now be invisible to CI. The new linux64_x86canary lane already provides x86 smoke coverage, so moving multiprocess back off ARM does not serve that goal. Either restore multiprocess to ARM runners (keeping the x86 canary as the sole x86 coverage addition), or — if the move is intentional (e.g. multiprocess is currently unstable on ARM) — update the PR description and commit message to state that multiprocess was moved to x86 and explain why.

source: ['claude', 'codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/build.yml`:
- [SUGGESTION] lines 232-351: multiprocess lane moved from ARM to x86 — contradicts PR description and drops ARM coverage
  `src-linux64_multiprocess` (build.yml:232-243) now depends on the new x86-only `depends-linux64_multiprocess-x86` (build.yml:154-163) and runs on `runner-amd64`, and `test-linux64_multiprocess` (build.yml:343-351) likewise runs on `runner-amd64`. Only the TSAN lane still consumes the ARM-side `depends-linux64_multiprocess`. This contradicts the PR description's claim that "multiprocess + tsan were already on ARM — no change" and removes ARM coverage of the one lane that exercises the multiprocess/v2transport configuration, so any ARM-only regression there will now be invisible to CI. The new `linux64_x86canary` lane already provides x86 smoke coverage, so moving multiprocess back off ARM does not serve that goal. Either restore multiprocess to ARM runners (keeping the x86 canary as the sole x86 coverage addition), or — if the move is intentional (e.g. multiprocess is currently unstable on ARM) — update the PR description and commit message to state that multiprocess was moved to x86 and explain why.

Comment on lines 153 to +350
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 25, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: The regular multiprocess lane was moved off ARM, leaving no non-TSAN arm64 coverage for that configuration

This change introduces depends-linux64_multiprocess-x86 on amd64 and rewires both src-linux64_multiprocess and test-linux64_multiprocess to use it and run on runner-amd64. The existing arm64 depends-linux64_multiprocess job is now only consumed by linux64_tsan. That is a real coverage regression: the only normal multiprocess/v2transport build-and-test path no longer runs on arm64, and TSAN is not a substitute because it exercises a different sanitizer configuration. If the intent is to keep multiprocess on x86, the commit message and PR summary need to say so explicitly; otherwise this lane should stay on arm64 so the migration does not silently drop native ARM coverage.

source: ['claude', 'codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/build.yml`:
- [SUGGESTION] lines 154-351: The regular multiprocess lane was moved off ARM, leaving no non-TSAN arm64 coverage for that configuration
  This change introduces `depends-linux64_multiprocess-x86` on amd64 and rewires both `src-linux64_multiprocess` and `test-linux64_multiprocess` to use it and run on `runner-amd64`. The existing arm64 `depends-linux64_multiprocess` job is now only consumed by `linux64_tsan`. That is a real coverage regression: the only normal multiprocess/v2transport build-and-test path no longer runs on arm64, and TSAN is not a substitute because it exercises a different sanitizer configuration. If the intent is to keep multiprocess on x86, the commit message and PR summary need to say so explicitly; otherwise this lane should stay on arm64 so the migration does not silently drop native ARM coverage.

Comment on lines 347 to +350
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 26, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: Non-TSAN multiprocess lane no longer has native ARM coverage

After this migration, depends-linux64_multiprocess on arm64 (lines 143–152) is only triggered when SKIP_LINUX64_TSAN == '', and the only consumer of that arm64 depends cache is the TSAN build/test pair (src-linux64_tsan/test-linux64_tsan at lines 270–281 and 373–381). The regular multiprocess/v2transport lane has been redirected to the new amd64-only depends-linux64_multiprocess-x86 and runs both build (lines 232–243) and test (lines 343–351) on runner-amd64. The PR description states multiprocess was intentionally kept on x86 due to instability observed during ARM validation, but this means any ARM-only regression in the non-TSAN multiprocess configuration will escape CI. TSAN does not substitute for the regular build/test lane (different sanitizer, different runtime characteristics). Consider tracking restoration of native ARM coverage for the non-TSAN multiprocess configuration once the underlying instability is understood, or explicitly document this gap so it is not forgotten.

source: ['codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/build.yml`:
- [SUGGESTION] lines 143-351: Non-TSAN multiprocess lane no longer has native ARM coverage
  After this migration, `depends-linux64_multiprocess` on arm64 (lines 143–152) is only triggered when `SKIP_LINUX64_TSAN == ''`, and the only consumer of that arm64 depends cache is the TSAN build/test pair (`src-linux64_tsan`/`test-linux64_tsan` at lines 270–281 and 373–381). The regular multiprocess/v2transport lane has been redirected to the new amd64-only `depends-linux64_multiprocess-x86` and runs both build (lines 232–243) and test (lines 343–351) on `runner-amd64`. The PR description states multiprocess was intentionally kept on x86 due to instability observed during ARM validation, but this means any ARM-only regression in the non-TSAN multiprocess configuration will escape CI. TSAN does not substitute for the regular build/test lane (different sanitizer, different runtime characteristics). Consider tracking restoration of native ARM coverage for the non-TSAN multiprocess configuration once the underlying instability is understood, or explicitly document this gap so it is not forgotten.


test-linux64_nowallet:
name: linux64_nowallet-test
Expand All @@ -346,7 +357,7 @@ jobs:
bundle-key: ${{ needs.src-linux64_nowallet.outputs.key }}
build-target: linux64_nowallet
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

test-linux64_sqlite:
name: linux64_sqlite-test
Expand All @@ -356,7 +367,7 @@ jobs:
bundle-key: ${{ needs.src-linux64_sqlite.outputs.key }}
build-target: linux64_sqlite
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}

test-linux64_tsan:
name: linux64_tsan-test
Expand All @@ -377,3 +388,14 @@ jobs:
build-target: linux64_ubsan
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
Comment on lines 129 to 390
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 25, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: UBSAN was not actually migrated to arm64

depends-linux64-x86 is the only depends cache feeding src-linux64_ubsan, and both src-linux64_ubsan and test-linux64_ubsan still run on runner-amd64. That directly contradicts the commit message, which says depends-linux64 and its consumers, including ubsan, were moved to ARM and lists only nowallet, mac, win64, and lint as the jobs remaining on x86. Either move the UBSAN lane onto arm64 or correct the change description to state that UBSAN is intentionally still x86-only.

source: ['codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/build.yml`:
- [SUGGESTION] lines 130-391: UBSAN was not actually migrated to arm64
  `depends-linux64-x86` is the only depends cache feeding `src-linux64_ubsan`, and both `src-linux64_ubsan` and `test-linux64_ubsan` still run on `runner-amd64`. That directly contradicts the commit message, which says `depends-linux64` and its consumers, including `ubsan`, were moved to ARM and lists only `nowallet`, `mac`, `win64`, and `lint` as the jobs remaining on x86. Either move the UBSAN lane onto arm64 or correct the change description to state that UBSAN is intentionally still x86-only.


test-linux64_x86canary:
name: linux64_x86canary-test
uses: ./.github/workflows/test-src.yml
needs: [check-skip, container-slim, src-linux64_x86canary, lint]
with:
bundle-key: ${{ needs.src-linux64_x86canary.outputs.key }}
build-target: linux64_x86canary
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}
integration-tests-args: "--exclude feature_pruning,feature_dbcrash"
9 changes: 7 additions & 2 deletions .github/workflows/test-src.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ on:
description: "Runner label to use (e.g., ubuntu-24.04 or ubuntu-24.04-arm)"
required: true
type: string
integration-tests-args:
description: "Override for INTEGRATION_TESTS_ARGS passed to test_integrationtests.sh"
required: false
type: string
default: "--extended --exclude feature_pruning,feature_dbcrash"

env:
INTEGRATION_TESTS_ARGS: "--extended --exclude feature_pruning,feature_dbcrash"
INTEGRATION_TESTS_ARGS: ${{ inputs.integration-tests-args }}
CI_FAILFAST_TEST_LEAVE_DANGLING: 1 # GHA does not care about dangling processes and setting this variable avoids killing the CI script itself on error

jobs:
Expand Down Expand Up @@ -49,7 +54,7 @@ jobs:
with:
path: |
releases
key: releases-${{ hashFiles('ci/test/00_setup_env_native_qt5.sh', 'test/get_previous_releases.py') }}
key: releases-${{ runner.arch }}-${{ hashFiles('ci/test/00_setup_env_native_qt5.sh', 'test/get_previous_releases.py') }}
Comment on lines 54 to +57
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: Releases cache keyed by runner.arch but still gated on build-target == 'linux64'

The cache step correctly includes ${{ runner.arch }} in the key so aarch64 and x86 binaries don't collide, but if: inputs.build-target == 'linux64' means only the primary linux64 target benefits from this cache. linux64_x86canary sets DOWNLOAD_PREVIOUS_RELEASES=false, so it's fine today — but if another target on another runner ever enables previous-release downloads, it'll silently re-download every run. Either expand the gate (non-trivial since DOWNLOAD_PREVIOUS_RELEASES is decided inside the container script), or add a comment noting the cache is intentionally linux64-only.

source: ['claude']


- name: Run functional tests
id: test
Expand Down
6 changes: 3 additions & 3 deletions ci/dash/matrix.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/l
export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1"
export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"

if [ "$BUILD_TARGET" = "aarch64-linux" ]; then
source ./ci/test/00_setup_env_aarch64.sh
elif [ "$BUILD_TARGET" = "linux64" ]; then
if [ "$BUILD_TARGET" = "linux64" ]; then
source ./ci/test/00_setup_env_native_qt5.sh
elif [ "$BUILD_TARGET" = "linux64_asan" ]; then
source ./ci/test/00_setup_env_native_asan.sh
Expand All @@ -34,6 +32,8 @@ elif [ "$BUILD_TARGET" = "linux64_tsan" ]; then
source ./ci/test/00_setup_env_native_tsan.sh
elif [ "$BUILD_TARGET" = "linux64_ubsan" ]; then
source ./ci/test/00_setup_env_native_ubsan.sh
elif [ "$BUILD_TARGET" = "linux64_x86canary" ]; then
source ./ci/test/00_setup_env_native_x86canary.sh
elif [ "$BUILD_TARGET" = "linux64_valgrind" ]; then
source ./ci/test/00_setup_env_native_valgrind.sh
elif [ "$BUILD_TARGET" = "mac" ]; then
Expand Down
5 changes: 3 additions & 2 deletions ci/dash/test_integrationtests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib
if [ "$DOWNLOAD_PREVIOUS_RELEASES" = "true" ]; then
echo "Downloading previous releases..."
# shellcheck disable=SC2086
./test/get_previous_releases.py -b -t "$PREVIOUS_RELEASES_DIR"
./test/get_previous_releases.py -b -t "$PREVIOUS_RELEASES_DIR" ${PREVIOUS_RELEASES_TAGS:-}
fi

cd "build-ci/dashcore-$BUILD_TARGET"
Expand All @@ -43,8 +43,9 @@ echo "Using socketevents mode: $SOCKETEVENTS"
EXTRA_ARGS="--dashd-arg=-socketevents=$SOCKETEVENTS"

set +e
# Keep PASS_ARGS before TEST_RUNNER_EXTRA so per-target flags can override workflow defaults.
# shellcheck disable=SC2086
LD_LIBRARY_PATH="$DEPENDS_DIR/$HOST/lib" ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=4000 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir="$(pwd)/testdatadirs" $PASS_ARGS $EXTRA_ARGS
LD_LIBRARY_PATH="$DEPENDS_DIR/$HOST/lib" ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=4000 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" $PASS_ARGS ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir="$(pwd)/testdatadirs" $EXTRA_ARGS
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: PASS_ARGS / TEST_RUNNER_EXTRA order swapped — silently flips precedence for all CI targets

The diff swaps the order from ... ${TEST_RUNNER_EXTRA} ... $PASS_ARGS $EXTRA_ARGS to ... $PASS_ARGS ${TEST_RUNNER_EXTRA} ... $EXTRA_ARGS. With argparse, later occurrences of overlapping flags (e.g. --exclude ...) take precedence, so TEST_RUNNER_EXTRA (set per-target in 00_setup_env_native_*.sh) now overrides PASS_ARGS (the args forwarded via INTEGRATION_TESTS_ARGS from test-src.yml) for every existing target — linux64, sqlite, ubsan, tsan, multiprocess, nowallet — not just the new x86canary lane. If this was intentional so that target-specific excludes (like aarch64 qt5's feature_unsupported_utxo_db exclusion) survive external overrides, split it into its own commit with an explanation; otherwise revert the swap, since it's a silent behavioral change hidden inside a runner-migration commit.

source: ['claude']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `ci/dash/test_integrationtests.sh`:
- [SUGGESTION] line 47: PASS_ARGS / TEST_RUNNER_EXTRA order swapped — silently flips precedence for all CI targets
  The diff swaps the order from `... ${TEST_RUNNER_EXTRA} ... $PASS_ARGS $EXTRA_ARGS` to `... $PASS_ARGS ${TEST_RUNNER_EXTRA} ... $EXTRA_ARGS`. With argparse, later occurrences of overlapping flags (e.g. `--exclude ...`) take precedence, so `TEST_RUNNER_EXTRA` (set per-target in `00_setup_env_native_*.sh`) now overrides `PASS_ARGS` (the args forwarded via `INTEGRATION_TESTS_ARGS` from test-src.yml) for every existing target — linux64, sqlite, ubsan, tsan, multiprocess, nowallet — not just the new x86canary lane. If this was intentional so that target-specific excludes (like aarch64 qt5's `feature_unsupported_utxo_db` exclusion) survive external overrides, split it into its own commit with an explanation; otherwise revert the swap, since it's a silent behavioral change hidden inside a runner-migration commit.

RESULT=$?
set -e

Expand Down
27 changes: 25 additions & 2 deletions ci/test/00_setup_env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,31 @@ export MAKEJOBS=${MAKEJOBS:--j$(nproc)}
export BASE_SCRATCH_DIR=${BASE_SCRATCH_DIR:-$BASE_ROOT_DIR/ci/scratch}
# What host to compile for. See also ./depends/README.md
# Tests that need cross-compilation export the appropriate HOST.
# Tests that run natively guess the host
export HOST=${HOST:-$("$BASE_ROOT_DIR/depends/config.guess")}
# Tests that run natively detect the host based on architecture.
# We use explicit triplets rather than config.guess to ensure they match
# the triplets used by depends (e.g. aarch64-linux-gnu, not aarch64-unknown-linux-gnu).
if [ -z "$HOST" ]; then
case "$(uname -m)" in
aarch64)
export HOST=aarch64-linux-gnu
;;
x86_64)
export HOST=x86_64-pc-linux-gnu
;;
*)
if command -v dpkg >/dev/null 2>&1; then
arch="$(dpkg --print-architecture)"
if [ "${arch}" = "arm64" ]; then
export HOST=aarch64-linux-gnu
elif [ "${arch}" = "amd64" ]; then
export HOST=x86_64-pc-linux-gnu
fi
fi
# Final fallback to config.guess
export HOST=${HOST:-$("$BASE_ROOT_DIR/depends/config.guess")}
;;
esac
fi
Comment on lines +34 to +58
Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: Catch-all branch may fall back to config.guess triplet that doesn't match depends directory

In the *) branch, if uname -m is neither aarch64 nor x86_64, dpkg is then consulted; if dpkg returns something other than arm64 or amd64, HOST stays unset and the final export HOST=${HOST:-$("$BASE_ROOT_DIR/depends/config.guess")} takes over. For exotic archs that fallback yields strings like armv7l-unknown-linux-gnueabihf, which typically don't match any depends/$HOST/ directory — so subsequent $DEPENDS_DIR/$HOST/lib usage will fail. Not a regression (old single-line fallback had the same issue), but since this PR is explicitly centralizing HOST detection to avoid the aarch64-unknown-linux-gnu vs aarch64-linux-gnu mismatch, consider either documenting that exotic archs aren't supported or emitting a warning from the catch-all. Low priority.

source: ['claude']

# Whether to prefer BusyBox over GNU utilities
export USE_BUSY_BOX=${USE_BUSY_BOX:-false}

Expand Down
Loading
Loading