f78cc90524 ci: Fix merge_script in MSVC task (Hennadii Stepanov)
Pull request description:
The new `merge_script` in the MSVC build task does not really exit early when the task is triggered by a non-pr.
In the current code e4aa9b15b9/.cirrus.yml (L104)
the `exit 0` command exits from the PowerShell call, not the recent `merge_script`. This cause the next lines e4aa9b15b9/.cirrus.yml (L105-L107) are executed unconditionally.
Here is an excerpt from [CI task log](https://api.cirrus-ci.com/v1/task/4578647416766464/logs/merge.log) for the ["Merge #22915: Remove confusing CAddrDB " commit](896649996b):
```
...
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIAByAGUAcwBlAHQAIAAtAC0AaABhAHIAZAA=
HEAD is now at 896649996 Merge bitcoin/bitcoin#22915: Remove confusing CAddrDB
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand aQBmACAAKAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBQAFIAIAAtAGUAcQAgACQAbgB1AGwAbAApACAAewAgAGUAeABpAHQAIAAwADsAIAB9AA==
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABmAGUAdABjAGgAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBSAEUAUABPAF8AQwBMAE8ATgBFAF8AVQBSAEwAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBCAEEAUwBFAF8AQgBSAEEATgBDAEgA
From https://github.com/bitcoin/bitcoin
* branch HEAD -> FETCH_HEAD
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABtAGUAcgBnAGUAIABGAEUAVABDAEgAXwBIAEUAQQBEAA==
Already up to date.
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0
```
This PR fixes this issue, and makes `merge_script` log more readable.
ACKs for top commit:
MarcoFalke:
Concept ACK f78cc90524
Tree-SHA512: c88b115f99f9019a4100a10df051e32c05487612c13105d10873b9cf38965eeca731604d36610ae750cb1f93ba77ce97dca7599fe4984181210d0753be4eb9a0
Init should only concern itself with the initialization order, not the
detailed initialization logic of every module.
Also, inlining logic into a method that is ~800 lines of code, makes it
impossible to unit test on its own.
fa0c194db3 cirrus: Enable tests on windows (MarcoFalke)
fadecbd9a4 test: Fix tests on Windows (MarcoFalke)
Pull request description:
Only a cherry-picked list. `--extended` can be enabled in a follow-up.
ACKs for top commit:
hebasto:
ACK fa0c194db3, tested locally on Windows 10 Pro 20H2 (build 19042.1165):
Tree-SHA512: 47cfbcef7ce5fe0c62b77a1e45ace513c4f0109b1fcfaec94faf9e488fe9430d6ba0e852230021d432847eb1389a4e4b7cf39bf17f7b09bae36af3079e0d7399
fade9a1a4d Remove confusing CAddrDB (MarcoFalke)
fa7f77b7d1 Fix addrdb includes (MarcoFalke)
fa3f5d0dae Move addrman includes from .h to .cpp (MarcoFalke)
Pull request description:
Split out from #22762 to avoid having to carry it around in (an)other rebase(s)
ACKs for top commit:
ryanofsky:
Code review ACK fade9a1a4d
lsilva01:
Code Review ACK fade9a1a4d
Tree-SHA512: 7615fb0b6235d0c1e6f8cd6263dd18c4d95890567c2b797fe6fce6cb12cc85ce6eacbe07dbb6d81b05d179ef03b42edfd61c940e35a1044ce6d363b54c2dae5c
fdd71448e7 system: skip trying to set the locale on NetBSD (fanquake)
Pull request description:
Just treat it the same as the other BSDs.
Fixes#17379.
ACKs for top commit:
laanwj:
Code review ACK fdd71448e7
practicalswift:
cr ACK fdd71448e7
Tree-SHA512: 5fe0a66f014279ad2683b548692a36af493377fb92d1f28b15dc4feef871190fe08ef40dcc4f5ba21a525fe365c42fb429fe4be0673a1e96db163af587c23204
fa57fa1a2e Enable clang-tidy bugprone-argument-comment and fix violations (MarcoFalke)
Pull request description:
Named arguments can be dangerous when they are wrong, because they are not enforced by the compiler. Currently there are only minor typos, no actual bugs.
Fix the typos and add the `.clang-tidy` file to make it easier to find them in the future.
ACKs for top commit:
practicalswift:
cr ACK fa57fa1a2e
fanquake:
ACK fa57fa1a2e
Tree-SHA512: b66f01e0a1e77e56ed8454002176df660cc2cc0947a90785aa33cc5b8003a1f99fd8b2f8f89f2a0bf180ff2c42c031d69e669d127bb557b879c17975275a220b
69a439b880 doc: add missing copyright header to getuniquepath.cpp (fanquake)
Pull request description:
This was missed in #21052.
ACKs for top commit:
hebasto:
ACK 69a439b880, but a scripted-diff using `copyright_header.py insert` looks preferable.
Tree-SHA512: 1a75ffd93fa8e5e83821e1fe984353422740ebf9e203dc873debf4dffec0e23f55a1e31f806dd2d66087d3620f6dc447af69f9124f0bcc6676ef91ee35374832
fab0b55cf0 addrman: Fix format string in deserialize error (MarcoFalke)
facce4ca44 test: Remove useless overwrite (MarcoFalke)
Pull request description:
The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later).
Work around the behaviour change in compilers by pinning the underlying type of the format arguments.
Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later).
ACKs for top commit:
jonatack:
ACK fab0b55cf0 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected
mzumsande:
ACK fab0b55cf0
Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609
This change allows users to not patch `common.qt.init.vcxproj` to fit
their Qt and Visual Studio versions. Also it simplifies code base
maintaining.
To specify a non-default path, the QTBASEDIR environment variable can be
used.
DecopeAsmap is a pure utility function and doesn't have any
dependencies on addrman, so move it to util/asmap.
Reviewer hint: use:
`git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
The class only stores the file path, reading it from a global. Globals
are confusing and make testing harder.
The method reading from a stream does not even use any class members, so
putting it in a class is also confusing.
5fabde6fad wallet: AddWalletDescriptor requires cs_wallet lock (João Barbosa)
32d036e8da wallet: GetLabelAddresses requires cs_wallet lock (João Barbosa)
Pull request description:
This is another small change towards non recursive wallet lock.
ACKs for top commit:
hebasto:
ACK 5fabde6fad, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 00506f0159c56854a171e58a451db8dd9b9f735039697b1cf2ca7f54de61fb51cc1e5eff42265233e041b4b1bfd29c2247496dc4456578e1a23c323bdec2901b
d319c4dae9 qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addView (Hennadii Stepanov)
92ddc02a16 qt, refactor: Declare getWalletModel with const and noexcept qualifiers (Hennadii Stepanov)
ca0e680bdc qt, refactor: Drop redundant checks of walletModel (Hennadii Stepanov)
404373bc6a qt, refactor: Pass WalletModel object to WalletView constructor (Hennadii Stepanov)
Pull request description:
An instance of the `WalletView` class without the `walletModel` data member being set is invalid. So, it is better to set it in the constructor.
Establishing one more `WalletView` class's invariant in constructor:
- allows to drop all of checks of the`walletModel` in member functions
- makes reasoning about the code that uses instances of the `WalletView` class easier
Possible follow ups could extend this approach to other classes, e.g., `OverviewPage`, `TransactionView`, `ReceiveCoinsDialog`, `SendCoinsDialog`, `AddressBookPage`.
ACKs for top commit:
ShaMan239:
Code review ACK d319c4dae9
promag:
Code review ACK d319c4dae9.
jarolrod:
ACK d319c4dae9
Tree-SHA512: b0c61f82811bb5aba2738067b53dc9ea4439230d547ce5c8fd85c480d8d70ea15f9942dbf13842383acbce467fba1ab4e132e37c56b654b46ba897301a41066e
