fae20e6b50306f91c74037e915aa0ab75a0a6b3b Revert "Avoid the use of P0083R3 std::set::merge" (MarcoFalke)
fab53b5fd45cf55a1d4d313e46ffce7396c9590e ci/doc: Set minimum required clang/libc++ version to 8.0 (MarcoFalke)
Pull request description:
This is not for 23.0, but for 24.0. It comes with the following benefits:
* Can use C++17 P0083R3 std::set::merge from libc++ 8.0
* No longer need to provide support for clang-7, which already fails to compile on some architectures (https://github.com/bitcoin/bitcoin/issues/21294#issuecomment-998098483)
This should be fine, given that all supported operating systems ship with at least clang-10:
* CentOS 8: clang-12
* Stretch: https://packages.debian.org/stretch/clang-11
* Buster: https://packages.debian.org/buster-backports/clang-11
* Bionic: https://packages.ubuntu.com/bionic-updates/clang-10
* Focal: https://packages.ubuntu.com/focal/clang-10
ACKs for top commit:
fanquake:
ACK fae20e6b50306f91c74037e915aa0ab75a0a6b3b - I think this is fine to do. I would be surprised if in another 6 months time someone was stuck on a system we supported, needing to compile Core, and only had access to Clang 7 or older. As mentioned in the PR description, all systems we currently support, already support multiple newer versions of Clang.
hebasto:
ACK fae20e6b50306f91c74037e915aa0ab75a0a6b3b, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3b4c6c130ff40dd7e84934af076863415e5dd661d823c72e3e3832566c65be6e877a7ef9164bbcf394bcea4b897fc29a48db0f231c22ace0e2c9b5638659a628
Self-compiled BDB was added to this job as opposed to using depends BDB
due to linking issues, however the compiled BDB is not actually used.
Remove it for now, given we don't actually lose any coverage (note that
BDB is also no used the MSAN fuzz job), and in future, we can use
depends BDB.
956f7322f60db7b8be551c9074b4c633e514079d build: Bump minimum Qt version to 5.11.3 (Hennadii Stepanov)
e22d10b936eb7563b2b6611332d9e4c73a2f59d4 ci: Switch from bionic to buster (Hennadii Stepanov)
Pull request description:
The current minimum Qt version is 5.9.5 which has been set in bitcoin/bitcoin#21286.
Distro support:
- centos 7 -- unsupported since bitcoin/bitcoin#23511
- centos 8 -- [5.15.2](http://mirror.centos.org/centos/8/AppStream/x86_64/os/Packages/qt5-qtbase-5.15.2-3.el8.x86_64.rpm)
- buster -- [5.11.3](https://packages.debian.org/buster/libqt5core5a)
- bullseye -- [5.15.2](https://packages.debian.org/bullseye/libqt5core5a)
- _bionic_ -- [5.9.5](https://packages.ubuntu.com/bionic/libqt5core5a)
- focal -- [5.12.8](https://packages.ubuntu.com/focal/libqt5core5a)
As another Ubuntu LTS is coming soon, it seems unreasonable to stick to Qt 5.9 which support [ended](https://www.qt.io/blog/2017/06/07/renewed-qt-support-services) on 2020-05-31. Anyway, it's still possible to build Bitcoin Core GUI with depends on bionic system.
Bumping the minimum Qt version allows to make code safer and more reliable, e.g.:
- functor-parameter overload of [`QMetaObject::invokeMethod`](https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-4)
- fixed https://bugreports.qt.io/browse/QTBUG-10907
An example of the patch using the functor-overload of `QMetaObject::invokeMethod`:
```diff
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -349,7 +349,7 @@ bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureStri
static void NotifyUnload(WalletModel* walletModel)
{
qDebug() << "NotifyUnload";
- bool invoked = QMetaObject::invokeMethod(walletModel, "unload");
+ bool invoked = QMetaObject::invokeMethod(walletModel, &WalletModel::unload);
assert(invoked);
}
```
It uses the same new syntax as signal-slot connection with compile-time check. Also see bitcoin/bitcoin#16348.
This PR is intended to be merged early [after](https://github.com/bitcoin/bitcoin/issues/22969) branching `23.x` off.
ACKs for top commit:
MarcoFalke:
cr ACK 956f7322f60db7b8be551c9074b4c633e514079d
fanquake:
ACK 956f7322f60db7b8be551c9074b4c633e514079d
Tree-SHA512: 3d652bcdcd990ce785ad412ed70234d4f27743895e535a53ed44b35d4afc3052e066c4c84f417e30bc53d0a3dd9ebed62444c57b7c765cb1e9aa687fbf866877
2c03cec2ff8cdbfd5da92bfb507d218e5c6435b0 ci: Build bitcoin-chainstate (Carl Dong)
095aa6ca37bf0bd5c5e221bab779978a99b2a34c build: Add example bitcoin-chainstate executable (Carl Dong)
Pull request description:
Part of: #24303
This PR introduces an example/demo `bitcoin-chainstate` executable using said library which can print out information about a datadir and take in new blocks on stdin.
Please read the commit messages for more details.
-----
#### You may ask: WTF?! Why is `index/*.cpp`, etc. being linked in?
This PR is meant only to capture the state of dependencies in our consensus engine as of right now. There are many things to decouple from consensus, which will be done in subsequent PRs. Listing the files out right now in `bitcoin_chainstate_SOURCES` is purely to give us a clear picture of the task at hand, it is **not** to say that these dependencies _belongs_ there in any way.
### TODO
1. Clean up `bitcoin-chainstate.cpp`
It is quite ugly, with a lot of comments I've left for myself, I should clean it up to the best of my abilities (the ugliness of our init/shutdown might be the upper bound on cleanliness here...)
ACKs for top commit:
ajtowns:
ACK 2c03cec2ff8cdbfd5da92bfb507d218e5c6435b0
ryanofsky:
Code review ACK 2c03cec2ff8cdbfd5da92bfb507d218e5c6435b0. Just rebase, comments, formatting change since last review
MarcoFalke:
re-ACK 2c03cec2ff8cdbfd5da92bfb507d218e5c6435b0 🏔
Tree-SHA512: 86e7fb5718caa577df8abc8288c754f4a590650d974df9d2f6476c87ed25c70f923c4db651c6963f33498fc7a3a31f6692b9a75cbc996bf4888c5dac2f34a13b
774323e378acea998069980edf50e1a26f4b6b9e ci: Force `--enable-external-signer` to prevent future regressions (Hennadii Stepanov)
69978858a4d8a9c34070f96133b8f95737a6505b build: Fix Boost.Process check for Boost 1.73 and older (Hennadii Stepanov)
2199ef79cba865af7949c3a55938ad54519ca5b8 build: Fix a non-portable use of `test` (Hennadii Stepanov)
d436c488d4216298016b09f8f97e10eada489bda build, refactor: Replace tabs with spaces (Hennadii Stepanov)
Pull request description:
On master (5f44c5c428b696af4214b2519cb2bbeb0e4a1027) Boost.Process check false fails without the `-lpthread` flag.
```
$ grep -C 2 pthread_detach config.log
/usr/bin/ld: /tmp/cczCQfQv.o: in function `boost::asio::detail::posix_global_impl<boost::asio::system_context>::~posix_global_impl()':
conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xa3): undefined reference to `pthread_join'
/usr/bin/ld: conftest.cpp:(.text._ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED2Ev[_ZN5boost4asio6detail17posix_global_implINS0_14system_contextEED5Ev]+0xc4): undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status
configure:26674: $? = 1
```
Not required for Boost 1.74+.
ACKs for top commit:
laanwj:
Code review ACK 774323e378acea998069980edf50e1a26f4b6b9e, is a bugfix/workaround, seems fine to merge last minute for 23.0.
Tree-SHA512: 2a9d4b67fd8910e107af972d8c223286b7c933bc310616f86c8b6d8c903438916980fc76bd7e37f2698f6ce5361dc706cbf2933d1ac2c081bcabe1b83ca7d6b6
fa27745ccbdf8df7949a2e79dba18de49dc89169 ci: Bump fuzz tasks to jammy (MarcoFalke)
fab8cd5f8764883dc48695c0e0916bae42f1cfdc Revert "ci: Run fuzzer task for the master branch only" (MarcoFalke)
Pull request description:
This reverts commit 5a9e255e5a324e7aa0b63a9634aa3cfda9a300bd.
I think we should attempt to maintain the fuzz tasks for release branches as well.
If it is too difficult for one branch, it could make sense to disable it for that branch, but not for all branches unconditionally.
Also, bump to jammy.
ACKs for top commit:
fanquake:
ACK fa27745ccbdf8df7949a2e79dba18de49dc89169 - we'll see how we go with the 23.x release branch.
Tree-SHA512: d6d08e7dce0884b556c51ff1896aebbbb5a805c22decd58af81a04192d19876978696017b489ec55886ddfd5c022963baaab5f11022369ae5291016826ff8017
a1515cdd9683942d733ff509b785cb3ca73c7a30 ci: use Ubuntu Jammy for Windows CI (fanquake)
Pull request description:
This means we'll compile using [GCC 10.3.x](https://packages.ubuntu.com/jammy/g++-mingw-w64) and [mingw-w64 8.0.0](https://packages.ubuntu.com/jammy/mingw-w64) which better matches our Guix release environment.
ACKs for top commit:
MarcoFalke:
cr ACK a1515cdd9683942d733ff509b785cb3ca73c7a30
hebasto:
ACK a1515cdd9683942d733ff509b785cb3ca73c7a30, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: a57cce1874324c9dd00e5d8989996d214facbdd561440471c15e6cc1808bca1c6fd758abe7a1b87378b2e7f9c25e7c9d8242df911cd1ef6cfbe49718adc3be5d
...to make sure that the linker errors that arise from coupling
regressions are caught by CI.
Adding to the "no wallet" ci job as suggested by MarcoFalke.
This change is a prerequisite for the following bumping Qt minimum
version to 5.11.3. It is required as bionic has Qt 5.9.5.
Effectively, this also changes:
- gcc from 8.4.0 to 8.3.0
- python from 3.6.5 to 3.7.3
1362d6173f863428de17c4017f4e3bb6883b0be8 scripted-diff: Insert missed copyright headers (Hennadii Stepanov)
f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 scripted-diff: Bump copyright headers (Hennadii Stepanov)
c29105efdc3daac0e8c43138f7c0a50535556455 script: Fix copyright_header.py (Hennadii Stepanov)
Pull request description:
This PR is an alternative to #23903.
It bumps the existing copyright headers as we did every year, and adds the missed copyright headers.
A small fix has been applied to the `copyright_header.py` in order to prevent such weird bumping as `2021` --> `2021-2017`.
ACKs for top commit:
MarcoFalke:
ACK 1362d6173f863428de17c4017f4e3bb6883b0be8
Tree-SHA512: 204d970fe8c51546b26b8f03fe4297db8a9bef5101df851540b7b9eddbd3a09677ee81fdd882c60937d732407f42c9883165bd978272200cff8f90190f075905
Our minimum required GCC is GCC 8, and this change in required for
changes like #23839 which take advantage of flags introduced in that
version of GCC.
This should have been included as part of
182de7ba10811ec39e24ec5bec7cd2119f776f2f.
2f356a0ca8b080c860c8924d201b98f1f9fa0ad5 scripted-diff: Drop Darwin version for better maintainability (Hennadii Stepanov)
Pull request description:
After this PR, any macOS tools version bumping in the future will touch fewer files in the repo.
Pointing a Darwin version for the `--host` system does not matter for the following reasons:
- in terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system.
- in terms of the build system itself, the usage of the `$(host)` variable is self-consistent enough. Btw `$(host_os)` value already has the version dropped:
```
$ make -C depends --no-print-directory print-host_os HOST=x86_64-apple-darwin19
host_os=darwin
```
ACKs for top commit:
gruve-p:
ACK 2f356a0ca8
promag:
ACK 2f356a0ca8b080c860c8924d201b98f1f9fa0ad5.
fanquake:
ACK 2f356a0ca8b080c860c8924d201b98f1f9fa0ad5
Tree-SHA512: 374896ab0ba02b0d8b4b21431fe963bd213b0d09586e0898c13a4c5fa294c1b693f1b2c92880c245c4157c14217b4825b36522f461930477f4d2a727086ebb2a
