💬 fanquake commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575676917)
> That's not that long ago though, and we have `OSX_MIN_VERSION=11.0` for now.
Just because we build release binaries that support that operating system, does not mean you need to be able to compile on that system (regardless of using the system compiler).
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575676917)
> That's not that long ago though, and we have `OSX_MIN_VERSION=11.0` for now.
Just because we build release binaries that support that operating system, does not mean you need to be able to compile on that system (regardless of using the system compiler).
💬 fanquake commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575678033)
Why would LLVM not work? We use the near latest release of LLVM to build the macOS release binaries. There's no point making this LLVM 14 just to "bump it" almost immediately. We should just point to the latest version unless there's an actual reason not to.
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575678033)
Why would LLVM not work? We use the near latest release of LLVM to build the macOS release binaries. There's no point making this LLVM 14 just to "bump it" almost immediately. We should just point to the latest version unless there's an actual reason not to.
💬 knocte commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2071479147)
cACK
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2071479147)
cACK
💬 maflcko commented on pull request "contrib: rpcauth.py - Add new option (-json) to output text in json format":
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-2071512668)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-2071512668)
Are you still working on this?
💬 maflcko commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2071541769)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/26114#issuecomment-2071541769)
Are you still working on this?
💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575734223)
That's also true, though so far I haven't seen evidence that compilation is impossible.
Homebrews main motivation for dropping macOS 11 seems to be support requests. From their install script:
> This installation may not succeed.
After installation, you will encounter build failures with some formulae.
Please create pull requests instead of asking for help on Homebrew\'s GitHub,
Twitter or any other official channels. You are responsible for resolving any
issues you experience while yo
...
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575734223)
That's also true, though so far I haven't seen evidence that compilation is impossible.
Homebrews main motivation for dropping macOS 11 seems to be support requests. From their install script:
> This installation may not succeed.
After installation, you will encounter build failures with some formulae.
Please create pull requests instead of asking for help on Homebrew\'s GitHub,
Twitter or any other official channels. You are responsible for resolving any
issues you experience while yo
...
💬 maflcko commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575739845)
I guess someone will need to test either way. Once they have tested with `brew install llvm` vs `brew install llvm@_version_`, this can be bumped to the maximum working `_version_` (or none at all).
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575739845)
I guess someone will need to test either way. Once they have tested with `brew install llvm` vs `brew install llvm@_version_`, this can be bumped to the maximum working `_version_` (or none at all).
💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575740996)
> Why would LLVM not work?
I have no way to test it. Since Homebrew itself does not support macOS 11 it's reasonable to expect that their newer formulas break, while the old ones might be fine.
This instruction is specifically for old versions of macOS. For newer versions we still have them use the latest thing.
(https://github.com/bitcoin/bitcoin/pull/29934#discussion_r1575740996)
> Why would LLVM not work?
I have no way to test it. Since Homebrew itself does not support macOS 11 it's reasonable to expect that their newer formulas break, while the old ones might be fine.
This instruction is specifically for old versions of macOS. For newer versions we still have them use the latest thing.
💬 maflcko commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2071572818)
> Rebased to address conflict
The rebase is wrong. Please pay attention to correctly address any merge conflicts, as they arise. One way to avoid mistakes, is to review your own code changes and to check rebases, as other reviewers will do later. For example, using the range-diff, as explained in the docs.
(https://github.com/bitcoin/bitcoin/pull/29771#issuecomment-2071572818)
> Rebased to address conflict
The rebase is wrong. Please pay attention to correctly address any merge conflicts, as they arise. One way to avoid mistakes, is to review your own code changes and to check rebases, as other reviewers will do later. For example, using the range-diff, as explained in the docs.
💬 asctime commented on issue "ReadAnchor throws exception on second run":
(https://github.com/bitcoin/bitcoin/issues/29931#issuecomment-2071603173)
> That should not be the case. The point of standard C++ library `std::filesystem::remove` is to provide the same interface behavior, regardless of compiler or operating system.
>
> The specification says that `false` should be returned when the file does not exist, not an exception be thrown.
Ah good point ><. I'll try it once with my clang install, it's a fair bit newer than my gcc which is due to be updated anyway. Ok to close from my side. Thanks.
(https://github.com/bitcoin/bitcoin/issues/29931#issuecomment-2071603173)
> That should not be the case. The point of standard C++ library `std::filesystem::remove` is to provide the same interface behavior, regardless of compiler or operating system.
>
> The specification says that `false` should be returned when the file does not exist, not an exception be thrown.
Ah good point ><. I'll try it once with my clang install, it's a fair bit newer than my gcc which is due to be updated anyway. Ok to close from my side. Thanks.
💬 laanwj commented on issue "RFC: In guix compile the GUI sequentially from everything else?":
(https://github.com/bitcoin/bitcoin/issues/29914#issuecomment-2071614437)
Sure, and there may still be other reasons to have seperate build step; the idea of fully static binaries for the non-GUI utilities was raised again at CoreDev. This is not possible with the GUI as it necessarily needs access to the dynamic linker. And as this might require different compile and linker flags, this would also effectively need two seperate builds.
(https://github.com/bitcoin/bitcoin/issues/29914#issuecomment-2071614437)
Sure, and there may still be other reasons to have seperate build step; the idea of fully static binaries for the non-GUI utilities was raised again at CoreDev. This is not possible with the GUI as it necessarily needs access to the dynamic linker. And as this might require different compile and linker flags, this would also effectively need two seperate builds.
💬 glozow commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2071619983)
Thank you @maflcko!
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2071619983)
Thank you @maflcko!
💬 maflcko commented on pull request "test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply":
(https://github.com/bitcoin/bitcoin/pull/29617#issuecomment-2071624163)
ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑
<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 ec1f1abfefa281e62bb876aa1c4
...
(https://github.com/bitcoin/bitcoin/pull/29617#issuecomment-2071624163)
ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑
<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 ec1f1abfefa281e62bb876aa1c4
...
✅ maflcko closed an issue: "ReadAnchor throws exception on second run"
(https://github.com/bitcoin/bitcoin/issues/29931)
(https://github.com/bitcoin/bitcoin/issues/29931)
💬 maflcko commented on issue "ReadAnchor throws exception on second run":
(https://github.com/bitcoin/bitcoin/issues/29931#issuecomment-2071645719)
Closing for now, but please leave a comment if there are more details to debug this issue.
(https://github.com/bitcoin/bitcoin/issues/29931#issuecomment-2071645719)
Closing for now, but please leave a comment if there are more details to debug this issue.
💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071673231)
I wiped the disk and reinstalled macOS 13.6.6. Still getting the permission denied error.
I narrowed it down further: this only happens when I clone the repo on my external SSD drive, not when it's on the built in disk.
```
Preprocessing bdb...
patching file 'dbinc/atomic.h'
Can't create '/var/folders/6c/n_bj8h8s0212j2tdtk_c_93m0000gn/T/patchogFOtRafNrn', output is in '/var/folders/6c/n_bj8h8s0212j2tdtk_c_93m0000gn/T/patchogFOtRafNrn': Permission denied
patch: **** can't create '/var/f
...
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071673231)
I wiped the disk and reinstalled macOS 13.6.6. Still getting the permission denied error.
I narrowed it down further: this only happens when I clone the repo on my external SSD drive, not when it's on the built in disk.
```
Preprocessing bdb...
patching file 'dbinc/atomic.h'
Can't create '/var/folders/6c/n_bj8h8s0212j2tdtk_c_93m0000gn/T/patchogFOtRafNrn', output is in '/var/folders/6c/n_bj8h8s0212j2tdtk_c_93m0000gn/T/patchogFOtRafNrn': Permission denied
patch: **** can't create '/var/f
...
💬 maflcko commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071685458)
Does macOS, or patch respect `TMPDIR, TMP, TEMP,` or `TEMPDIR`? If yes, it could be used as a temporary workaround.
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071685458)
Does macOS, or patch respect `TMPDIR, TMP, TEMP,` or `TEMPDIR`? If yes, it could be used as a temporary workaround.
💬 maflcko commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1575878130)
Maybe move this to util.cpp as well? In theory it is the wrong place, because it depends on `FuzzedDataProvider`, but I think this isn't too important in tests.
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1575878130)
Maybe move this to util.cpp as well? In theory it is the wrong place, because it depends on `FuzzedDataProvider`, but I think this isn't too important in tests.
👍 maflcko approved a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936#pullrequestreview-2016623784)
lgtm, nice!
(https://github.com/bitcoin/bitcoin/pull/29936#pullrequestreview-2016623784)
lgtm, nice!
💬 Sjors commented on issue "depens: bdb build fails on Intel macOS 13.6.6 ":
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071750271)
It seems to:
```
mkdir /tmp/test
% TMPDIR=/tmp/test make NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
Preprocessing bdb...
patching file 'dbinc/atomic.h'
Can't create '/tmp/test/patchoSHQPQFIlmq', output is in '/tmp/test/patchoSHQPQFIlmq': Permission denied
patch: **** can't create '/tmp/test/patchoSHQPQFIlmq': Permission denied
make: *** [/Volumes/SSD/test/bitcoin/depends/work/build/x86_64-apple-darwin22.6.0/bdb/4.8.30-0de9a3a86cb/.stamp_preprocessed
...
(https://github.com/bitcoin/bitcoin/issues/29792#issuecomment-2071750271)
It seems to:
```
mkdir /tmp/test
% TMPDIR=/tmp/test make NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_NATPMP=1 NO_USDT=1
Preprocessing bdb...
patching file 'dbinc/atomic.h'
Can't create '/tmp/test/patchoSHQPQFIlmq', output is in '/tmp/test/patchoSHQPQFIlmq': Permission denied
patch: **** can't create '/tmp/test/patchoSHQPQFIlmq': Permission denied
make: *** [/Volumes/SSD/test/bitcoin/depends/work/build/x86_64-apple-darwin22.6.0/bdb/4.8.30-0de9a3a86cb/.stamp_preprocessed
...