faeb1babe2 ci: refactor: Use version id over version codename consistently (MarcoFalke)
fae322a43a ci: Slim down lint image (MarcoFalke)
3333273a8f ci: Bump lint imagefile FROM base (MarcoFalke)
Pull request description:
Currently, the lint_test_runner is built and installed into the lint CI image. This is problematic, because it triggers a full image build on every change to its source code. Doing a build of the lint test_runner on every run is easier and faster.
ACKs for top commit:
l0rinc:
ACK faeb1babe2
janb84:
Re- ACK [faeb1ba](faeb1babe2)
Tree-SHA512: 39103e61ec2587096213bc1ce55b80087f6f03775592827d8c96a366453b798570d912690bf96fde4685798e5fc8ee2695ce851f473b4c8782d1a4c50c65a594
fa21f83d29 ci: Use G++ in valgrind tasks (MarcoFalke)
fabd05bf65 refactor: Fix net_processing iwyu includes (MarcoFalke)
fa1622db20 refactor: Make node_id a const& in RemoveBlockRequest (MarcoFalke)
Pull request description:
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7
It is possible to work around this bug by pulling at least one value from the stack. For example, by making `from_peer` a `const` reference. Alternatively, by replacing `auto [node_id, list_it]` with `const auto& [node_id, list_it]`, which is done here.
I think this workaround is acceptable, because it does not look like valgrind can trivially fix this. The alternative would be to add a (temporary?) suppression.
Fixes https://github.com/bitcoin/bitcoin/issues/27741
Also, fix iwyu includes, while touching this module.
Also, switch the CI valgrind scripts to use G++.
ACKs for top commit:
achow101:
ACK fa21f83d29
TheCharlatan:
ACK fa21f83d29
darosior:
utACK fa21f83d29
ryanofsky:
Code review ACK fa21f83d29. Code changes all look good but I'm a little confused about purpose of the third commit, so left a question about that
Tree-SHA512: 7b92cdafd525a5ac53ae2c1a7a92e599bc9b5fd5d315a694b493cd5079ac323d884393b57aa18581b7789247a588c9a27d47698de25b340bc76fc9f1dd1850b4
25b56fd9b4 ci: Test cross-built Windows executables on Windows natively (Hennadii Stepanov)
3501bca8c7 ci: Move "Windows cross" job from Cirrus CI to GHA CI (Hennadii Stepanov)
f8619196ce ci: Use `bash` by default for all platforms (Hennadii Stepanov)
Pull request description:
This PR enables on the CI tests of cross-compiled Windows binaries on Windows.
It is important to have such tests in CI because the release binaries for Windows are also cross-compiled.
Two functional tests, `wallet_migration.py` and `wallet_multiwallet.py`, are temporarily disabled. They require fixes, such as https://github.com/bitcoin/bitcoin/pull/31410, and adjustments for error message handling. Re-enabling these tests will be addressed in follow-up PRs.
Resolves https://github.com/bitcoin/bitcoin/issues/31071.
ACKs for top commit:
davidgumberg:
tested reACK 25b56fd9b4
hodlinator:
re-ACK 25b56fd9b4
willcl-ark:
utACK 25b56fd9b4
maflcko:
review-only ACK 25b56fd9b4🍎
Tree-SHA512: fb9150807b7ebb248e8f4fe7b16e5179251e7be9336459287787f27e542583d73d937e6969667fd836378b676bb9be7f66756dc1abca8a01364bc9ee3e3720a5
c8fab35617 ci: remove -Wno-error=deprecated-declarations from ASAN (fanquake)
a130bbd154 Squashed 'src/leveldb/' changes from 04b5790928..4188247086 (fanquake)
Pull request description:
Cherry-picks two commits from upstream (302786e211, e829478c6a), which remove the usage of `std::aligned_storage/std::aligned_union`.
Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. See https://github.com/google/leveldb/pull/1249 for more details.
Also see https://issues.chromium.org/issues/388068052, although note that they [reverted the roll to latest leveldb](https://issues.chromium.org/issues/388068052#comment9). I'm guessing due to the acidental reversion issue above.
ACKs for top commit:
l0rinc:
ACK c8fab35617
darosior:
ACK c8fab35617 -- checked it's a clean subtree pull from https://github.com/bitcoin-core/leveldb-subtree/tree/bitcoin-fork
dergoegge:
utACK c8fab35617
Tree-SHA512: 966e61b9ac88af5ae7bf71514bfd5bbdbd8c38c7af65feb6d5e4415062dcff5896dc33fe968ded3462cc599abd921d49ee8336db3e12ed3f59c91ceb949317b7
9b033bebb1 cmake: rename Kernel component to bitcoinkernel for consistency (Cory Fields)
2e0c92558e cmake: add and use install_binary_component (Cory Fields)
0264c5d86c cmake: use per-target components for bitcoin-qt and bitcoin-gui (Cory Fields)
fb0546b1c5 ci: don't try to install for a fuzz build (Cory Fields)
Pull request description:
This makes it possible to build/install only the desired binaries regardless of the configuration.
For consistency, the component names match the binary names. `Kernel` and `GUI` have been renamed.
Additionally it fixes#31762 by installing only the manpages for the configured targets (and includes them in the component installs for each).
Also fixes#31745.
Alternative to #31765 which is (imo) more correct/thorough.
Can be tested using (for ex):
```bash
$ cmake -B build
$ cmake --build build -t bitcoind -t bitcoin-cli
$ cmake --install build --component bitcoind
$ cmake --install build --component bitcoin-cli
```
ACKs for top commit:
hebasto:
ACK 9b033bebb1.
TheCharlatan:
Re-ACK 9b033bebb1
stickies-v:
re-ACK 9b033bebb1
Tree-SHA512: fd4818e76f190dbeafbf0c246b466f829771902c9d6d7111ed917093b811c8a5536a4a45e20708f73e7f581d6cb77c8e61cfa69e065788dcf0886792f553a355
fa952acdb6 ci: Skip read-write of default env vars (MarcoFalke)
Pull request description:
If they remain unset, they use the default anyway. Except for `USER`, but this seems unused anyway.
Can be checked via:
```
sh-5.2# touch /tmp/empty_env
sh-5.2# podman run --rm --env-file /tmp/empty_env 'ubuntu:24.04' env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
container=podman
HOME=/root
HOSTNAME=19ece5c9e052
ACKs for top commit:
0xB10C:
ACK fa952acdb6
Prabhat1308:
utACK [fa952ac](fa952acdb6)
Tree-SHA512: fe0c173b23cfda3025306303a44ffe32ecc57c2e0e1a2376594696f9887ed22f5105da84e898e790041bf15a4aa42a365fba016710ad269d439dda691977be90
Currently the manpages are installed, but that is a bug. An upcoming commit
will avoid installing manpages for targets that aren't configured, which
removes the "install" target for fuzz builds.
fa5a02bcfa ci: Use clang-20 for sanitizer tasks (MarcoFalke)
Pull request description:
A new clang version generally comes with bugfixes, new (sanitizer) features, and deprecations.
Upgrade the sanitizer tasks to use the new version.
This was also suggested in https://github.com/bitcoin/bitcoin/pull/31691#issuecomment-2602517116
ACKs for top commit:
fanquake:
ACK fa5a02bcfa - tested 20 in some other infra, we just needed to fix the same deprecation warnings we'd seen, in cryptofuzz: 09ca550c3e.
Tree-SHA512: 6114d790b5d7145eb5f019e02da6c2c833342707ad67fb9f9c09506001afbef0c9b9beee7e51321f17f12ea692509d6428e6072ad105dba51e4d54cd057621cd
8888ee4403 ci: Allow build dir on CI host (MarcoFalke)
Pull request description:
This is required to pass cross builds on to a different machine after the build.
See for example https://github.com/bitcoin/bitcoin/pull/31176, but this pull will also allow someone to implement it outside this repo.
ACKs for top commit:
davidgumberg:
lgtm ACK 8888ee4403
hebasto:
re-ACK 8888ee4403.
Tree-SHA512: a1e2c32bc1b95efbd0b48287ac5b49e0e1bacbf5a5800845be5352bbdd3e17fa478e90348b2e94e95cf3ae863cdf75ab444089376588f6f8eec438f73a4b5b97
e87429a2d0 ci: optionally use local docker build cache (0xb10c)
Pull request description:
By setting `DANGER_DOCKER_BUILD_CACHE_HOST_DIR`, the task-specific docker images built during the CI run can be cached. This allows, for example, ephemeral CI runners to reuse the docker images (or layers of it) from earlier runs, by persisting the image cache before the ephemeral CI runner is shut down. The cache keyed by `CONTAINER_NAME`.
As `--cache-to` doesn't remove old cache files, the existing cache is removed after a successful `docker build` and the newly cached image is moved to it's location to avoid the cache from growing indefinitely with old, unused layers.
When `--cache-from` doesn't find the directory, the cached version is a cache-miss, or the cache can't be imported for whatever other reason, it warns and `docker build` continues by building the docker image.
This feature is opt-in. The documentation for the docker build cache of `type=local` can be found on https://docs.docker.com/build/cache/backends/local/
This replaces https://github.com/bitcoin/bitcoin/pull/31377 - some of the discussion there might provide more context.
ACKs for top commit:
maflcko:
I haven't tested this, and it looks harmless and is easy to revert, if needed. So lgtm ACK e87429a2d0
achow101:
ACK e87429a2d0
TheCharlatan:
tACK e87429a2d0
willcl-ark:
ACK e87429a2d0
Tree-SHA512: 0887c395dee2e2020394933246d4c1bfb6dde7165219cbe93eccfe01379e05c75dce8920b6edd7df07364c703fcee7be4fba8fa45fd0e0e89da9e24759f67a71
faaabfaea7 ci: Bump centos stream 10 (MarcoFalke)
Pull request description:
This is a follow-up to fa47baa03b, which bumped the gcc version to avoid a warning bloat in the CI log. However, it is also required to bump python3, see https://github.com/bitcoin/bitcoin/issues/31476#issue-2735206340
> This will uncover an issue in the centos task that the correct python version is missing. I guess this should be fixed by installing and activating an acceptable python version.
Instead of bumping the packages individually in centos stream 9, just bump to stream 10.
ACKs for top commit:
fanquake:
ACK faaabfaea7
Tree-SHA512: a564ff3a2a0dc4d39874e87540e67072f293bbed82c8eca22266fcadc16c5571e0e41d38576a63e466b64d13f7e3acbd95be10cf2420de33127aa420eca3b928
fabefd9915 ci: Turn CentOS task into native one (MarcoFalke)
Pull request description:
Cross-compiling to `i686-pc-linux-gnu` on CentOS in the CI is mostly redundant with the `ci/test/00_setup_env_i686_multiprocess.sh` task (albeit it using clang):
35bf426e02/ci/test/00_setup_env_i686_multiprocess.sh (L9-L12)
One task seems sufficient as a sanity check, given that there seems to be no real demand for this architecture anyway.
Turning the task into a native one makes it possible to run the task natively on aarch64 or any other supported architecture.
Also, remove the install of the `lbzip2` package, which is unused since commit a46065e36c
Also, remove the `CONFIG_SHELL` env var, which is unused since the cmake migration. (`CONFIG_SHELL` in depends is still kept).
ACKs for top commit:
davidgumberg:
ACK fabefd9915
hebasto:
ACK fabefd9915, tested locally on Ubuntu 24.10.
Tree-SHA512: 5a7b3131b379d11ef602e5821165861e9bdf61d605014bf8fcb33b8e12d8823450798af2d3289b96f7559dfa47b839bf939ddc0b3725efecfeac7ae570a981e7
According to the description for pkg-config, "pkgconf is a
replacement for pkg-config, providing additional functionality
while also maintaining compatibility. This package only provides
a dependency link to the pkgconf package to help with package
upgrades. It can be safely removed."
Thus several scripts and markdown files are updated.
For the task `MSan, depends (Cirrus CI)` we build a custom libc++ for
which we already use `-DLIBCXX_HARDENING_MODE=debug`. Compile it also
with `_LIBCPP_ABI_BOUNDED_*` to enable further checks.
Docs at: https://libcxx.llvm.org/Hardening.html#abi-options
By setting DANGER_DOCKER_BUILD_CACHE_HOST_DIR, the task-specific
docker images built during the CI run can be cached. This allows,
for example, ephemeral CI runners to reuse the docker images (or
layers of it) from earlier runs, by persisting the image cache
before the ephemeral CI runner is shut down. The cache keyed by
`CONTAINER_NAME`.
As --cache-to doesn't remove old cache files, the existing cache
is removed after a successful `docker build` and the newly cached
image is moved to it's location to avoid the cache from growing
indefinitly with old, unused layers.
When --cache-from doesn't find the directory, the cached version is
a cache-miss, or the cache can't be imported for whatever other reason,
it warns and `docker build` continues by building the docker image.
This feature is opt-in. The documentation for the cache type=local
can be found https://docs.docker.com/build/cache/backends/local/
This replaces https://github.com/bitcoin/bitcoin/pull/31377
This change switches to the latest IWYU 0.23, which is compatible with
Clang 19.
Fixed new "modernize-use-starts-ends-with" warnings.
The new "bugprone-use-after-move" warning in `result_tests.cpp` is a
false positive caused by a bug in Boost.Test versions < 1.87. This has
been addressed by introducing a local variable.
See upstream references:
- Issue: https://github.com/boostorg/test/issues/343
- Fix: https://github.com/boostorg/test/pull/348
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
fa5e706459 ci: Skip broken Wine64 tests by default (MarcoFalke)
Pull request description:
I don't think the unit tests run in Wine after the Windows cross-compilation have ever shown a true positive since the MSVC task was added. However, they are a source of frequent false-positives.
Thus, disable them by default for now. Anyone can still enable them by setting `RUN_UNIT_TESTS=true`.
A follow-up could run them on real Windows, see https://github.com/bitcoin/bitcoin/pull/31176.
Conceptually there are many other nightly tasks, which rarely find issues and are not run by default, like the valgrind or s390x tasks. So putting the Wine unit tests in the same bucket should be fine.
ACKs for top commit:
hebasto:
ACK fa5e706459, to avoid false-positives.
willcl-ark:
ACK fa5e706459
Tree-SHA512: 6bd54470e4d5ce18923c5d724aba0dbf475d053d7097d3f87e822a455cc537b6ce5f0dfcc8ccd0719c12c5d0c8fc7355a6c84185a6c9b5d484d98aee763d0c49
b031b7910d [ci] Split out native fuzz jobs for macOS and windows (dergoegge)
Pull request description:
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`.
ACKs for top commit:
maflcko:
re-lgtm ACK b031b7910d
achow101:
ACK b031b7910d
hebasto:
ACK b031b7910d.
Tree-SHA512: 7d0dc5a9cb299f6f4596dd9a5526b6aaf82efc6eba308bdc9d8b0a45f79dea87204fb6cd4b2ea2a1bd952466b2e958d64021999296d110d7a83c1667f4de51fe
