💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2629344469)
Rebased

💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629345112)
Proposing a fix for #31771:

Set USE_CCACHE=OFF for MSVC in cmake/ccache.cmake.
Update status reporting in CMakeLists.txt.

Questions:

Does this align with project conventions?
Any edge cases (MinGW/cross-compile) to test?

💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#discussion_r1938474813)
Unset the `srcs` variable at the end to avoid polluting the global namespace?

👍 hebasto approved a pull request: "cmake: add optional source files to bitcoin_crypto directly"
(https://github.com/bitcoin/bitcoin/pull/31268#pullrequestreview-2588535898)
ACK 7a53ce742207c5dfaeda7858a098049627b03d55, tested on Ubuntu 24.04, both x86_64 and aarch64.

These changes effectively revert https://github.com/hebasto/bitcoin/pull/171 and https://github.com/hebasto/bitcoin/pull/172.

Could you please mention the changes to `cmake/crc32c.cmake` in the PR title and description, and separate them into their own commit?

Additionally, it seems reasonable to mention [this](https://github.com/bitcoin/bitcoin/pull/31268/files#r1926593281) in the PR descript
...

💬 hebasto commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#discussion_r1938474356)
style nit: Keep the `PRIVATE` keyword on its own line with proper indentation, as in all surrounding code?

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2629391951)
Re-ACK bdec262bf661db9d46fa9b4e3fb42508751b9ea8

Only rebased since last review.

📝 hebasto opened a pull request: "cmake: Improve safety and robustness during building `crc32c` subtree"
(https://github.com/bitcoin/bitcoin/pull/31779)
It was [agreed](https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301) not to adopt the upstream buildsystem the `crc32c` subtree.

So this PR ensures that our custom build script properly handles potentially harmful situations, including:
1. The upstream buildsystem changes remaining unnoticed during the subtree update, even when some of those changes need to be reflected in `crc32c.cmake`.

2. The order of `include(cmake/introspection.cmake)` and `include(cmake/crc32c.cmak
...

📝 hebasto converted_to_draft a pull request: "cmake: Improve safety and robustness during building `crc32c` subtree"
(https://github.com/bitcoin/bitcoin/pull/31779)
It was [agreed](https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301) not to adopt the upstream buildsystem the `crc32c` subtree.

So this PR ensures that our custom build script properly handles potentially harmful situations, including:
1. The upstream buildsystem changes remaining unnoticed during the subtree update, even when some of those changes need to be reflected in `crc32c.cmake`.

2. The order of `include(cmake/introspection.cmake)` and `include(cmake/crc32c.cmak
...

📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31777)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 rhysbeynon opened a pull request: "Added new bitcoin.icns"
(https://github.com/bitcoin/bitcoin/pull/31780)
**Mac OS Minor icon change**
Apple's design guidelines have changed since the previous iteration of the Bitcoin-Core app icon. As it stands, the current icon is too large, and juts out when viewed alongside other program's icons.
My solution was to decrease the overall size to more closely fir Apple's current design documentation, keeping the main body of the icon in the body section of MacOS's icon template, while maintaining the same shape, color, and design.
My motives for this method wer
...

💬 purpleKarrot commented on pull request "cmake: Improve safety and robustness during building `crc32c` subtree":
(https://github.com/bitcoin/bitcoin/pull/31779#issuecomment-2629481474)
Whether the upstream `CMakeLists.txt` file is used or not, the `crc32c` subtree should be added with `add_subdirectory` and not `include`. The latter prevents variables from leaking into the parent directory.

💬 purpleKarrot commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629506821)
I don't understand the motivation for setting `CMAKE_<lang>_COMPILER_LAUNCHER` from inside the project. Ideally, the project should not interfere with compiler launchers at all.

This gives individual users as well as CI instances the highest flexibility.

Assuming I have `ENV{CMAKE_CXX_COMPILER_LAUNCHER}` set locally to use [`sccache`](https://github.com/mozilla/sccache). What is the expected behavior when `cmake/ccache.cmake` invokes `list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE
...

💬 davidgumberg commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1938551789)
I agree, but in that situation we would be back to printing generator expressions in the flags summary, which seems to me better than silently printing something that is not true.

💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629512226)
@purpleKarrot Should we remove the CMAKE_*_COMPILER_LAUNCHER assignments from ccache.cmake to prioritize user/CI flexibility? This would:

Avoid conflicts with external launchers like sccache set via ENV{CMAKE_CXX_COMPILER_LAUNCHER}.
Follow CMake best practices by letting users control launchers externally.
Prevent invalid command chains (e.g., sccache;ccache;cl.exe) when multiple launchers are set.

If agreed, I can update the PR to delete these lines and add documentation instead

💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629513923)
I'm a newbie to this repository. Can you help me to understand it?

💬 purpleKarrot commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629517352)
@vijayabhaskar78, I am a newbe in this repository too, but I am not a newbie with CMake ;-)
My recommendation would be to remove `cmake/ccache.cmake` for the reason you wrote.
But let the original authors of that file chime in.

💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2629524038)
OK let us see

💬 Oztayls commented on issue ""Rolling forward" at startup can take a long time, and is not interruptible":
(https://github.com/bitcoin/bitcoin/issues/11600#issuecomment-2629572914)
Any more on this? I've been stuck on 90% for hours. Wondering if a smaller dbcache is more preventative, albeit slower? My internet provider shut off at midnight over the weekend for maintenance, causing this issue.

✅ pablomartin4btc closed a pull request: "Add new "address type" column to the "receiving tab" address book page"
(https://github.com/bitcoin-core/gui/pull/753)

💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2629587602)
> Concept ~0 for adding a new feature that has not been vetted by designers, especially in light of development in https://github.com/bitcoin-core/gui-qml.

No problem! I originally worked on this PR as a way to get familiar with the Qt framework since I found the requirement in #646.

Thanks for your feedback - I'll close the PR to avoid any confusions.