Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2454415368)
> Should probably remove this line from the docs:

Thanks! Amended.
💬 hebasto commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#issuecomment-2454426896)
> > Additionally, the `cmake` has been removed from the required packages,
> > as it is no longer specific to depends.
>
> If `cmake` is needed to build depends it should remain listed. The same as `make`, or a compiler.

Thanks! Reworked per feedback.
💬 fanquake commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1827573255)
If you're adding `g++` here, you can remove it from the macOS section below.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2454451603)
@maflcko

> They'd have to be cleared on all machines. Let me known if you want that to happen.

I believe this PR is in quite good shape, and it might be worth clearing the depends caches for CI and DrahtBot's Guix builder machines.

Shall we proceed with that?
📝 fanquake opened a pull request: "doc: archive release notes for v27.2"
(https://github.com/bitcoin/bitcoin/pull/31208)
Archive `v27.2`.
💬 hebasto commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1827577630)
Thanks! Removed.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1827586383)
Ok. Then can you update the patch with all the missing context? This still seems to just be working around an unidentified bug, which I'm assuming Qt would want to fix. i.e moving `#include <experimental/source_location>` down 1 line. I assume this "fixes" something be shifting the include into a non-macOS-cross taken path, but that isn't obvious from the patch, and if this patch needed rebasing (on top of changed code) in the future, it's not clear how to fix what will likely be obscure compile
...
💬 hebasto commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2454476561)
Rebased.

Addressed https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2446497505.

Undrafted.
👋 hebasto's pull request is ready for review: "build: Unify `-logsourcelocations` format"
(https://github.com/bitcoin/bitcoin/pull/30811)
💬 willcl-ark commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2454477097)
That is odd, and a little disappointing. The code appears correct to me.

I was wondering whether `tmpfs` according to docker might just mean "ephemeral" (but still disk-based), but it appears that it it **is** both ephemeral and ramdisk based as per the underlying filesystem `tmpfs`.

I will run the CI on my own self-hosted runner, and see what sort of numbers I get from it...
💬 stickies-v commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2454487414)
re-ACK c189eec848e3c31f438151d4d3422718a29df3a3

Only change is to remove mention of BIP-125 in `doc/bips.md`.
💬 laanwj commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1827601126)
Its... imo a bit strange to mention skipping g++ as it's so clearly a dependency for building bitcoin core itself. While correct in a strict sense, having it not installed is going to fail later.
Also the multiprocess lib (and deps) require a C++ compiler.
💬 hebasto commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1827605146)
The user can use a different compiler to build depends, but Qt 5 still insists on GCC for building the native `qmake`.
💬 maflcko commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2454509235)
My plan is to get hold on an HDD (or some other slow storage) and try with that. Any effect should be more pronounced on slower persistent storage.
💬 laanwj commented on pull request "depends, doc: List packages required to build `qt` package separately":
(https://github.com/bitcoin/bitcoin/pull/31192#discussion_r1827609702)
Okay, sure, then it makes sense.
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1827611959)
In 54d81cab53a07fc47ddd38d3be0384651d785558: You're removing this patch, but looking at the upstream source for 6.7.3, it's not immediately clear where this was fixed upstream, and the commit doesn't explain why it's no-longer needed. Can you elaborate? I'm wondering if new patches you are introducing are just a result of deleting this one.
💬 maflcko commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2454518448)
re-ACK 788c1324f3d840f7a39b8bc3537dcff26ca0b552

Only change is dropping doc blob.

Didn't test locally.
👍 laanwj approved a pull request: "doc: archive release notes for v27.2"
(https://github.com/bitcoin/bitcoin/pull/31208#pullrequestreview-2412871966)
ACK 1a05c86ae473d5b615f966203ed1759f2077784a , matches release notes on branch
```
git diff 1a05c86ae473d5b615f966203ed1759f2077784a:doc/release-notes/release-notes-27.2.md 27.x:doc/release-notes.md
```
💬 laanwj commented on pull request "doc: Use relative hyperlinks in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31206#issuecomment-2454521609)
Yes this makes sense, ideally the documentation should work even when it's not on github.
📝 Abdulkbk opened a pull request: "test: cover edge case for lockunspent rpc"
(https://github.com/bitcoin/bitcoin/pull/31209)
This PR tests the scenario where a negative number is passed as the vout value during the `lockunspent` RPC call. The rationale behind this test is to validate the input parameters for the `lockunspent` function and ensure that the system behaves correctly when given invalid data. By confirming that the error message "Invalid parameter: vout cannot be negative" is returned, we can prevent unexpected behavior or vulnerabilities in the application.