fa2c894cbb ci: move-only CI_CONTAINER_ID to 02_run_container.sh (MarcoFalke)
fa695b4df0 ci: Work around podman stop bug (MarcoFalke)
fa09a031c1 ci: Add set -ex to 02_run_container.sh (MarcoFalke)
fac9abbf47 ci: Rename 04_install to 02_run_container (MarcoFalke)
Pull request description:
Sometimes, it seems that `podman stop` does not work. Presumably, it falls back to `podman kill`, which is async.
Try to work around this intermittent issue by using the `rm --force` over `stop`.
Example failing log https://cirrus-ci.com/task/4549784611061760?logs=ci#L238:
```
Restart docker before run to stop and clear all containers started with --rm
++ podman container stop --all
e4eca0766f87864d89fc230aa884a238c214cfbcd44cf76a4dbdb2a30c982009
++ echo 'Prune all dangling images'
Prune all dangling images
++ docker image prune --force
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
+++ docker run --cap-add LINUX_IMMUTABLE --rm --interactive --detach --tty --mount type=bind,src=/tmp/cirrus-build-1970593815,dst=/tmp/cirrus-build-1970593815,readonly --mount type=volume,src=ci_macos_cross_ccache,dst=/tmp/ccache_dir --mount type=volume,src=ci_macos_cross_depends,dst=/ci_container_base/depends --mount type=volume,src=ci_macos_cross_previous_releases,dst=/ci_container_base/prev_releases --env-file /tmp/env --name ci_macos_cross ci_macos_cross
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
time="2023-09-27T20:55:39Z" level=warning msg="The input device is not a TTY. The --tty and --interactive flags might not work properly"
Error: creating container storage: the container name "ci_macos_cross" is already in use by e4eca0766f87864d89fc230aa884a238c214cfbcd44cf76a4dbdb2a30c982009. You have to remove that container to be able to reuse that name: that name is already in use
ACKs for top commit:
hebasto:
ACK fa2c894cbb, I have reviewed the code and tested it locally.
Tree-SHA512: 31fca340c6bedaadf4dd51fa745d9b3969042cebc0c7c904ef18af3f2f986039ec4354ccdff1422fbf77cf223e4423857368dce53cfa67ef15c76b78d007eace
78d3062b68 ci: Install Android API 31 platform as Qt expects (Hennadii Stepanov)
Pull request description:
When building the `qt` package, it expects that the default (in Qt's view) Android API platform is installed.
During the recent Qt version [update](https://github.com/bitcoin/bitcoin/pull/28561), it has been changed:
```diff
--- a/mkspecs/features/android/sdk.prf
+++ b/mkspecs/features/android/sdk.prf
@@ -1,6 +1,6 @@
API_VERSION_TO_USE = $$(ANDROID_API_VERSION)
isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = $$API_VERSION
-isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = android-28
+isEmpty(API_VERSION_TO_USE): API_VERSION_TO_USE = android-31
ANDROID_JAR_FILE = $$ANDROID_SDK_ROOT/platforms/$$API_VERSION_TO_USE/android.jar
!exists($$ANDROID_JAR_FILE) {
```
This PR fixes the CI for the Android task and addresses https://github.com/bitcoin/bitcoin/pull/28561#issuecomment-1749180177.
Qt [docs](https://doc.qt.io/qt-5/android.html) still claim that Android API Level 21 and up are supported, however, I did not test every possible configuration.
NOTE: https://github.com/bitcoin/bitcoin/pull/28611 is still valid.
ACKs for top commit:
maflcko:
lgtm ACK 78d3062b68
jarolrod:
tACK 78d3062b68
Tree-SHA512: 781fba6d80aae7e6500854de14af0d30169c258e395b9e482a5430a7b4a2211a6181f8c9ee58543c896b431abf09e3e7c5573b9672ed128658f11f98a2006e7e
fa3b5e5e57 ci: Use nproc over MAKEJOBS in 01_base_install (MarcoFalke)
Pull request description:
Currently `$MAKEJOBS` is the default value in `01_base_install.sh` when building the container image.
This problem can't be fixed (see below), so just use `nproc` for now.
Other solutions would be bad:
* Passing in the `MAKEJOBS` as a dockerfile env would create a new image if the number of tasks are changed, seems verbose and confusing.
* Leaving `master` as-is would leave CPUs unused if there are more than `4`.
ACKs for top commit:
fanquake:
ACK - I think this is fine as-is, it's an improvement over the current state fa3b5e5e57
Tree-SHA512: 15014eb7b548945737b0fed5cd46f0cd4b72b1d54d044f60fce628f914053a2de6518878428a54822d236d08d6f257d8c464d3fb589400f4be56022d2ec8676d
de8f9123af test: cover read-only blockstore (Matthew Zipkin)
5c2185b3b6 ci: enable chattr +i capability inside containers (Matthew Zipkin)
e573f24202 unit test: add coverage for BlockManager (Matthew Zipkin)
Pull request description:
This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782
ACKs for top commit:
jonatack:
ACK de8f9123af modulo suggestions in https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1319010039, tested on macOS, but not on Linux for the Linux-related change in the last push
achow101:
ACK de8f9123af
MarcoFalke:
lgtm ACK de8f9123af📶
Tree-SHA512: b9bd684035dcea11c901b649fc39f397a2155a9a8459f3348e67947e387e45312fddeccb52981aef486f8a31deebb5356a7901c1bb94b78f82c24192a369af73
fa07ac48d8 ci: Asan with -ftrivial-auto-var-init=pattern (MarcoFalke)
Pull request description:
This makes memory bugs deterministic. `-ftrivial-auto-var-init=pattern` is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them.
`-ftrivial-auto-var-init=pattern` goes well with `-fsanitize=bool` and `-fsanitize=enum`, but those are already enabled via `-fsanitize=undefined`. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks
ACKs for top commit:
fanquake:
ACK fa07ac48d8 - going to get back to fixing up the cxxflags usage in CI, but not a blocker here:
Tree-SHA512: 2ea6c5128a9cd262bdd1b1475c7e1be23ce2c520fad05f6c2aace6c29e203573323c0564d272e25da35ad5ff46fde488a5eae1ed14d3349e793d14a5e2533fb1
fa70cbd969 ci: Remove unused TEST_RUNNER_ENV="LC_ALL=C" from s390x task (MarcoFalke)
fa33354dcb ci: Remove /ro_base bind mount (MarcoFalke)
fa0df9d4c4 doc: Remove sudo from command that is already run as root (MarcoFalke)
Pull request description:
Remove some CI stuff no longer needed.
ACKs for top commit:
fanquake:
ACK fa70cbd969 - did not test the s390x job.
Tree-SHA512: 3a6ed0cfc855a92c2f834e59494c0a19a5647510247aece5e40a1aa78074894fe7454e684a1ea1f8f0662c50ac1caf2e390398b0fcfbf81544e6488fa9b8915e
fab7f5c01d ci: Add missing docker.io prefix to CI_IMAGE_NAME_TAG (MarcoFalke)
Pull request description:
Currently, the CI system may pick the wrong (non-native) architecture due to the missing prefix.
For example, assuming the CI_IMAGE_NAME_TAG is `debian:bookworm` and the user has previously pulled an s390x image:
```
$ podman run --rm 'docker.io/s390x/debian:bookworm' dpkg --print-architecture
exec /usr/bin/dpkg: exec format error
```
Now, `debian:bookworm` will refer to the same image:
```
$ podman run --rm 'debian:bookworm' dpkg --print-architecture
exec /usr/bin/dpkg: exec format error
```
However, `docker.io/debian:bookworm` works fine:
```
$ podman run --rm 'docker.io/debian:bookworm' dpkg --print-architecture
arm64
ACKs for top commit:
hebasto:
ACK fab7f5c01d.
Tree-SHA512: c423c5cd454a95fa3e67081411ca08d316b8c680a5bba49196c514b91df65d9cc46a47700cc00d9579327842615f98146d0ac50abb016616a9b17d042598dab6
fa56d17a4b ci: Add missing amd64 to win64-cross task (MarcoFalke)
Pull request description:
Currently the task will fail if run on non-`x86_64`.
Fix this by adding the missing `amd64`, similar to
7bf078f2b7/ci/test/00_setup_env_i686_multiprocess.sh (L11)
ACKs for top commit:
hebasto:
ACK fa56d17a4b
Tree-SHA512: faab1c5b945283b7e8d080bbcc8e9379c480cf6973506149ace5990cb4d04673f83f4bc36d08d5b4e9cb17a86fdbe23ac97ef4eab0e842616b367b8138229c58
This should fix the macOS-cross build on Cirrus CI containers.
Locally this was already working, because the SDK was cached in
/ci_container_base/ in the image, which is also the folder used for a
later CI run.
However, on Cirrus CI, when using an image *and* a custom BASE_ROOT_DIR,
the SDK will not be found in /ci_base_install/, nor in BASE_ROOT_DIR.
Fix this by normalizing *all* folders to /ci_container_base/.
9658d0dc17 ci: Run "macOS native x86_64" job on GitHub Actions (Hennadii Stepanov)
Pull request description:
From https://github.com/bitcoin/bitcoin/issues/28098:
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.
> If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.
---
**IMPORTANT NOTE**. We currently ship macOS release binaries for both architectures: `x86_64` and `arm64`. If this PR gets merged, only `x86_64` architecture will be tested on CI, which implies some [drawbacks](https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1658077549).
However, it has never been the case that our CI tested both architectures simultaneously. And we hope that GitHub Actions will soon host macOS `arm64` runners.
Historically, we moved from `x86_64` to `arm64` in https://github.com/bitcoin/bitcoin/pull/26388 less than a year ago.
---
Security concerns:
- https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1651432106
- https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1651688197
`GITHUB_TOKEN` permissions (from the build log in my personal repo):
```
2023-07-27T07:30:17.8313534Z ##[group]GITHUB_TOKEN Permissions
2023-07-27T07:30:17.8314113Z Contents: read
2023-07-27T07:30:17.8314608Z Metadata: read
2023-07-27T07:30:17.8314957Z Packages: read
2023-07-27T07:30:17.8315233Z ##[endgroup]
```
Comparison of resources:
| Resource | Current, Cirrus CI | Suggested, GitHub Actions |
|---|:-:|:-:|
| CPU | 4 | 4 \*\* |
| RAM, GB | 8 | 14 |
**\*\* NOTE**: However, [docs](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources) are mentioning:
> 3-core CPU (x86_64)
ACKs for top commit:
MarcoFalke:
re-ACK 9658d0dc17🏂
achow101:
ACK 9658d0dc17
jarolrod:
ACK 9658d0dc17
Tree-SHA512: 6123e68e6784cdf4e53c3e77b435709261db21f09091af2c22e667d3816a305fffb9d617297a5bc1bda18aaba84a6e210cec6a75c52afa7746a3780a67b69865
1c976c691c tidy: Integrate bicoin-tidy clang-tidy plugin (fanquake)
7de23cceb8 refactor: fix unterminated LogPrintf()s (fanquake)
0a1029aa29 lint: remove /* Continued */ markers from codebase (fanquake)
910007995d lint: remove lint-logs.py (fanquake)
d86a83d6b8 lint: drop DIR_IWYU global (fanquake)
Pull request description:
Demo of integrating the [bitcoin-tidy](https://github.com/theuni/bitcoin-tidy-plugin), [clang-tidy plugin](https://clang.llvm.org/extra/clang-tidy/) written by theuni into our tidy CI job.
The plugin currently has a single check, `bitcoin-unterminated-logprintf`. This would replace our current Python driven, `git-grep`-based, `.cpp` file only, lint-logs linter.
ACKs for top commit:
TheCharlatan:
ACK 1c976c691c
theuni:
ACK 1c976c691c
MarcoFalke:
re-ACK 1c976c691c👠
Tree-SHA512: 725b45c70e431d48e6f276671e05c694e10b6047cae1a31906ac3ee9093bc8105fb226b36a5bac6709557526ca6007222112d66aecec05a574434edc4897e4b8
This change aims to:
1) Remove our own `CCACHE_SIZE` environment variable that violates
Ccache's `CCACHE_*` namespace.
2) Introduce the `CCACHE_MAXSIZE` environment variable that is
documented since v3.3, which makes its usage consistent with other ones,
such as `CCACHE_DIR` and `CCACHE_NOHASHDIR`.
