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
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
105 changes: 64 additions & 41 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,28 @@ 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'] }}
effective-runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}
Comment on lines +115 to +128
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: runs-on is dead when effective-runs-on is also supplied on depends-linux64

depends-linux64 passes both runs-on: runner-amd64 and effective-runs-on: runner-arm64. In build-depends.yml every consumer (check-cache.runs-on at line 40, build.runs-on at line 106, and the cache-key prefix at line 74) reads inputs.effective-runs-on || inputs.runs-on, so the runner-amd64 value is never actually used — the job executes on ARM and the cache key contains runner-arm64. The call site reads as if the fallback matters, which it doesn't. Either drop runs-on here and set the ARM runner directly, or add a comment explaining what runs-on means when effective-runs-on is also set (logical target vs physical runner).

💡 Suggested change
Suggested change
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'] }}
effective-runs-on: ${{ needs.check-skip.outputs['runner-arm64'] }}
depends-linux64:
name: linux64 (native)
uses: ./.github/workflows/build-depends.yml
needs: [check-skip, container, cache-sources]
if: |
vars.SKIP_LINUX64 == '' ||
vars.SKIP_LINUX64_FUZZ == '' ||
vars.SKIP_LINUX64_SQLITE == ''
with:
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-arm64'] }}

source: ['claude']


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,17 +144,26 @@ 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 == '' }}
Expand Down Expand Up @@ -192,18 +203,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 +214,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 +227,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]
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 18, 2026

Choose a reason for hiding this comment

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

P2 Badge Gate ARM multiprocess depends on TSAN skip flag only

Now that src-linux64_multiprocess was switched to depends-linux64_multiprocess-x86, the ARM depends-linux64_multiprocess job is effectively only for TSAN, but its if still tracks SKIP_LINUX64_MULTIPROCESS as well. In runs where SKIP_LINUX64_TSAN is set (skip TSAN) and multiprocess remains enabled, CI still executes an unused ARM depends build, and a failure there can fail the workflow even though no downstream job needs it.

Useful? React with 👍 / 👎.

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 @@ -266,7 +265,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 +283,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 +338,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 +348,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 232 to +351
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.


test-linux64_nowallet:
name: linux64_nowallet-test
Expand All @@ -356,7 +368,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 +389,14 @@ jobs:
build-target: linux64_ubsan
container-path: ${{ needs.container-slim.outputs.path }}
runs-on: ${{ needs.check-skip.outputs['runner-amd64'] }}

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
4 changes: 2 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 @@ -44,7 +44,7 @@ EXTRA_ARGS="--dashd-arg=-socketevents=$SOCKETEVENTS"

set +e
# 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
26 changes: 0 additions & 26 deletions ci/test/00_setup_env_aarch64.sh
View file
Open in desktop

This file was deleted.

Loading
Loading