💬 laanwj commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1577456975)
i still think it'd be better, but probably not worth doing this last-minute change for autotools.
Don't know how the CMake port handles these kind of checks anyway, but might consider it there.
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1577456975)
i still think it'd be better, but probably not worth doing this last-minute change for autotools.
Don't know how the CMake port handles these kind of checks anyway, but might consider it there.
👍 laanwj approved a pull request: "netbase: clean up Proxy logging"
(https://github.com/bitcoin/bitcoin/pull/29882#pullrequestreview-2019199241)
ACK fb4cc5f423ce587c1e97377e8afdf92fb4850f59
(https://github.com/bitcoin/bitcoin/pull/29882#pullrequestreview-2019199241)
ACK fb4cc5f423ce587c1e97377e8afdf92fb4850f59
💬 hebasto commented on pull request "doc: Suggest only necessary Qt packages for installation on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/29947#issuecomment-2074363052)
> > Verified on a fresh install of OpenBSD 7.5 that these two packages are the minimum requirement to build the GUI:
>
> Please update the version number in line 3 of the doc as well. I don't know if there is a difference between 7.4 and 7.5, but it can't hurt to be accurate here.
Sure. Updated.
(https://github.com/bitcoin/bitcoin/pull/29947#issuecomment-2074363052)
> > Verified on a fresh install of OpenBSD 7.5 that these two packages are the minimum requirement to build the GUI:
>
> Please update the version number in line 3 of the doc as well. I don't know if there is a difference between 7.4 and 7.5, but it can't hurt to be accurate here.
Sure. Updated.
💬 laanwj commented on pull request "Remove redundant `-datacarrier` option":
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2074372166)
This particular option was introduced 10 years ago (in e44fea55ea73f46bc9460597c7001e77acb58db7). FWIW i think we would need a stronger reason to remove it than a bit of redundancy.
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2074372166)
This particular option was introduced 10 years ago (in e44fea55ea73f46bc9460597c7001e77acb58db7). FWIW i think we would need a stronger reason to remove it than a bit of redundancy.
💬 maflcko commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2074415872)
ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d 🌽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK f19f0a2e5af6c2a64900f1f229
...
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2074415872)
ACK f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d 🌽
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK f19f0a2e5af6c2a64900f1f229
...
💬 TheCharlatan commented on pull request "refactor: Rename `subprocess.hpp` to follow our header name conventions":
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2074457183)
> Resolved linter warnings.
Can you mention this in the PR description?
(https://github.com/bitcoin/bitcoin/pull/29910#issuecomment-2074457183)
> Resolved linter warnings.
Can you mention this in the PR description?
💬 Kellydutch1 commented on pull request "doc: Suggest only necessary Qt packages for installation on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/29947#issuecomment-2074462911)
I have bitcoin hacking software
(https://github.com/bitcoin/bitcoin/pull/29947#issuecomment-2074462911)
I have bitcoin hacking software
💬 fanquake commented on pull request "build: Consider `SOURCE_DATE_EPOCH` in Guix environment only":
(https://github.com/bitcoin/bitcoin/pull/29761#issuecomment-2074466635)
What is the status of this?
(https://github.com/bitcoin/bitcoin/pull/29761#issuecomment-2074466635)
What is the status of this?
📝 fanquake converted_to_draft a pull request: "doc: add LLVM instruction for macOS < 13"
(https://github.com/bitcoin/bitcoin/pull/29934)
#29208 bumped clang to 14, which users of old macOS versions need to install manually. This PR adds instructions.
Xcode 14.3.1 ships clang 14.0.3 (14.0.0 is broken, see #29918):
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)
The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-release-notes#
Homebrew itself officially supports macOS 12 or later, but _may_ still work o
...
(https://github.com/bitcoin/bitcoin/pull/29934)
#29208 bumped clang to 14, which users of old macOS versions need to install manually. This PR adds instructions.
Xcode 14.3.1 ships clang 14.0.3 (14.0.0 is broken, see #29918):
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)
The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-release-notes#
Homebrew itself officially supports macOS 12 or later, but _may_ still work o
...
⚠️ mouhand-a opened an issue: "0xF2852f0DD3703afc3A5B4343e262839eEf73eD23"
(https://github.com/bitcoin/bitcoin/issues/29950)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
My address [](url)
0xF2852f0DD3703afc3A5B4343e262839eEf73eD23
(https://github.com/bitcoin/bitcoin/issues/29950)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
My address [](url)
0xF2852f0DD3703afc3A5B4343e262839eEf73eD23
✅ hebasto closed an issue: "0xF2852f0DD3703afc3A5B4343e262839eEf73eD23"
(https://github.com/bitcoin/bitcoin/issues/29950)
(https://github.com/bitcoin/bitcoin/issues/29950)
:lock: hebasto locked an issue: "0xF2852f0DD3703afc3A5B4343e262839eEf73eD23"
(https://github.com/bitcoin/bitcoin/issues/29950)
(https://github.com/bitcoin/bitcoin/issues/29950)
💬 TheCharlatan commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1577616561)
I tested this without specifying a llvm version on macos 12.7.4 monterey. It did resolve the compilation errors. I would prefer this without a version, since it is yet another thing to keep track of in our versioning zoo, and I don't think it is preferable for developers to be instructed to use an older compiler. Also not sure if the guide should cover an OS version that is no longer supported by its package manager. If something eventually breaks for a developer, this could be revisited.
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1577616561)
I tested this without specifying a llvm version on macos 12.7.4 monterey. It did resolve the compilation errors. I would prefer this without a version, since it is yet another thing to keep track of in our versioning zoo, and I don't think it is preferable for developers to be instructed to use an older compiler. Also not sure if the guide should cover an OS version that is no longer supported by its package manager. If something eventually breaks for a developer, this could be revisited.
💬 fanquake commented on pull request "ci: Drop no longer needed `-I` flag in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2074566263)
Did this change with LLVM version, or IWYU version or changing the base image?
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2074566263)
Did this change with LLVM version, or IWYU version or changing the base image?
💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1577622427)
How important is it to preserve invalid encodings? For example the decode of `%1 ` (percent, 1, space) differs between this implementation and the one of libevent.
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1577622427)
How important is it to preserve invalid encodings? For example the decode of `%1 ` (percent, 1, space) differs between this implementation and the one of libevent.
💬 hebasto commented on pull request "ci: Drop no longer needed `-I` flag in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2074570556)
> Did this change with LLVM version, or IWYU version or changing the base image?
My guess is LLVM version, but I didn't check.
(https://github.com/bitcoin/bitcoin/pull/29929#issuecomment-2074570556)
> Did this change with LLVM version, or IWYU version or changing the base image?
My guess is LLVM version, but I didn't check.
🚀 fanquake merged a pull request: "ci: Drop no longer needed `-I` flag in "tidy" task"
(https://github.com/bitcoin/bitcoin/pull/29929)
(https://github.com/bitcoin/bitcoin/pull/29929)
💬 fanquake commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1577642260)
> but probably not worth doing this last-minute change for autotools.
Not sure. Everything not done for Autotools, but done in CMake is just more bebaviour/divergence to maintain, so I'd prefer to do as much as possible prior.
(https://github.com/bitcoin/bitcoin/pull/29876#discussion_r1577642260)
> but probably not worth doing this last-minute change for autotools.
Not sure. Everything not done for Autotools, but done in CMake is just more bebaviour/divergence to maintain, so I'd prefer to do as much as possible prior.
💬 fanquake commented on pull request "[WIP] libevent @ master + use CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2074617393)
> and if we liked the CMake build parts we'd patch them in and drop the rest.
Yep. From the OP:
> Depending on the outcome, we could possibly pull a (minified) patch set into depends.
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2074617393)
> and if we liked the CMake build parts we'd patch them in and drop the rest.
Yep. From the OP:
> Depending on the outcome, we could possibly pull a (minified) patch set into depends.
💬 hebasto commented on pull request "build: Consider `SOURCE_DATE_EPOCH` in Guix environment only":
(https://github.com/bitcoin/bitcoin/pull/29761#issuecomment-2074627637)
> > However, using it outside Guix scripts for local builds on macOS is not meaningful.
>
> Why? Using Guix should not be a requirement for SOURCE_DATE_EPOCH to be useful.
Not a requirement. Both Guix and SOURCE_DATE_EPOCH are tools of the same domain, which is build reproducibility.
> I would think that given the pinned compilers, and provisioned std lib, the macOS cross build would actually pretty be close to deterministic outside of Guix.
Is anyone interesting / working on that?
...
(https://github.com/bitcoin/bitcoin/pull/29761#issuecomment-2074627637)
> > However, using it outside Guix scripts for local builds on macOS is not meaningful.
>
> Why? Using Guix should not be a requirement for SOURCE_DATE_EPOCH to be useful.
Not a requirement. Both Guix and SOURCE_DATE_EPOCH are tools of the same domain, which is build reproducibility.
> I would think that given the pinned compilers, and provisioned std lib, the macOS cross build would actually pretty be close to deterministic outside of Guix.
Is anyone interesting / working on that?
...