💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814865983)
Is that important for max 3 divisions?
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814865983)
Is that important for max 3 divisions?
💬 fanquake commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435152474)
Guix Build:
```bash
245051dc0f18c4da7060996df1ce9eb298b449e48b62cbad30017707c94faed2 guix-build-6c6b2442edab/output/aarch64-linux-gnu/SHA256SUMS.part
55b4b8b790069826c0564c1d3372fada3c8e71dc1a172f2c360aa74c7c041ef1 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu-debug.tar.gz
133bdd966b70875c8aa6b62eed27235fc327dfeaa8e048db524ec9ccf6085602 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu.tar.gz
f34455a7dc5581bb
...
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435152474)
Guix Build:
```bash
245051dc0f18c4da7060996df1ce9eb298b449e48b62cbad30017707c94faed2 guix-build-6c6b2442edab/output/aarch64-linux-gnu/SHA256SUMS.part
55b4b8b790069826c0564c1d3372fada3c8e71dc1a172f2c360aa74c7c041ef1 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu-debug.tar.gz
133bdd966b70875c8aa6b62eed27235fc327dfeaa8e048db524ec9ccf6085602 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu.tar.gz
f34455a7dc5581bb
...
💬 fanquake commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435157929)
Even though the package is available, looks like it didn't work there either.
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435157929)
Even though the package is available, looks like it didn't work there either.
👍 fanquake approved a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450#pullrequestreview-2392337705)
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.
(https://github.com/bitcoin/bitcoin/pull/29450#pullrequestreview-2392337705)
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814887974)
No I think this one is just "belt" -- if you look at lines 146-148 above, we're not always modifying the "child" transaction here, so sometimes it'll be a duplicate of something we already have.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814887974)
No I think this one is just "belt" -- if you look at lines 146-148 above, we're not always modifying the "child" transaction here, so sometimes it'll be a duplicate of something we already have.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814895952)
Yep you're right. I swapped the order of those two commits to avoid this problem, and I also added a test for package rbf with priority (which fails on https://github.com/bitcoin/bitcoin/commit/1c33e133db5f876f2ba6eeb28877a5dcf3f6ac04, but passes by the end of this branch).
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814895952)
Yep you're right. I swapped the order of those two commits to avoid this problem, and I also added a test for package rbf with priority (which fails on https://github.com/bitcoin/bitcoin/commit/1c33e133db5f876f2ba6eeb28877a5dcf3f6ac04, but passes by the end of this branch).
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435184924)
Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435184924)
Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435190097)
> Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
Removed.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435190097)
> Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?
Removed.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435192271)
Add the https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399 requirement at the same time?
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435192271)
Add the https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399 requirement at the same time?
🚀 fanquake merged a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450)
(https://github.com/bitcoin/bitcoin/pull/29450)
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571)
> Add the [#31042 (comment)](https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399) requirement at the same time?
Rebased on https://github.com/bitcoin/bitcoin/pull/31042.
Please review https://github.com/bitcoin/bitcoin/pull/31042 and https://github.com/bitcoin/bitcoin/pull/31147 first.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571)
> Add the [#31042 (comment)](https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399) requirement at the same time?
Rebased on https://github.com/bitcoin/bitcoin/pull/31042.
Please review https://github.com/bitcoin/bitcoin/pull/31042 and https://github.com/bitcoin/bitcoin/pull/31147 first.
💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2435214195)
> > It is definitely broken in #30997.
>
> Shouldn't #30997 be based on this PR then (or at least mention in the PR description it's expected to be broken)?
[Done](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571).
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2435214195)
> > It is definitely broken in #30997.
>
> Shouldn't #30997 be based on this PR then (or at least mention in the PR description it's expected to be broken)?
[Done](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571).
💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2435222694)
@fanquake pushed
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2435222694)
@fanquake pushed
✅ maflcko closed an issue: "CI failure: `Error: Directory '/tmp/ccache_dir' must be created in advance.`"
(https://github.com/bitcoin/bitcoin/issues/31108)
(https://github.com/bitcoin/bitcoin/issues/31108)
💬 maflcko commented on issue "CI failure: `Error: Directory '/tmp/ccache_dir' must be created in advance.`":
(https://github.com/bitcoin/bitcoin/issues/31108#issuecomment-2435224696)
Done in https://github.com/bitcoin/bitcoin/pull/31146. Let's continue the discussion there.
(https://github.com/bitcoin/bitcoin/issues/31108#issuecomment-2435224696)
Done in https://github.com/bitcoin/bitcoin/pull/31146. Let's continue the discussion there.
💬 fanquake commented on pull request "cmake, qt, test: Remove problematic code":
(https://github.com/bitcoin/bitcoin/pull/31147#issuecomment-2435227027)
> As a side effect, this PR fixes https://github.com/bitcoin-core/gui/issues/842.
Tested that this fixes building on Tumbleweed. `-DWITH_QRENCODE=OFF` is required, but that seems to be another issue, probably with the `libqrencode` package on that distro.
I take it this code has been dead/broken since it was introduced? 975d67369b8f3a33a21fd7618c299c0ec138292c
(https://github.com/bitcoin/bitcoin/pull/31147#issuecomment-2435227027)
> As a side effect, this PR fixes https://github.com/bitcoin-core/gui/issues/842.
Tested that this fixes building on Tumbleweed. `-DWITH_QRENCODE=OFF` is required, but that seems to be another issue, probably with the `libqrencode` package on that distro.
I take it this code has been dead/broken since it was introduced? 975d67369b8f3a33a21fd7618c299c0ec138292c
💬 hebasto commented on pull request "cmake, qt, test: Remove problematic code":
(https://github.com/bitcoin/bitcoin/pull/31147#issuecomment-2435232893)
> I take it this code has been dead/broken since it was introduced? [975d673](https://github.com/bitcoin/bitcoin/commit/975d67369b8f3a33a21fd7618c299c0ec138292c)
Correct.
(https://github.com/bitcoin/bitcoin/pull/31147#issuecomment-2435232893)
> I take it this code has been dead/broken since it was introduced? [975d673](https://github.com/bitcoin/bitcoin/commit/975d67369b8f3a33a21fd7618c299c0ec138292c)
Correct.
👍 fanquake approved a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147#pullrequestreview-2392428144)
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0
(https://github.com/bitcoin/bitcoin/pull/31147#pullrequestreview-2392428144)
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0
💬 laanwj commented on pull request "run_command: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/30756#issuecomment-2435244592)
Concept ACK. Closing unnecessary fds before starting external commands is good practice.
> cpp-subprocess has a feature to address this called close_fds. Not sure why it was removed in https://github.com/bitcoin/bitcoin/pull/29961 rather than fixing this during the migration, but this PR restores it
Pretty sure i brought up this [exact concern](https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2054078250) prior to the cpp-subprocess cleanup, but was assured it wasn't removed. Appa
...
(https://github.com/bitcoin/bitcoin/pull/30756#issuecomment-2435244592)
Concept ACK. Closing unnecessary fds before starting external commands is good practice.
> cpp-subprocess has a feature to address this called close_fds. Not sure why it was removed in https://github.com/bitcoin/bitcoin/pull/29961 rather than fixing this during the migration, but this PR restores it
Pretty sure i brought up this [exact concern](https://github.com/bitcoin/bitcoin/pull/29865#issuecomment-2054078250) prior to the cpp-subprocess cleanup, but was assured it wasn't removed. Appa
...
💬 hebasto commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435264454)
> Even though the package is available, looks like it didn't work there either.
The `qrencode-devel` package should be installed.
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435264454)
> Even though the package is available, looks like it didn't work there either.
The `qrencode-devel` package should be installed.