[pull] master from bitcoin:master #1450
Merged
Conversation
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
Also, rename lint_doc to lint_doc_args.
Also, run cargo fmt on main.rs
This is redundant and inconsistent, because call-sites are expected to add the semicolon, which they all do.
This refactor makes the code easier to read and maintain.
There is only one call-site, which provided an empty caption. Note that noui_ThreadSafeQuestionRedirect is test-only and currently entrirely unused, so the logging format string change is not a behavior change. This refactor does not change any behavior.
The caption was empty for all call-sites, so this refactor does not change any behavior. Note that noui_ThreadSafeMessageBoxRedirect is test-only, so no end-user behavior is changed here.
This refactor does not change any behavior.
The only behavior change is in noui_ThreadSafeQuestion, which can not detect a style and will log a strCaption=": ". Fix this by removing it.
Change this so we catch the case where the capnp shared libs have been
updated, and can no-longer be loaded by the Python module, resulting in
a skipped test, even though pycapnp is installed. i.e:
```bash
stderr:
Traceback (most recent call last):
File "/root/ci_scratch/build/test/functional/interface_ipc.py", line 20, in <module>
import capnp # type: ignore[import] # noqa: F401
^^^^^^^^^^^^
File "/usr/local/lib64/python3.14/site-packages/capnp/__init__.py", line 36, in <module>
from .version import version as __version__
File "/usr/local/lib64/python3.14/site-packages/capnp/version.py", line 1, in <module>
from .lib.capnp import _CAPNP_VERSION_MAJOR as LIBCAPNP_VERSION_MAJOR # noqa: F401
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: libcapnpc.so.1.0.1: cannot open shared object file: No such file or directory
```
Failing in this way should make it clear that `pycapnp` needs to be
reinstalled/rebuilt.
If `pycapnp` is not installed, the test still skips as expected:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py skipped (capnp module not available.)
TEST | STATUS | DURATION
interface_ipc.py | ○ Skipped | 0 s
```
Fixes: #34016.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Also, fix whitespace in this function, while touching it. Can be reviewed via the git option --ignore-all-space
This is used for argument parsing in the retry script, however we don't use the script with any arguments. So remove the unused code, and the dependency on gnu-getopt. This came up in the context of adding new CI jobs, where gnu-getopt might not be available, or working properly. It seemed easier to just remove the unused code, than look for more workarounds.
Co-authored-by: Mark "Murch" Erhardt <murch@murch.one>
fad7bd9 noui: Remove always empty caption while formatting (MarcoFalke) fa8ebeb refactor: [gui] Document that the title is always empty for node message (MarcoFalke) fafe71b refactor: Remove empty caption from ThreadSafeMessageBox (MarcoFalke) fa8d008 refactor: Remove empty caption from ThreadSafeQuestion (MarcoFalke) fa01954 refactor: [gui] Use lambdas over std::bind (MarcoFalke) eeee1e3 refactor: Remove trailing semicolon after ADD_SIGNALS_DECL_WRAPPER (MarcoFalke) Pull request description: Currently, the user interface (noui, gui) has a caption for each message. However, the caption has many issues: * It is always hard-coded to the empty string. * This is confusing and tedious when reading or maintaining the code. * It is redundant, because `noui` will ignore the caption and set the logging prefix (error, warning, info) based on the `style`. * The gui does prefer to set the title based on the caption, but since it the caption is always empty, the fallback will always be used. Fix all issues by removing it. ACKs for top commit: hebasto: ACK fad7bd9, I have reviewed the code and it looks OK. Tested on Ubuntu 25.10. sedited: ACK fad7bd9 Tree-SHA512: 58ef538b9b3e1cfdcf2955f6de9b8cee335edbf6339723cb693cb4d584817904c962dac5199ee44d7e2860a5332dec1a6abf47e621eb5cf919aa1cdae271b55f
ddae1b4 ci: remove gnu-getopt usage (fanquake) Pull request description: This is used for argument parsing in the `retry` script, however we don't use the script with any arguments. So remove the unused code, and the dependency on `gnu-getopt`. This came up in the context of adding new CI jobs, where gnu-getopt might not be available, or working properly. It seemed easier to just remove the unused code, than look for more workarounds. ACKs for top commit: maflcko: review ACK ddae1b4 🔀 sedited: ACK ddae1b4 Tree-SHA512: a73cf61fe0965127f87f1725b3a25a305ebfd354c318f5f44ecfa20da02ba72fef42dca656dae07f6e1ece956b9d7c58e99edb124d968a4bffb2ce6ac8fc018b
3400db8 doc: add missing param description to SRD (yancy) Pull request description: The params documentation is missing `change_fee` and the description is lacking recent changes. ACKs for top commit: murchandamus: ACK 3400db8 brunoerg: code review ACK 3400db8 Tree-SHA512: 8f6fac0d92873c5c9f77b19fbc0c6ecfb425b2a6b3d5f5ad69c82ed706b21cf4627e68c71acbc43661000e6063e8f8dbcd3b8ff60e3c727bdcba497d13ee1383
On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when "GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py. This requires write access to the .git directory. Make the mounted .git directory writable. This is currently not run on PR branches or locally which caused a miss during review.
c8abac9 ci: mount .git dir rw (ci) Pull request description: On merges to master we set LINT_CI_SANITY_CHECK_COMMIT_SIG (when "GITHUB_REPOSITORY == bitcoin/bitcoin") which runs verify-commits.py. This requires write access to the .git directory. Make the mounted .git directory writable. This is currently not run on PR branches or locally which caused a miss during review. Ideally we can have the same checks running in PRs as on merges to master to avoid future discrepancies like this. ACKs for top commit: maflcko: lgtm ACK c8abac9 l0rinc: untested code review ACK c8abac9 Tree-SHA512: 7ae4f63227ecffe1dc9003454a7473d6d592550af2e1c899457f34a947e5604b04c13319fb8979f36789ae7787bed62066be60697d163ad5ebedde3fbe8ce45f
…em as errors fdc9fe2 ci, iwyu: Fix warnings in `src/primitives` and treat them as errors (Hennadii Stepanov) Pull request description: This PR [continues](#33725 (comment)) the ongoing effort to enforce IWYU warnings. See [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu). ACKs for top commit: maflcko: review ACK fdc9fe2 📀 janb84: ACK fdc9fe2 sedited: ACK fdc9fe2 Tree-SHA512: d290545c7aab477b4a5bf121b694899a78e0526be72efa31fa4205b0fd840e6e8240d32f9134a18c9dc58c5f91e7847d7f20ca34f8d2edc4d541ac858ec0dccc
fa578d9 lint: [move-only] Move python related lints to lint_py.rs (MarcoFalke) fa392c3 lint: [move-only] Move repo related lints to lint_repo_hygiene.rs (MarcoFalke) fab0cfa lint: [move-only] Move cpp related lints to lint_cpp.rs (MarcoFalke) fa3e48e lint: [move-only] Move docs related lints to lint_docs.rs (MarcoFalke) fad09e7 lint: [move-only] Move text related lints to text_format.rs (MarcoFalke) faf40c2 lint: [move-only] Move util functions to util.rs (MarcoFalke) Pull request description: The single, large `main.rs` file is fine, but at some point it becomes harder to read. So reduce the size by pulling functions out into modules. This can be reviewed with the git option: `--color-moved=dimmed-zebra` ACKs for top commit: l0rinc: Lightly tested code review ACK fa578d9 sedited: ACK fa578d9 Tree-SHA512: f1e29fd3cf695fb6634d0b9f9e55508992b4b9885afee9dbe4d5d9e99cad3061e7141f39acbfe69d698422888169128cd7658a6dc991fd904b8520328b51586d
ab649ce guix: documented shasum gathering command (janb84) Pull request description: When a PR requires proof of Guix builds (sha256sums), the PR author or reviewer uses a not well documented command to collect the sha256sums of build outputs or manually gathers them from files. This pull request introduces a new section in the documentation, providing some documentation on the command's functionality and usage. ACKs for top commit: willcl-ark: ACK ab649ce sedited: ACK ab649ce Tree-SHA512: 0188663ad117b636c7d32a1b655db97610f558cfcffe4abd6f0fb097b3990db0dc6d23ab972926fefd2531b21f429742dcbea6b0fa579d22d5da7a7d6a4c753e
…onditionally fa9c92d log: Print warning about privacy-sensitive log info unconditionally (MarcoFalke) Pull request description: There is a warning about logs containing privacy-sensitive information. However, it is only printed when at least one debug log category is enabled. This is confusing, because: * Setting (let's say) `-debug=reindex` enables this warning, but it is hard to see what sensitive logs could be contained in reindex debug logs. * Dropping `-debug=reindex` again disabled this warning, but the wallet continues to log txids (and other sensitive stuff) at info level. So instead of implying the wrong thing, it would be better to remove this log line (because it should be common sense), or log it unconditionally. ACKs for top commit: l0rinc: ACK fa9c92d sedited: ACK fa9c92d Tree-SHA512: 42f71b030e7722203f225f04e979143e829dae3556f64e322a791361a3b9c16150d53bb7bb9a99839c975d9052115770b9473138acc58baeee457253526fd892
905dfde test: use ModuleNotFoundError in interface_ipc.py (fanquake) Pull request description: Change this so we catch the case where the capnp shared libs have been updated, and can no-longer be loaded by the Python module, resulting in a skipped test, even though pycapnp is installed. i.e: ```bash stderr: Traceback (most recent call last): File "/root/ci_scratch/build/test/functional/interface_ipc.py", line 20, in <module> import capnp # type: ignore[import] # noqa: F401 ^^^^^^^^^^^^ File "/usr/local/lib64/python3.14/site-packages/capnp/__init__.py", line 36, in <module> from .version import version as __version__ File "/usr/local/lib64/python3.14/site-packages/capnp/version.py", line 1, in <module> from .lib.capnp import _CAPNP_VERSION_MAJOR as LIBCAPNP_VERSION_MAJOR # noqa: F401 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ImportError: libcapnpc.so.1.0.1: cannot open shared object file: No such file or directory ``` Failing in this way should make it clear that `pycapnp` needs to be reinstalled/rebuilt. If `pycapnp` is not installed, the test still skips as expected: ```bash Remaining jobs: [interface_ipc.py] 1/1 - interface_ipc.py skipped (capnp module not available.) TEST | STATUS | DURATION interface_ipc.py | ○ Skipped | 0 s ``` Fixes: #34016. ACKs for top commit: maflcko: lgtm ACK 905dfde hebasto: ACK 905dfde, I have reviewed the code and it looks OK. However, I'm [not able](#34016 (comment)) to reproduce #34016. sedited: ACK 905dfde Tree-SHA512: 3cedbe8fc51cc18f1c993f7747d20905f3bf94c736db99a9c4090f5823bf8c09dfbc19ef03c573d504dcdfba6ea0f7d088a7f4563b220742c9a441167c04cfd6
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 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 : )