Age | Commit message (Collapse) | Author |
|
This is needed for the next commit.
This also requires dropping CI_RETRY from the docker build step, which
is fine, because CI_RETRY should be called inside the build script, not
outside.
Also, fix a doc typo.
|
|
The --workdir setting to the docker run command is not needed. And
P_CI_DIR/PWD is equal to BASE_ROOT_DIR, so just use that directly.
|
|
`noreturn` attributes have been added to the mingw-w64 headers, meaning
that from 11.0.0 onwards, you'll no-longer see `-Wreturn-type` warnings
when using assert(false):
https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe.
Add -Wno-return-type to the Windows CI, where is should have been all
along, and document why it's required. This can be dropped when we are
using the fixed version of the mingw-w64 headers there.
Drop the -Werror -Wno-return-type special case from our build system.
-Wreturn-type is on by default in Clang and GCC.
|
|
|
|
|
|
These work for me now. If they still don't work in other setups,
maybe we can better document the issues.
```bash
time FILE_ENV="./ci/test/00_setup_env_s390x.sh" ./ci/test_run_all.sh
...
Running tests: coins_tests from test/coins_tests.cpp
PASS: qt/test/test_bitcoin-qt
Running tests: coinstatsindex_tests from test/coinstatsindex_tests.cpp
...
Stop and remove CI container by ID
+ docker container kill 617bef8accb87530e5fbb03ff07b3b9f0aa9e3030d4da424c9612d153ab98dbf
617bef8accb87530e5fbb03ff07b3b9f0aa9e3030d4da424c9612d153ab98dbf
real 51m37.809s
```
|
|
62633b50461cb67dfb37d6485e604152e727559c ci: filter all subtrees from tidy output (fanquake)
Pull request description:
We are currently dumping output for some. i.e:
```bash
diff --git a/src/minisketch/src/fields/clmul_1byte.cpp b/src/minisketch/src/fields/clmul_1byte.cpp
index 8826af9..7fd6f2a 100644
--- a/src/minisketch/src/fields/clmul_1byte.cpp
+++ b/src/minisketch/src/fields/clmul_1byte.cpp
@@ -4,21 +4,16 @@
* file LICENSE or http://www.opensource.org/licenses/mit-license.php.*
**********************************************************************/
-/* This file was substantially auto-generated by doc/gen_params.sage. */
-#include "../fielddefines.h"
-
+class Sketch;
#if defined(ENABLE_FIELD_BYTES_INT_1)
```
ACKs for top commit:
hebasto:
re-ACK 62633b50461cb67dfb37d6485e604152e727559c
Tree-SHA512: fd0a17af6b37fc7641547dab329c2d14ec784941c4d100db1e80d232aff39e45ad9c588982810a2cfc54b4fe820bfe0d50638b53209fec6774fd556b9b0ae180
|
|
fae7c50d201726f605938c3511dd9119efeea5ec test: Run fuzz tests on macOS (MarcoFalke)
Pull request description:
Any reason not to?
ACKs for top commit:
jamesob:
Github ACK https://github.com/bitcoin/bitcoin/pull/27932/commits/fae7c50d201726f605938c3511dd9119efeea5ec
dergoegge:
utACK fae7c50d201726f605938c3511dd9119efeea5ec
Tree-SHA512: e45122d73fafb17cea312258314b826cb0745e08daadd28465f687ec02d4c127d2f8cbe20179a9fff5712038850c02c968abb4838fa088b7555e28709317d3a3
|
|
We are currently dumping output for some. i.e:
```bash
diff --git a/src/minisketch/src/fields/clmul_1byte.cpp b/src/minisketch/src/fields/clmul_1byte.cpp
index 8826af9..7fd6f2a 100644
--- a/src/minisketch/src/fields/clmul_1byte.cpp
+++ b/src/minisketch/src/fields/clmul_1byte.cpp
@@ -4,21 +4,16 @@
* file LICENSE or http://www.opensource.org/licenses/mit-license.php.*
**********************************************************************/
-/* This file was substantially auto-generated by doc/gen_params.sage. */
-#include "../fielddefines.h"
-
+class Sketch;
#if defined(ENABLE_FIELD_BYTES_INT_1)
```
|
|
|
|
|
|
32e2ffc39374f61bb2435da507f285459985df9e Remove the syscall sandbox (fanquake)
Pull request description:
After initially being merged in #20487, it's no-longer clear that an internal syscall sandboxing mechanism is something that Bitcoin Core should have/maintain, especially when compared to better maintained/supported alterantives, i.e [firejail](https://github.com/netblue30/firejail).
There is more related discussion in #24771.
Note that given where it's used, the sandbox also gets dragged into the kernel.
If it's removed, this should not require any sort of deprecation, as this was only ever an opt-in, experimental feature.
Closes #24771.
ACKs for top commit:
davidgumberg:
crACK https://github.com/bitcoin/bitcoin/pull/27896/commits/32e2ffc39374f61bb2435da507f285459985df9e
achow101:
ACK 32e2ffc39374f61bb2435da507f285459985df9e
dergoegge:
ACK 32e2ffc39374f61bb2435da507f285459985df9e
Tree-SHA512: 8cf71c5623bb642cb515531d4a2545d806e503b9d57bfc15a996597632b06103d60d985fd7f843a3c1da6528bc38d0298d6b8bcf0be6f851795a8040d71faf16
|
|
Also, fix a few bugs:
* Error: RPC command "enumeratesigners" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp.
* in run_once: ...format(" ".join(result.args), ... TypeError: sequence item 2: expected str instance, PosixPath found
|
|
cbee1d70918b7c8e524c07f3da7049c3a1a2cbff depends: modernize clang flags (Cory Fields)
2a85857ce5cddd365353216960e2d5d76d6102b8 ci: disable false-positive warnings for now (Cory Fields)
Pull request description:
This is a cleaner and simpler alternative to #25098. Inspired by [this conversation](https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1562543301). The diff is large but the change itself is quite small.
Fixes builds with llvm >= 11 in guix by working around the problem. As a bonus, this is much cleaner and more maintainable than what we had before.
See the updated comment for more info. At a high level: rather than playing tricks and trying to work around clang's default includes, disable them and re-add what we want.
ACKs for top commit:
fanquake:
ACK cbee1d70918b7c8e524c07f3da7049c3a1a2cbff - tested Guix and the depends cross-compile. Would like to move this along, to unblock #27676, which itself might be a blocker for #27897. Note that macOS might seem somewhat in flux for the moment, but once we finish the migration to LLVM Clang + LLD, things will be must simpler, and ultimately more maintainable.
TheCharlatan:
ACK cbee1d70918b7c8e524c07f3da7049c3a1a2cbff
Tree-SHA512: 5a8300be528f550e15ab23d869e77df7a62201c6d40c0384795a9eecee38118a676e0b79b2b76c5e597597181443caada54a01b75a544dbcde76da1deba8e3a4
|
|
0000f552937ee787d25c8fd0af3278ea94889216 ci: Run fuzz target even if input folder is empty (MarcoFalke)
Pull request description:
This should catch trivial integer sanitizer bugs if the author and all reviewers forget to look for them.
ACKs for top commit:
brunoerg:
reACK 0000f552937ee787d25c8fd0af3278ea94889216
dergoegge:
reACK 0000f552937ee787d25c8fd0af3278ea94889216
Tree-SHA512: f139b9d56f0cf1aae339c2890721c77c88d1fea77b73d492c1386ec99b4f393c5b664029919ff4a22e4e8a2929f085699a148c6acc2cc3e40df8a72fd39ff474
|
|
clang <=17 warns on -nostdlibinc, which causes an error on our -Werror builds.
Note that this breaks the "-fPIE" check in configure because it relies on
catching warnings, but that is not a problem for macOS.
|
|
|
|
|
|
|
|
After initially being merged in #20487, it's no-longer clear that an
internal syscall sandboxing mechanism is something that Bitcoin Core
should have/maintain, especially when compared to better
maintained/supported alterantives, i.e firejail.
Note that given where it's used, the sandbox also gets dragged into the
kernel.
There is some related discussion in #24771.
This should not require any sort of deprecation, as this was only ever
an opt-in, experimental feature.
Closes #24771.
|
|
|
|
|
|
This should avoid a race where the kill is not done when spinning up the
new container. podman stop waits 10 seconds by default.
|
|
|
|
Use Native.
|
|
|
|
5763b232e6e6a0f72d046f8aa322b39328be135b ci: return to using Ubuntu 22.04 in MSAN jobs (fanquake)
d3cbcbf62693ad7394b3f8693b1c08d4271903fa ci: compile clang and compiler-rt in MSAN jobs (fanquake)
796bd1d0d147ac90f921ce3831961f97748d4e1a ci: use LLVM 16.0.4 in MSAN jobs (fanquake)
883bc9f5611648c44956a09795afd924842c1d1d ci: remove extra CC & CXX from MSAN jobs (fanquake)
2d4f4b8f29c015c26cb02b26a517450bb6056ed4 ci: standardize custom libc++ usage in MSAN jobs (fanquake)
Pull request description:
This reworks the MSAN CIs, to first compile Clang and compiler-rt (using GCC 12), and then, compile an MSAN instrumented libc++ using the just-built Clang 16. This fixes the `native_fuzz_with_msan` job, working around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, by not using the Debian provided Clang/LLVM.
Also included are changes to streamline how we use our "custom libc++", according to upstream: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc, as well as other minor cleanups in the CI configs.
An example job is currently running in the qa-assets repo: https://github.com/bitcoin-core/qa-assets/pull/129 (https://cirrus-ci.com/task/4632561431871488).
ACKs for top commit:
dergoegge:
utACK 5763b232e6e6a0f72d046f8aa322b39328be135b
Tree-SHA512: 4f2a6e0b796bb1830b8346dd1e55eaa86a79037b8b4f16a336c1e29f4fc460acca2ecba076635459370bcbb4009333cb79d27ef1521c1fb5db7599cd5bdf558c
|
|
fa3ab4520317f48d4700b81dab023c4e639bbd68 ci: Enable float-divide-by-zero check (MarcoFalke)
Pull request description:
Enable it, because
* It is enabled on OSS-Fuzz, so to be able to catch bugs earlier, enable it here as well.
* It makes sense to enable, because when a float is divided by zero, it may be a logic bug in our code, so it should be suppressed in the suppressions file.
ACKs for top commit:
willcl-ark:
utACK fa3ab4520317f48d4700b81dab023c4e639bbd68
dergoegge:
ACK fa3ab4520317f48d4700b81dab023c4e639bbd68
Tree-SHA512: 2c2c025af4fe3ec267b3cfa38f25495e9da678cf6c529a6438ec923ef09a06ad37fa4503c30cbacc83578ac2856a7f729ef70a24befffd61d10ec075132d1ee0
|
|
RESTART_CI_DOCKER_BEFORE_RUN
fa123077bc3f39aa0969d883e2d799a054cd4543 ci: Use podman for persistent workers (MarcoFalke)
fa9c65a74cf18e9c75cd3472112d5197532ac2f2 ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN (MarcoFalke)
Pull request description:
This should prevent the persistent workers from running out of disk space. Containers are already removed, but not images. This is required since CI images are built and cached.
ACKs for top commit:
hebasto:
ACK fa123077bc3f39aa0969d883e2d799a054cd4543
Tree-SHA512: 07c4faec57d659d1762e4e6d776c882ee48d4bac6ce6d438d56d9ab13277be3e39d6aa38816165a5a3e0938ac5d47674ee2921b6e115a4bb54e3e4910b34c4b6
|
|
|
|
|
|
|
|
We no-longer need to use 23.04, now that we aren't installing clang-16
and friends.
|
|
This works around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341.
|
|
|
|
This is passed through from depends.
|
|
Use `-isystem` & `-nostd*` flags, which is the preferred way to use a
custom libc++ (ours is libc++ build with MSAN) with Clang, as opposed to
our current ad-hoc flags.
See: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc
for more info.
|
|
debug mode
59c89447499bd9d6202269879555b8bc37373aa2 build: disable boost multi index safe mode (willcl-ark)
Pull request description:
Fixes #27586
Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 and the [boost docs](https://www.boost.org/doc/libs/1_58_0/boost/multi_index/detail/safe_mode.hpp) for more information.
Re-enable it on the CI builds which previously had it enabled.
Re-enable it on the msan fuzz task so that we have fuzz tasks testing
with it enabled and disabled in this repo.
ACKs for top commit:
hebasto:
~ACK 59c89447499bd9d6202269879555b8bc37373aa2~
fanquake:
ACK 59c89447499bd9d6202269879555b8bc37373aa2
Tree-SHA512: ed654f63dbebdd02e4414d1f81147d92a4d490dbb5a2e0376858e3129097645f3a2df45191d6b40c410a76e803b0d28796d1a01c1d2fd995b94e8b7eb3949027
|
|
1f97572b9c0d339a8497340e7066050aba9d7694 Fix `#include`s in `src/wallet` (Hennadii Stepanov)
Pull request description:
This PR is a minimum required changes to fix https://github.com/bitcoin/bitcoin/pull/27571#discussion_r1195497290.
ACKs for top commit:
MarcoFalke:
lgtm ACK 1f97572b9c0d339a8497340e7066050aba9d7694
Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
|
|
This will lead to a duplicate install, see https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564122573
|
|
It is unclear what the point is of maintaining a "default", the meaning
of which is unclear.
|
|
Also, set -x for easier debugging.
Also, do the same for ci/test/00_setup_env.sh
|
|
|
|
Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 for more information.
Re-enable it on the CI builds which previously had it enabled.
Re-enable it on the msan fuzz target so that we have fuzz tasks testing
with it enabeld and disabled in this repo.
|
|
fa1b3abc834a90fb1cfbd5ac63deb28f3990c1fb ci: Log qa-assets repo last commit (MarcoFalke)
fa22966f3307e22b77ea386e1abb60bf8c606170 fuzz: Print error message when FUZZ is missing (MarcoFalke)
Pull request description:
Some trivial UX improvements.
* Change the exit code for `PRINT_ALL_FUZZ_TARGETS_AND_ABORT` and `WRITE_ALL_FUZZ_TARGETS_AND_ABORT` to `EXIT_SUCCESS` instead of `Aborted (core dumped)`.
* Print readable error message when `FUZZ` is missing instead of `Aborted (core dumped)`.
* Clarify that a fuzz target needs to be compiled into the executable.
ACKs for top commit:
dergoegge:
ACK fa1b3abc834a90fb1cfbd5ac63deb28f3990c1fb
Tree-SHA512: 065ef8920449c64b3516f89a61cb397b505eccf531318c4f3830895d5ff6cd7ae2525cb857320481e3d0ed0b2f8a522cd8f7835e69f021241b6ec297a6102fc8
|
|
5228223e1ff2af29e6e77668ce3288005c2adbbc ci: remove MSAN getrandom syscall workaround (fanquake)
d5e06919db5e221bfef445c5a40c88de72dc5869 random: switch to using getrandom() directly (fanquake)
c2ba3f5b0c7d0eece7d16d1ffc125d8a6a9297af random: add [[maybe_unused]] to GetDevURandom (fanquake)
c13c97dbf846cf0e6a5581ac414ef96a215b0dc6 random: getentropy on macOS does not need unistd.h (fanquake)
Pull request description:
This requires a linux kernel of `3.17`+, which seems entirely
reasonable. `3.17` went EOL in 2015, and the last supported `3.x` kernel
(`3.16`) went EOL > 4 years ago, in 2020. For reference, the current
oldest maintained kernel is `4.14` (released 2017, going EOL Jan 2024).
Support for `getrandom()` (and `getentropy()`) was added to
glibc `2.25` https://sourceware.org/legacy-ml/libc-alpha/2017-02/msg00079.html:
> * The getentropy and getrandom functions, and the <sys/random.h> header
file have been added.
and we already require `2.27` or later.
All that being said, I don't think you would encounter a current day (+~6 months from now)
system, running with kernel headers older than 3.17 (released 2014) but also having a
glibc of 2.27+ (released 2018)?
Removing this (our only) use of `syscall()` also means we can drop a workaround in our MSAN jobs.
If this is merged, I'll drop the [same workaround in oss-fuzz](https://github.com/google/oss-fuzz/blob/25946a544856413d31d9cbb3a366a4aef5a8fd60/projects/bitcoin-core/build.sh#L49-L56).
ACKs for top commit:
josibake:
ACK https://github.com/bitcoin/bitcoin/pull/27699/commits/5228223e1ff2af29e6e77668ce3288005c2adbbc
hebasto:
ACK 5228223e1ff2af29e6e77668ce3288005c2adbbc, I've tested build system changes on Ubuntu 22.04 and macOS Monterey 12.6.6 (x86_64).
Tree-SHA512: cc978e08510c461b875ca8c08ae176b4519fa1108f0efd74dcb7474518945357e0184e54423282c9a496de195e4ddc3e221ee78623bd63e24c50cc86acdf32e2
|
|
6a936580d1c42576f627d5fac5423ec7af88e547 ci: remove RUN_SECURITY_TESTS (fanquake)
Pull request description:
We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e #26953.
ACKs for top commit:
josibake:
code review ACK https://github.com/bitcoin/bitcoin/pull/27683/commits/6a936580d1c42576f627d5fac5423ec7af88e547
TheCharlatan:
ACK 6a936580d1c42576f627d5fac5423ec7af88e547
Tree-SHA512: c0eec61a4b873bac487ba9321b50116a215b4796bd7d416d98ffcd09969dbf635c2cb5aeb225c89d1e6462838fa2a48565048ebe730f48d76d3db46b64855a91
|
|
This documents the state in the CI output and may help debugging in case
of failure.
|
|
The corresponding workaround will also be dropped in oss-fuzz:
https://github.com/google/oss-fuzz/blob/25946a544856413d31d9cbb3a366a4aef5a8fd60/projects/bitcoin-core/build.sh#L49.
|
|
|