f49840dd902cd9b14b6aadb431b16a4aeb719c3f doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc8fd426135809a8a4becdae2d797bb5 bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b89dc07017e9feaec1a8746a449a60b1 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a0294afbe045f1b17a77e4a3caf21e674 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc564b8b095099a28d1d5437951927c4 cmake: Move internal binaries from bin/ to libexec/ (Ryan Ofsky)
Pull request description:
This change moves binaries that are not typically invoked directly by users from the `bin/` directory to the `libexec/` directory in CMake installs and binary releases. The goal of the PR is to introduce a distinction between internal and external binaries so starting with #31802, we can use IPC to implement features in new binaries without adding those binaries to the CLI. The change also helps reduce clutter in `bin/`, making it easier for users to identify useful tools to run. Summary of changes:
- For **source builds** (i.e. developer builds) — There are no changes.
- For **source installs** (i.e. `cmake --install` result) — `test_bitcoin`, `test_bitcoin-qt`, and `bench_bitcoin` are installed in `${CMAKE_PREFIX_PATH}/libexec` instead of `${CMAKE_PREFIX_PATH}/bin`, so they are no longer on the system `PATH`. However, they can still be invoked from the `libexec/` directory, or from the CLI as `bitcoin test`, `bitcoin test-gui`, and `bitcoin bench`, respectively.
- For **binary releases** — Since `test_bitcoin` is the only test binary enabled in releases, the only change is moving `test_bitcoin` from `bin/` to `libexec/`.
<details><summary>Details</summary>
<p>
The table below shows the install location of each binary after this change, and the availability of each binary.
| Binary | Location | Availability | Change |
|----------------------|--------------|----------------------|-------------------------------|
| `bitcoin` | `bin/` | 📦 Binary release (since #31375) | Unchanged |
| `bitcoin-cli` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoind` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-qt` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-tx` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-util` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-wallet` | `bin/` | 📦 Binary release | Unchanged |
| `bench_bitcoin` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
| `bitcoin-chainstate` | `libexec/` | 🛠 Source build only | Newly installed (was built) |
| `bitcoin-gui` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `bitcoin-node` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `test_bitcoin` | `libexec/` | 📦 Binary release | Moved from `bin/` |
| `test_bitcoin-qt` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
</p>
</details>
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
l0rinc:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Sjors:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
achow101:
ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
janb84:
re ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
BrandonOdiwuor:
Tested ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
hodlinator:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
willcl-ark:
utACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
3333d3f75f8917cf8c183984e9b81e2d7a447ca5 ci: Only pass documented env vars (MarcoFalke)
Pull request description:
The CI currently inherits almost all env vars from the host. This was problematic in the past and causing non-determinism, e.g. the fix in commit fa12558d21aa03c22407a1458a0345d8a7ab6a4b. It is still problematic today, see e.g. https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586828644, or https://github.com/bitcoin/bitcoin/issues/32935
This fixes https://github.com/bitcoin/bitcoin/issues/32935 by only passing env vars documented in `./ci/test/00_setup_env.sh`.
Implementation-wise, instead of cramming the python code into the `python -c ""` statement, just start a fresh py file, which is easier to handle.
ACKs for top commit:
willcl-ark:
ACK 3333d3f75f8917cf8c183984e9b81e2d7a447ca5
Tree-SHA512: f922e481a844128d7fbf773563278a3992c178ead60a3050eceb9ded2aad979afc815a5cbdb9f68494493c5d8d942cdd1111c21e32a5746d19505b87745cb84a
7aa5b67132dfb71e915675a3dbcb806284e08197 ci: remove DEBUG_LOCKORDER from TSAN job (fanquake)
b09af2ce508185086bb551bfeb1409355c897e7b ci: instrument libc++ in TSAN job (fanquake)
6653cafd0b70b0e7a29c6cfe236d3bf9d1bce91e ci: allow libc++ instrumentation other than msan (fanquake)
Pull request description:
Allow for instrumenting libc++ with a sanitizer other than MemoryWithOrigins.
Would also close#33087, as with the extra instrumentation, the issue from https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3114706601 is avoided (also see https://github.com/bitcoin/bitcoin/pull/33081), and we can drop `DEBUG_LOCKORDER`.
ACKs for top commit:
maflcko:
re-ACK 7aa5b67132dfb71e915675a3dbcb806284e08197 🦀
dergoegge:
utACK 7aa5b67132dfb71e915675a3dbcb806284e08197
Tree-SHA512: 95f123e37da5e81d571244e4b1cd7658107676f1ea763ff16e5b69f4dfadb88236a577bb2ee52230ff542872c1da151c88fc50aba0f32540e765080120cec55e
fa1fd074685ca96b9bd3855e9e6fe730a4f6462c ci: Enable more shellcheck (MarcoFalke)
Pull request description:
shellcheck is often the main "reviewer" of CI code written in Bash, so it seems odd to disable it by putting commands into `bash -c "cmd..."`.
Fix that by removing `bash -c`, where it isn't intended and where the removal is easily possible.
ACKs for top commit:
hebasto:
ACK fa1fd074685ca96b9bd3855e9e6fe730a4f6462c.
Tree-SHA512: 6412dd3f8d702bca7762a8f1be3f9d2782132936fcc7ae5c31690b594e04f69708110e6f6233d5a61901289d13c7089ab5646a2c3ef2266fffc36d0543f4b7ae
fad191ff48b15832a90c19d560a7c0525c146be3 ci: Avoid cd into build dir (MarcoFalke)
Pull request description:
Changing into the build dir is confusing and brittle, because the following commands implicitly assume it. So they could break on unrelated changes.
The changes are required for stuff like:
* cmake presets (see https://github.com/bitcoin/bitcoin/pull/30871#issuecomment-2344031208)
* meta ci tests (like https://github.com/bitcoin/bitcoin/pull/32874)
So remove the `cd` and just make the build dir explicit.
ACKs for top commit:
hebasto:
ACK fad191ff48b15832a90c19d560a7c0525c146be3, I have reviewed the code and it looks OK.
Tree-SHA512: a88a9341445ffe28a0dac3815f235ec8eb0459d10a91a80829fd3184762d3c807d0f68c56243b20c04a6efa5becd8a7fad568f43c2b1e6af1ff8ba07b140ef87
Changing into the build dir is confusing and brittle.
This can be reviewed using the git option `--word-diff-regex=.`.
Also:
* add missing -j1 to the fallback that prints a verbose build failure
* remove quotes around $GOAL in the fallback
666016e56b28b77f798dc85c767b95c1ca0abfae ci: use --usecli in one of the CI jobs (Martin Zumsande)
7ea248a020997fcbbd6d62a4ec191aa858c463ca test: Disable several (sub)tests with cli (Martin Zumsande)
f420b6356b6f886638282a6aa9309b9982768775 test: skip subtests that check for wrong types with cli (Martin Zumsande)
6530d0015b958412825e1b8ae2aaefeee4372f08 test: add function to convert to json for height_or_hash params (Martin Zumsande)
54d28722baeac84950bbe6da1d315b9202012259 test: Don't send empty named args with cli (Martin Zumsande)
cca422060e96abbdca68a931fd45653738923caa test: convert tuple to json for cli (Martin Zumsande)
af34e980866e16970e0c4b837f56cd29038ae8bc test: make rpc_psbt.py usable with --usecli (Martin Zumsande)
8f8ce9e1740dfb5249b20d2bf759c23080367553 test: rename .rpc to ._rpc and remove unnecessary uses (Martin Zumsande)
5b088859863224a94514c78ea841d7314ec8fa50 test: enable functional tests with large rpc args for cli (Martin Zumsande)
7d5352ac7373e2beb9af671cef0a1a1e9ecb6658 test: use -stdin for large rpc commands (Martin Zumsande)
6c364e0c10de4d8036e696b3726bb0acffb94617 test: Enable various tests for usage with cli (Martin Zumsande)
Pull request description:
Fixes#32264
I looked into all current failures listed in the issue, as well all tests that are already disabled for the cli with `self.supports_cli = False`. There are several reasons why existing tests fail with `--usecli` on many systems, the most important ones are:
- Most common reason is that the test executes a RPC call with a large arg that exceeds `MAX_ARG_STRLEN` of the OS, which is usually 128kb on linux: This is fixed by using `-stdin` for these large calls (idea by 0xB10C)
- they test specifically the rpc interface - nothing to do there except disabling.
- Some functional test submit wrong types to params on purpose to test the error message (which is different when using the cli) - deactivated these specific subtests locally for the cli when there is just one or two of them, deactivated the entire tests when there are more spots
- When python sets `None` for an arg, the cli converts this to 'null' in `arg_to_cli`. This is fine e.g. for boolean args, but doesn't work for strings where it's interpreted as the string 'null'. Bypass this for named args by not including args in case the value is `None` for the cli is used (it's effectively the same as leaving the optional arg out).
- the `height_or_hash` param used in some RPC needs to be converted to a JSON (effectively adding full quotes).
- Some tests were marked with `self.supports_cli = False` in the past but run fine on master today - enabled those.
In total, this PR fixes all tests that fail on master and reduces the number of tests that are deactivated (`self.supports_cli = False`) from 40 to 21.
It also adds `--usecli` to one CI job (multiprocess, i686, DEBUG) to detect regressions.
ACKs for top commit:
maflcko:
re-ACK 666016e56b28b77f798dc85c767b95c1ca0abfae 🔀
pinheadmz:
re-ACK 666016e56b28b77f798dc85c767b95c1ca0abfae
Tree-SHA512: 7a1efd212649ca100b236a1239294d40ecd36e2720e3b173a230b14545bb40b135111db7fed8a0d1448120f5387da146a03f1912e2028c8d03a0b6a3ca8761b0
941b8f54c0d35d3243bb6083f3b52681d1b9a555 ci: run get_previous_releases as part of test cross win job (Max Edwards)
5e2182140bcd26afbe89b13d1d83ece6d5a89731 test: increment mocked time for migrating wallet backups (Max Edwards)
5174565802f426db1e9c8200a9421f82f61a5c99 ci: disable feature_unsupported_utxo_db functional test (Max Edwards)
3dc90d69a64f8bb39af27fa755683589b1bc76a7 test: remove mempool.dat before copying (Max Edwards)
67a6b20d5030ee51bf6d5e0fd77c9bdc8bafa96b test: add windows support to get previous releases script (Max Edwards)
1a1b478ca31be1f670754a47da17863271e46b7b scripted-diff: rename tarball to archive (Max Edwards)
4f06dc848460c887ad8337702ed900ba78725906 test: remove building from source from get prev releases script (Max Edwards)
Pull request description:
This PR updates the `test/get_previous_releases.py` script to also work on Windows by changing to be pure python rather than using unix tools such as `curl` and `tar`.
This enables additional functional tests to run such as `wallet_migration.py`, `mempool_compatability.py` and `wallet_backwards_compatibility.py`.
Unfortunately `feature_unsupported_utxo_db.py` _could_ run but this test requires Bitcoin `v0.14.3` which will not run under windows with emojis in the data directory (as the functional test runner has by default) . This test could be run as it's own step in the ci workflow file and would pass but as it's quite an old version / feature I have assumed it's not worth worrying about and best just to exclude.
Two tests needed to be slightly modified to run under windows. Both were issues with trying to overwrite a file that already exists which windows seems to be more strict on than the unix based systems.
Finally, building from source has been dropped from the `get_previous_releases.py` script. This had not been updated after the move to cmake and so it was assumed that nobody could have been using that feature.
ACKs for top commit:
maflcko:
re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555 🍪
achow101:
ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555
hodlinator:
re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555
Tree-SHA512: 22933d0ec278b9b0ffcd2a8e90026e1a3631b00186e7f78bd65be925049021e319367d488c36a82ab526a07b264bac18c2777f87ca1174b231ed49fed56d11cb
Using the get_previous_releases.py script to build from source only works for
releases prior to v29 due to removal of Autotools (in favor of CMake). It also
does not support building on Windows, and we are adding support for downloading
Windows release binaries in later commits of this PR.
As there were no complaints during review, it is assumed nobody uses this
functionality.
rsync --archive will preserve owner and group, which is then required to
be handled by adding a git safe.directory workaround.
Remove the need for the workaround by only preserving permissions during
the recursive rsync copy.
Some of the primary changes are:
- lief.EXE_FORMATS became lief.Binary.FORMATS IN 0.14.0
- 494f116c6b/doc/sphinx/changelog.rst (L702)
- lief.ARCHITECTURES became lief.Header.ARCHITECTURES in 0.16.0
- 494f116c6b/doc/sphinx/changelog.rst (L226C18-L227C18)
- lief.ELF.ARCH.x86_64 became lief.ELF.ARCH.X86_64
This commit includes a workaround for the bug fixed in
https://github.com/lief-project/LIEF/pull/1218, but the workaround can
be kept, since it makes `has_nx` checks stricter by enforcing both heap
and stack are non-executable.
This change also requires a patch to partially revert a commit to LIEF
(f23ced2f4f)
which broke compatibility with versions of scikit-build-core <= 0.10.x.
This patch can be dropped once the guix time machine advances to or
beyond 35c5f07e96,
which bumps the scikit-build-core version in guix from 0.9.3 to 0.10.7.
Co-authored-by: willcl-ark <will8clark@gmail.com>
Co-authored-by: fanquake <fanquake@gmail.com>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
This change moves binaries that are not typically invoked directly by users
from the `bin/` directory to the `libexec/` directory in CMake installs and
binary releases. The goal is to simplify the contents of `bin/` for end users
while still making all binaries available when needed. After this change, the
binaries remaining in `bin/` are:
- bitcoin
- bitcoin-cli
- bitcoind
- bitcoin-qt
- bitcoin-tx
- bitcoin-util
- bitcoin-wallet
And the binaries that are moved to `libexec/` are:
- bench_bitcoin
- bitcoin-chainstate(*)
- bitcoin-gui(***)
- bitcoin-node(***)
- test_bitcoin(**)
- test_bitcoin-qt
(*) bitcoin-chainstate was previously missing an install rule and was actually
not installed even when it was enabled.
(**) test_bitcoin is the only libexec/ binary that is currently included in
bitcoin binary releases. The others are only installed when building from
source with relevant cmake options enabled.
(***) bitcoin-node and bitcoin-gui are not currently built by default or
included in binary releases but both of these changes are planned and
implemented in #31802
a5ac43d98d1ad3ebed934f2c50208a85aae17e5e doc: Add release notes describing bitcoin wrapper executable (Ryan Ofsky)
258bda80c009a25d1f1bdeffccf9ed1ffde29cb2 doc: Mention bitcoin wrapper executable in documentation (Ryan Ofsky)
d2739d75c911c8bf73a4d3005c57add1ae4a67ae build: add bitcoin.exe to windows installer (Sjors Provoost)
ba649c00063a43b59a63db17b509179a658a8d9a ci: Run multiprocess tests through wrapper executable (Ryan Ofsky)
29bdd743bb843f8b8ed2e426b6df36e9d7e54215 test: Support BITCOIN_CMD environment variable (Ryan Ofsky)
9c8c68891b43053acfe7b8eb9d2e0d2bcfcb4e1e multiprocess: Add bitcoin wrapper executable (Ryan Ofsky)
5076d20fdb70a4bfafc4bdfe8293e347cb6bfa78 util: Add cross-platform ExecVp and GetExePath functions (Ryan Ofsky)
Pull request description:
Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don't cause confusion.
Idea and implementation of this were discussed in https://github.com/bitcoin/bitcoin/issues/30983.
---
Initial implementation of this feature is deliberately minimal so the UX can evolve in response to feedback and there are not too many details to debate and discuss in a single PR. But many improvements are possible or planned:
- Adding manpage and bash completions.
- Showing nicer error messages that detect if an executable isn't installed and suggest how to fix [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2073194474)
- Showing wrapper command lines in subcommand in help output [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2077800405). This could be done conditionally as suggested in the comment or be unconditional.
- Showing wrapper command lines in subcommand error output. There is a bitcoin-cli error pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243) that is needlessly confusing.
- Integrating help so `bitcoin help subcommand` invokes `bitcoin subcommand -h`. `bitcoin -h subcommand` should also be supported and be equivalent [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2093116725)
- Adding support for `bitcoin-util` subcommands. Ideal interface would probably be more like `bitcoin grind` not `bitcoin util grind` but this has been punted for now. Supporting subcommands directly would require some ArgsManager modifications
- Adding a dedicated python functional test for the wrapper. Right now there is some CI coverage by setting the `BITCOIN_CMD` variable, but this doesn't cover things like the help output and version output, and support for different directory layouts.
- Better `--multiprocess` (`-m`) / `--monolithic` (`-M`) default selection. Right now, default is monolithic but it probably makes sense to chose more intelligently depending on whether -ipc options are enabled and what binaries are available.
- Maybe parsing `bitcoin.conf` and supporting options to control wrapper behavior like custom locations or preferences or aliases.
- Better command command line usability. Allow combining short options like (`-ah`). Allow fuzzy matching of subcommands or suggestions if you misspell. (suggested by stickies in review club)
- Not directly related to this PR but `bitcoin-cli named` implementation used by the wrapper should do a better job disambiguating named arguments from base64 arguments ending in = as pointed out in [(comment)](https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091886628)
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722). A review club meeting for it took place in https://bitcoincore.reviews/31375
ACKs for top commit:
Sjors:
utACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
achow101:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
vasild:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
theStack:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
ismaelsadeeq:
fwiw my last review implied an ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
hodlinator:
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e
Tree-SHA512: 570e6a4ff8bd79ef6554da3d01f36c0a7c6d2dd7dace8f8732eca98f4a8bc2284474a9beadeba783114fe2f3dd08b2041b3da7753bae0b7f881ec50668cb821f
3a18075aedd7cff6f06b5fe10966d618b6378701 ci: Drop `-DENABLE_EXTERNAL_SIGNER=ON` configure option (Hennadii Stepanov)
719fa9f4ef6886c7c3be3b33d249b16f4e36a7e9 build: Re-enable external signer support for Windows (Hennadii Stepanov)
6e5fc2bf9b18cd492fe994dacb39182f601e9b86 test: Reintroduce Windows support in `system_tests/run_command` test (Hennadii Stepanov)
Pull request description:
This PR partially reverts:
- https://github.com/bitcoin/bitcoin/pull/28967
- https://github.com/bitcoin/bitcoin/pull/29489
After this PR, we can proceed to actually remove the [unused code](https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752) from `src/util/subprocess.h`.
ACKs for top commit:
Sjors:
ACK 3a18075aedd7cff6f06b5fe10966d618b6378701.
theStack:
Light ACK 3a18075aedd7cff6f06b5fe10966d618b6378701
laanwj:
Code review and lightly tested ACK 3a18075aedd7cff6f06b5fe10966d618b6378701
Tree-SHA512: 00d200685906e716750aae7cffa0794cca451653738ea590f50dfa28e1f3c5762a9be0ae0917aa0cf7436f00fe1e565236bff2853896530a5879466f7f45cb25
84de8c93e7d4979575161a2bb8f7eb64e1317b89 ci: Add `deploy` target for native macOS CI job (Hennadii Stepanov)
fad57e9e0fe2b57e9569aa26307c6344522c2a43 build: Fix `macdeployqtplus` after switching to Qt 6 (Hennadii Stepanov)
938208d91a27e3354f42eedbd6c5bf26117c4610 build: Resolve `@rpath` in `macdeployqtplus` (Hennadii Stepanov)
Pull request description:
Homebrew's Qt 6 package — namely `qt` or `qt@6` — introduces a few differences that must be properly handled by the `macdeployqtplus` script:
1. Use of `@rpath` references:
```
% objdump --macho --dylibs-used $(brew --prefix qt@5)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt@5/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt@5/lib/QtGui.framework/Versions/5/QtGui (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/Cellar/qt@5/5.15.16_1/lib/QtCore.framework/Versions/5/QtCore (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 65.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
% objdump --macho --dylibs-used $(brew --prefix qt@6)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt/lib/QtGui.framework/Versions/A/QtGui (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 170.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/opt/glib/lib/libglib-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtDBus.framework/Versions/A/QtDBus (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (compatibility version 61100.0.0, current version 61100.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/local/opt/freetype/lib/libfreetype.6.dylib (compatibility version 27.0.0, current version 27.2.0)
/usr/local/opt/glib/lib/libgthread-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtCore.framework/Versions/A/QtCore (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/UniformTypeIdentifiers.framework/Versions/A/UniformTypeIdentifiers (compatibility version 1.0.0, current version 709.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1226.0.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
```
2. Different directory layout:
```
% ls -l $(brew --prefix qt@5)/
total 544
drwxr-xr-x 79 hebasto admin 2528 13 Nov 06:22 Frameworks
-rw-r--r-- 1 hebasto admin 7533 16 Apr 09:09 INSTALL_RECEIPT.json
-rw-r--r-- 1 hebasto admin 22961 13 Nov 06:22 LICENSE.FDL
-rw-r--r-- 1 hebasto admin 36363 13 Nov 06:22 LICENSE.GPL3-EXCEPT
-rw-r--r-- 1 hebasto admin 15351 13 Nov 06:22 LICENSE.GPLv2
-rw-r--r-- 1 hebasto admin 35641 13 Nov 06:22 LICENSE.GPLv3
-rw-r--r-- 1 hebasto admin 26828 13 Nov 06:22 LICENSE.LGPLv21
-rw-r--r-- 1 hebasto admin 8174 13 Nov 06:22 LICENSE.LGPLv3
-rw-r--r-- 1 hebasto admin 106262 13 Nov 06:22 LICENSE.QT-LICENSE-AGREEMENT
-rw-r--r-- 1 hebasto admin 3842 13 Nov 06:22 README
drwxr-xr-x 57 hebasto admin 1824 16 Apr 09:09 bin
drwxr-xr-x 4 hebasto admin 128 13 Nov 06:22 doc
drwxr-xr-x 95 hebasto admin 3040 13 Nov 06:22 include
drwxr-xr-x 119 hebasto admin 3808 16 Apr 09:09 lib
drwxr-xr-x 8 hebasto admin 256 13 Nov 06:22 libexec
drwxr-xr-x 79 hebasto admin 2528 16 Apr 09:09 mkspecs
drwxr-xr-x 15 hebasto admin 480 13 Nov 06:22 phrasebooks
drwxr-xr-x 31 hebasto admin 992 13 Nov 06:22 plugins
drwxr-xr-x 28 hebasto admin 896 13 Nov 06:22 qml
-rw-r--r-- 1 hebasto admin 6952 16 Apr 09:09 sbom.spdx.json
drwxr-xr-x 3 hebasto admin 96 13 Nov 06:22 share
drwxr-xr-x 347 hebasto admin 11104 13 Nov 06:22 translations
% ls -l $(brew --prefix qt@6)/share/qt/
total 0
drwxr-xr-x 4 hebasto admin 128 30 Mar 09:49 doc
drwxr-xr-x 35 hebasto admin 1120 16 Apr 09:16 libexec
drwxr-xr-x 167 hebasto admin 5344 30 Mar 09:49 metatypes
drwxr-xr-x 70 hebasto admin 2240 16 Apr 09:16 mkspecs
drwxr-xr-x 178 hebasto admin 5696 30 Mar 09:49 modules
drwxr-xr-x 15 hebasto admin 480 30 Mar 09:49 phrasebooks
drwxr-xr-x 31 hebasto admin 992 30 Mar 09:49 plugins
drwxr-xr-x 34 hebasto admin 1088 30 Mar 09:49 qml
drwxr-xr-x 45 hebasto admin 1440 30 Mar 09:49 sbom
drwxr-xr-x 285 hebasto admin 9120 30 Mar 09:49 translations
```
This PR addresses both issues and additionally adds a `deploy` target to the native macOS CI job to prevent any similar recessions in the future.
Fixes https://github.com/bitcoin/bitcoin/issues/32267.
ACKs for top commit:
fanquake:
ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89
Tree-SHA512: 27a0eff3cd9317647529ff4571bd79c5dd8f007775b19415c8c27ca4912a60d85074c840cf0443be314d9a404f78bb015029d46dab18e292462249a5d90c6c47
faeb1babe283d8d61d830ce78310ce23984543e2 ci: refactor: Use version id over version codename consistently (MarcoFalke)
fae322a43a4b71b95b68f0674a5d2bd597fee68b ci: Slim down lint image (MarcoFalke)
3333273a8f74c72d00d7ef1cfe858410031fe60e 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 faeb1babe283d8d61d830ce78310ce23984543e2
janb84:
Re- ACK [faeb1ba](faeb1babe2)
Tree-SHA512: 39103e61ec2587096213bc1ce55b80087f6f03775592827d8c96a366453b798570d912690bf96fde4685798e5fc8ee2695ce851f473b4c8782d1a4c50c65a594
fa21f83d2983d97006ec1e3c47634dc0fe0349dc ci: Use G++ in valgrind tasks (MarcoFalke)
fabd05bf651138679f76728f974f141ac8ce99a9 refactor: Fix net_processing iwyu includes (MarcoFalke)
fa1622db208025e1744e78c4f5b135db11b293d4 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 fa21f83d2983d97006ec1e3c47634dc0fe0349dc
TheCharlatan:
ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
darosior:
utACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc
ryanofsky:
Code review ACK fa21f83d2983d97006ec1e3c47634dc0fe0349dc. 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
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.
