[pull] master from bitcoin:master#1013
Merged
pull[bot] merged 7 commits intomrpeertopeer:masterfrom Feb 27, 2026
Merged
Conversation
…r warning This refactor does not change any behavior, except for the integer sanitizer warning. Can be tested via: UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./bld-cmake/bin/bitcoin-cli -stdinrpcpass uptime
When `git range-diff` cannot match a commit between two versions of a branch, it shows the old commit as removed (`<`) and the new commit as added (`>`), and it does not show the patch contents. This output is easy to misread as "no code changes" because the diff for that commit is effectively empty. It really means the commits were considered unrelated and the new commit should be reviewed from scratch. This is analogous to rename detection in `git diff`: if similarity is too low, a rename shows up as delete+add. Example (exact SHAs from PR #34320): B=ff338fdb53a66ab40a36e1277e7371941fc89840; A=dd76338a57b9b1169ac27f7b783d6d0d4c6e38ab; git fetch upstream $B $A git range-diff 0ca4295..$B 139aa4b..$A This produced output like: 1: 4b32181 < -: ---------- test: add `HaveInputs` call-path unit tests -: ---------- > 1: 277c57f test: add `HaveInputs` call-path unit tests Even though the subject matches, the first commit had no matching diff shown. That should be treated as "unmatched" rather than "unchanged". If you expected a match, try increasing the creation factor so `git range-diff` searches harder: git range-diff --creation-factor=95 <old_range> <new_range>
Appveyor is not longer used however the test still requires to check for permissions including 666 as otherwise the tests fail on Windows
8834e4e test: remove appveyor reference in comment (Max Edwards) Pull request description: Appveyor is not longer used however the test still requires to check for permissions including 666 as otherwise the tests fail on Windows Fixes: #32576 ACKs for top commit: maflcko: lgtm ACK 8834e4e hebasto: ACK 8834e4e. Tree-SHA512: 655b44e52da5e0c6c11c79bb4f92c701c6e0e66dce8d7791ccf1d64e4561fe4d1d5f37c1317bead89c88e4d7208a278925168b419482a6be17abf93d0ebc5dfa
45133c5 doc: clarify `git range-diff` add/delete output (Lőrinc) Pull request description: ### Problem Range diffs in git are useful after PR rebases, but it has an easy-to-misread failure mode: if it cannot match a commit between the old and new ranges, it will show the old commit as removed (<) and the new commit as added (>), without showing any patch contents for that commit. It can look like there were no code changes when in reality the commit was just treated as unrelated and needs full re-review. ### Example ```bash git fetch upstream ff338fd dd76338 git range-diff ff338fd...dd76338 ``` This produced output like: ```patch 1: 0ca4295 = 93: 139aa4b bench: add on-disk `HaveInputs` benchmark 2: 4b32181 < -: ---------- test: add `HaveInputs` call-path unit tests -: ---------- > 94: 277c57f test: add `HaveInputs` call-path unit tests 3: 8c57687 ! 95: c0c94ec dbwrapper: have `Read` and `Exists` reuse `ReadRaw` @@ Metadata ## Commit message ## dbwrapper: have `Read` and `Exists` reuse `ReadRaw` - `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl` (except that it copies the resulting string on success, but that will be needed for caching anyway). + `ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl`. ``` Even though the subject matches, there is no diff shown because the commits did not match - the reviewer could think that only the commit message was changed. This should be treated as **unmatched** rather than **unchanged**. If you expected a match, you can try increasing the search effort: ```bash git range-diff --creation-factor=95 ff338fd...dd76338 ``` which would show for example: ```patch 1: 0ca4295 = 93: 139aa4b bench: add on-disk `HaveInputs` benchmark 2: 4b32181 ! 94: 277c57f test: add `HaveInputs` call-path unit tests @@ Commit message The tests document that `HaveInputs()` consults the cache first and that a cache miss pulls from the backing view via `GetCoin()`. + Co-authored-by: Novo <eunovo9@gmail.com> + ## src/test/coins_tests.cpp ## @@ src/test/coins_tests.cpp: BOOST_FIXTURE_TEST_CASE(ccoins_flush_behavior, FlushTest) } } -+BOOST_AUTO_TEST_CASE(ccoins_haveinputs_cache_miss_uses_base_getcoin) ++BOOST_AUTO_TEST_CASE(ccoins_cache_behavior) ``` ### Fix This PR updates `doc/productivity.md` to raise awareness and document this pitfall and mentions `--creation-factor` as a knob to try when the output seems unexpectedly empty. ACKs for top commit: maflcko: review ACK 45133c5 🏦 Sjors: ACK 45133c5 rkrux: crACK 45133c5 sedited: ACK 45133c5 Tree-SHA512: 52dcf6db51425a3ac9789627f80233fb1e3437f7a351acf4a761504d9917837aa1ff8c964605a842ee099fae9842946784f7603f9bffa7051429b2f04b7900be
fa6af85 refactor: Use static_cast<decltype(...)> to suppress integer sanitizer warning (MarcoFalke) fa69297 util: Fix UB in SetStdinEcho when ENOTTY (MarcoFalke) Pull request description: The call to `tcgetattr` may fail with `ENOTTY`, leaving the struct possibly uninitialized (UB). Fix this UB by returning early when `isatty` fails, or when `tcgetattr` fails. (Same for Windows) This can be tested by a command that fails valgrind before the change and passes after: ``` echo 'pipe' | valgrind --quiet ./bld-cmake/bin/bitcoin-cli -stdinrpcpass uptime ACKs for top commit: achow101: ACK fa6af85 l0rinc: lightly tested code review ACK fa6af85 sedited: ACK fa6af85 Tree-SHA512: 76e2fbcb6c323b17736ee057dbd5e932b2e8cbb7d9fe4488c1dc7ab6ea928a3cde7e72ca0a63f8c8c78871ccb8b669263b712c0e1b530d88f2d45ea41f071201
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )