Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.
In both jobs the fuzz binary is built with -DBUILD_FOR_FUZZING to enable Assume assertions as well as FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31221.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hebasto, maflcko, achow101 |
| Concept ACK | marcofleon, fanquake |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
bitcoin-chainstate.exe to bitcoinkernel.dll on Windows by hebasto)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
73@@ -74,20 +74,27 @@ jobs:
74 # Use clang++, because it is a bit faster and uses less memory than g++
75 git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWERROR=ON -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_USDT=ON -DCMAKE_CXX_FLAGS='-Wno-error=unused-member-function' && cmake --build build -j $(nproc) && ctest --output-on-failure --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }}
76
77- macos-native-arm64:
78- name: 'macOS 14 native, arm64, no depends, sqlite only, gui'
79- # Use latest image, but hardcode version to avoid silent upgrades (and breaks).
80- # See: https://github.com/actions/runner-images#available-images.
81+ macos-native:
82+ name: 'macOS 14 native'
You might want to use ${{ matrix.job_name }}.
Same for the Windows job.
job-name not job_name
205 uses: actions/cache/save@v4
206 if: github.event_name != 'pull_request' && steps.vcpkg-binary-cache.outputs.cache-hit != 'true'
207 with:
208 path: ~/AppData/Local/vcpkg/archives
209- key: ${{ github.job }}-vcpkg-binary-${{ hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json') }}
210+ key: ${{ matrix.job-type }}-vcpkg-binary-${{ hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json') }}
161+ - job-type: standard
162+ generate-options: '-DBUILD_GUI=ON -DWITH_BDB=ON -DWITH_ZMQ=ON -DBUILD_BENCH=ON -DWERROR=ON'
163+ job-name: 'Win64 native, VS 2022'
164+ - job-type: fuzz
165+ generate-options: '-DBUILD_FOR_FUZZING=ON -DWERROR=ON'
166+ job-name: 'Win64 native fuzz, VS 2022'
It seems a bit annoying that the Windows build is now duplicated and seems to take 2h each times 2 = 4 CI hours total.
I wonder if this could be avoided (or reduced) by using a cmake multi config build. Putting the fuzz result into a separate folder and then using that in the fuzz_runner should be enough?
It seems a bit annoying that the Windows build is now duplicated and seems to take 2h each times 2 = 4 CI hours total.
I assumed this is a caching issue? This shouldn’t be doing anything different from the current windows CI…
I would also think we can just turn off everything GUI related off for fuzzing?
I don’t think this is possible while sharing the cache
Looks like it’s rebuilding everything again, could it be that it can’t store new caches on PRs?
The pkg cache is not stored on PRs (see here).
But I think the main problem was that I changed the job name from win64-native to windows-native which changed the lookup key for the cache (I assume). Re-pushed again.
129@@ -120,6 +130,8 @@ jobs:
130
131 - name: CI script
132 run: ./ci/test_run_all.sh
133+ env:
134+ FILE_ENV: ${{ matrix.file-env }}
135
136 - name: Save Ccache cache
137 uses: actions/cache/save@v4
have a separate ccache for each job type
Done
skimmed over the diff and a bit of the CI output and it appears plausible.
lgtm ACK c3354ea6dc34a3cadc60346c363478af22a5c0c2
92+ - job-type: standard
93+ file-env: './ci/test/00_setup_env_mac_native.sh'
94+ job-name: 'macOS 14 native, arm64, no depends, sqlite only, gui'
95+ - job-type: fuzz
96+ file-env: './ci/test/00_setup_env_mac_native_fuzz.sh'
97+ job-name: 'macOS 14 native arm64 fuzz'
job-name?
6+
7+export LC_ALL=C.UTF-8
8+
9+# Homebrew's python@3.12 is marked as externally managed (PEP 668).
10+# Therefore, `--break-system-packages` is needed.
11+export PIP_PACKAGES="--break-system-packages zmq"
160+ include:
161+ - job-type: standard
162+ generate-options: '-DBUILD_GUI=ON -DWITH_BDB=ON -DWITH_ZMQ=ON -DBUILD_BENCH=ON -DWERROR=ON'
163+ job-name: 'Win64 native, VS 2022'
164+ - job-type: fuzz
165+ generate-options: '-DBUILD_FOR_FUZZING=ON -DWERROR=ON'
This allows skipping the build of unneeded vcpkg packages:
0 generate-options: '-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite" -DBUILD_GUI=OFF -DBUILD_FOR_FUZZING=ON -DWERROR=ON'
-DBUILD_GUI=OFF is implied by -DBUILD_FOR_FUZZING=ON.
What does -DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite" do? we’re using the same vcpkg cache for both jobs would that still work with this change?
-DBUILD_GUI=OFFis implied by-DBUILD_FOR_FUZZING=ON.
Yes. But it is evaluated after an attempt to look for the Qt packages. In this case, -DBUILD_GUI=OFF simply negates -DBUILD_GUI=ON from the “vs2022-static” preset.
What does
-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite"do?
The optional vcpkg packages are organised in “features” in the vcpkg.json manifest file.
-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite" means “build and install only the sqlite3 package, skipping all other optional packages”.
we’re using the same vcpkg cache for both jobs would that still work with this change?
To be precise, both jobs read the vcpkg binary cache, which works for a subset of packages just fine, and only the “standard” job writes the cache, which is correct.
Approach ACK c3354ea6dc34a3cadc60346c363478af22a5c0c2.
Please add strategy.fail-fast: false everywhere.
0@@ -0,0 +1,16 @@
1+#!/usr/bin/env bash
2+#
3+# Copyright (c) The Bitcoin Core developers
nit: Add a year?
0# Copyright (c) 2024-present The Bitcoin Core developers
