š¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559558868)
I'm not going to add the steps back in. It's too much of a game of whack-a-mole which invites bike-shedding (what works for one person doesn't work for someone else, it depends on macOS version, brew version etc.) and it looks like people are perfectly capable of figuring out what works for them on their own.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559558868)
I'm not going to add the steps back in. It's too much of a game of whack-a-mole which invites bike-shedding (what works for one person doesn't work for someone else, it depends on macOS version, brew version etc.) and it looks like people are perfectly capable of figuring out what works for them on their own.
š¬ l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489)
> it depends on macOS version, brew version etc
What if we documented a solution that would work with latest xcode and clang and whatever else is needed?
That shouldn't be too difficult and shouldn't involve "bike shedding" (I don't think it's the correct term here since it's not a "minor, easily understandable issues" at the expense of "neglecting more complex, important matters").
As mentioned, I strongly think we should have guidance for at least basic fuzzing support for Mac as well!
...
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489)
> it depends on macOS version, brew version etc
What if we documented a solution that would work with latest xcode and clang and whatever else is needed?
That shouldn't be too difficult and shouldn't involve "bike shedding" (I don't think it's the correct term here since it's not a "minor, easily understandable issues" at the expense of "neglecting more complex, important matters").
As mentioned, I strongly think we should have guidance for at least basic fuzzing support for Mac as well!
...
š¬ l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3575153013)
Concept ACK, I agree that we should clarify usage of fuzzing on a Max.
However, approach NACK, as described in https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489 (which was marked as resolved for some reason, so reposting here for visibility)
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3575153013)
Concept ACK, I agree that we should clarify usage of fuzzing on a Max.
However, approach NACK, as described in https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489 (which was marked as resolved for some reason, so reposting here for visibility)
š¬ diegoviola commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575174383)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?
The `Qt::WindowStaysOnTopHint` was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.
It seems like th
...
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575174383)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?
The `Qt::WindowStaysOnTopHint` was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.
It seems like th
...
ā
maflcko closed an issue: "RFC: Replacing `tinyformat` with `{fmt}`"
(https://github.com/bitcoin/bitcoin/issues/33942)
(https://github.com/bitcoin/bitcoin/issues/33942)
š¬ maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575193582)
> In addition to all of that, I think that will be a good step toward migrating to C++20 in the future, and we will be able to depend on the standard format library itself.
C++20 is already used, so I'll go ahead and close this for now.
I don't think there is value in switching `tinyformat` to `{fmt}` and then again to `std::format`, with all the churn.
If the switch is done, it should be straight to `std::format`.
So I'll close this for now, but the discussion can continue.
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575193582)
> In addition to all of that, I think that will be a good step toward migrating to C++20 in the future, and we will be able to depend on the standard format library itself.
C++20 is already used, so I'll go ahead and close this for now.
I don't think there is value in switching `tinyformat` to `{fmt}` and then again to `std::format`, with all the churn.
If the switch is done, it should be straight to `std::format`.
So I'll close this for now, but the discussion can continue.
š¬ hebasto commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575194223)
> Fixes #33859
Iām not convinced it does.
On the master branch @ 0690514d4f72aac251ee0b876cded9187d42c63e:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```
With this PR, the behaviour is unchanged:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575194223)
> Fixes #33859
Iām not convinced it does.
On the master branch @ 0690514d4f72aac251ee0b876cded9187d42c63e:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```
With this PR, the behaviour is unchanged:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```
š¬ diegoviola commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575197345)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?
The `Qt::WindowStaysOnTopHint` workaround was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.
It se
...
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575197345)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?
The `Qt::WindowStaysOnTopHint` workaround was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.
It se
...
š¬ maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575205474)
For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575205474)
For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1
š¬ maflcko commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575210686)
@psam21 Is this LLM generated? Have you tested this yourself?
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575210686)
@psam21 Is this LLM generated? Have you tested this yourself?
š¬ hebasto commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575216394)
The title says "depends: Fix native_capnp to respect build_CC and build_CXX", but it seems working as expected on the master:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx build_CXX=clang++
native_capnp_cxx=clang++
```
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575216394)
The title says "depends: Fix native_capnp to respect build_CC and build_CXX", but it seems working as expected on the master:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx build_CXX=clang++
native_capnp_cxx=clang++
```
š¬ fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669334)
Some of these are folders, but this will also work.
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669334)
Some of these are folders, but this will also work.
š¬ fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669730)
Done.
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669730)
Done.
š¬ hulxv commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575220797)
> For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1
For that, I thought about switching first to `{fmt}` and after that, we can switch to `std::format` easily when we bump the minimum required GCC version
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575220797)
> For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1
For that, I thought about switching first to `{fmt}` and after that, we can switch to `std::format` easily when we bump the minimum required GCC version
š¬ maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575238806)
Yes, I understand, but there are no meaningful user-visible benefits:
* Performance does not matter, because formatting isn't done in hot loops
* A few KiB of binary size don't matter, because the binary is several MiB in size
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575238806)
Yes, I understand, but there are no meaningful user-visible benefits:
* Performance does not matter, because formatting isn't done in hot loops
* A few KiB of binary size don't matter, because the binary is several MiB in size
š¬ fanquake commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3575244180)
> I think having two options to control such a minor feature is unintuive and overkill. If that's what people like, no objection as it really isn't important. But my preference would be to put everything in the -asmap option:
I agree.
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3575244180)
> I think having two options to control such a minor feature is unintuive and overkill. If that's what people like, no objection as it really isn't important. But my preference would be to put everything in the -asmap option:
I agree.
š fanquake merged a pull request: "test: add `-alertnotify` test for large work invalid chain warning"
(https://github.com/bitcoin/bitcoin/pull/33893)
(https://github.com/bitcoin/bitcoin/pull/33893)
š¬ hulxv commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575297752)
But from the developers' view, I think having features like compile-time error-checking and easy API and [other features](https://github.com/fmtlib/fmt?tab=readme-ov-file#features) will be helpful to have. At least it will make it a little bit easier. Maybe this is a little thing, but nice to have
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575297752)
But from the developers' view, I think having features like compile-time error-checking and easy API and [other features](https://github.com/fmtlib/fmt?tab=readme-ov-file#features) will be helpful to have. At least it will make it a little bit easier. Maybe this is a little thing, but nice to have
š¬ maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575329532)
There is compile-time error-checking already implemented for tinyformat in this codebase, so there is also no developer benefit of using `std::format` or `{fmt}`.
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575329532)
There is compile-time error-checking already implemented for tinyformat in this codebase, so there is also no developer benefit of using `std::format` or `{fmt}`.
š¬ fanquake commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3575373453)
Please rebase this.
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3575373453)
Please rebase this.