💬 Sjors commented on pull request "kernel: make m_tip_block std::optional":
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851750132)
I didn't think about it that deeply, but yes.
(https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851750132)
I didn't think about it that deeply, but yes.
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851750952)
Dropped the workaround, and it seems like using `pkgconf` is still broken, so we'd need to retain the workaround. We could also just update only the docs, and change the CI later if it starts breaking again.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851750952)
Dropped the workaround, and it seems like using `pkgconf` is still broken, so we'd need to retain the workaround. We could also just update only the docs, and change the CI later if it starts breaking again.
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851765882)
I'd presume you could only update the CI after GHA has updated their image?
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851765882)
I'd presume you could only update the CI after GHA has updated their image?
💬 hebasto commented on pull request "build: Use `-ffile-prefix-map` in release builds only":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490733585)
> I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
They [use](https://github.com/google/oss-fuzz/blob/51922e43fa80ae858fcd12aa84be3c0c75ed7b0d/infra/base-images/base-runner/coverage#L54-L55) the `-path-equivalence=...` option:
```bash
# Use path mapping, as $SRC directory from the builder is copied into $OUT/$SRC.
PATH_EQUIVALENCE_ARGS="-path-equivalence=/,$OUT"
```
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490733585)
> I'm slightly confused why oss-fuzz broke while my setup keeps working. Afaict I'm using the same flags and `llvm-*` tools as them...
They [use](https://github.com/google/oss-fuzz/blob/51922e43fa80ae858fcd12aa84be3c0c75ed7b0d/infra/base-images/base-runner/coverage#L54-L55) the `-path-equivalence=...` option:
```bash
# Use path mapping, as $SRC directory from the builder is copied into $OUT/$SRC.
PATH_EQUIVALENCE_ARGS="-path-equivalence=/,$OUT"
```
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270)
Probably. Added a note for that, dropping the package change, but now the build is failing again, with the same as #31334.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270)
Probably. Added a note for that, dropping the package change, but now the build is failing again, with the same as #31334.
💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851796441)
Or we can `brew update`. However, it might be time consuming.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851796441)
Or we can `brew update`. However, it might be time consuming.
💬 hodlinator commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851804865)
Still worried that this will backfire, but will investigate more and maybe spawn a new PR.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851804865)
Still worried that this will backfire, but will investigate more and maybe spawn a new PR.
💬 hodlinator commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851810120)
FYI: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp overloads 21-33
Okay with leaving as-is.
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1851810120)
FYI: https://en.cppreference.com/w/cpp/utility/optional/operator_cmp overloads 21-33
Okay with leaving as-is.
💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851825568)
It seems that `unlink` is still required because the `pkg-config` package is preinstalled in the GHA image.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851825568)
It seems that `unlink` is still required because the `pkg-config` package is preinstalled in the GHA image.
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490781666)
This is the upstream issue https://github.com/actions/runner-images/issues/10984 and related fix: https://github.com/actions/runner-images/pull/11015.
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490781666)
This is the upstream issue https://github.com/actions/runner-images/issues/10984 and related fix: https://github.com/actions/runner-images/pull/11015.
💬 fanquake commented on pull request "scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/29641#discussion_r1851843891)
forgot to add pkgconf?
(https://github.com/bitcoin/bitcoin/pull/29641#discussion_r1851843891)
forgot to add pkgconf?
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851847694)
Though, I am wondering if `brew unlink pkg-config` will start failing once pkg-config is removed from the GHA image?
Wouldn't a better fix be to drop the `unlink` and the install and add a comment to add `install pkgconf` in the future?
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851847694)
Though, I am wondering if `brew unlink pkg-config` will start failing once pkg-config is removed from the GHA image?
Wouldn't a better fix be to drop the `unlink` and the install and add a comment to add `install pkgconf` in the future?
👍 hebasto approved a pull request: "macOS: swap docs & CI from pkg-config to pkgconf"
(https://github.com/bitcoin/bitcoin/pull/31335#pullrequestreview-2451039128)
ACK 278687dcc010d0a826e55b864164cf42beddff46.
(https://github.com/bitcoin/bitcoin/pull/31335#pullrequestreview-2451039128)
ACK 278687dcc010d0a826e55b864164cf42beddff46.
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851849029)
> Wouldn't a better fix be to drop the unlink and the install and add a comment to add install pkgconf in the future?
See https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851849029)
> Wouldn't a better fix be to drop the unlink and the install and add a comment to add install pkgconf in the future?
See https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270.
📝 hodlinator opened a pull request: "test: Deduplicate assert_mempool_contents()"
(https://github.com/bitcoin/bitcoin/pull/31338)
Recently added `mempool_util` implementation probably evolved in parallel with the package RBF one before being submitted as part of ephemeral dust in e2e30e89ba4b9bdbcabaf5b4346610922f0728bb (despite feedback: https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825278134, https://github.com/bitcoin/bitcoin/pull/31279#pullrequestreview-2445579323).
(https://github.com/bitcoin/bitcoin/pull/31338)
Recently added `mempool_util` implementation probably evolved in parallel with the package RBF one before being submitted as part of ephemeral dust in e2e30e89ba4b9bdbcabaf5b4346610922f0728bb (despite feedback: https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825278134, https://github.com/bitcoin/bitcoin/pull/31279#pullrequestreview-2445579323).
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851869449)
> > Wouldn't a better fix be to drop the unlink and the install and add a comment to add install pkgconf in the future?
>
> See [#31335 (comment)](https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270).
IIRC you still tried to install. However, my recommendation is to do neither. Otherwise, either can fail, depending on the state of the GHA image.
I tested this in https://github.com/bitcoin/bitcoin/commit/4191e4a34c7f91fe1dfc68baa91998067a144619 and it passed. What am I m
...
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851869449)
> > Wouldn't a better fix be to drop the unlink and the install and add a comment to add install pkgconf in the future?
>
> See [#31335 (comment)](https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851793270).
IIRC you still tried to install. However, my recommendation is to do neither. Otherwise, either can fail, depending on the state of the GHA image.
I tested this in https://github.com/bitcoin/bitcoin/commit/4191e4a34c7f91fe1dfc68baa91998067a144619 and it passed. What am I m
...
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851871250)
Nothing, the suggested change is already in this PR.
(https://github.com/bitcoin/bitcoin/pull/31335#discussion_r1851871250)
Nothing, the suggested change is already in this PR.
💬 maflcko commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490854220)
ACK fe3457ccfff9a022d9f183e18217422e2e1f7689 🍭
<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 fe3457ccfff9a022d9f183e182
...
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2490854220)
ACK fe3457ccfff9a022d9f183e18217422e2e1f7689 🍭
<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 fe3457ccfff9a022d9f183e182
...
👍 l0rinc approved a pull request: "refactor: Prepare compile-time check of bilingual format strings"
(https://github.com/bitcoin/bitcoin/pull/31295#pullrequestreview-2450986722)
ACK fa3e074304780549b1e7972217930e34fa55f59a
Lots of versions for this change, loosely coupled, but I understand they will simplify follow-ups.
I have validated manually that:
* `run-lint-format-strings.py` needs the exception 👍
* all code moves are same as before 👍
* tidy warns for `feebumper.cpp`, `salvage.cpp` and `wallet.cpp` 👎 (couldn't reproduce any of them, but manually checked it and at least it shouldn't be worse than before)
(https://github.com/bitcoin/bitcoin/pull/31295#pullrequestreview-2450986722)
ACK fa3e074304780549b1e7972217930e34fa55f59a
Lots of versions for this change, loosely coupled, but I understand they will simplify follow-ups.
I have validated manually that:
* `run-lint-format-strings.py` needs the exception 👍
* all code moves are same as before 👍
* tidy warns for `feebumper.cpp`, `salvage.cpp` and `wallet.cpp` 👎 (couldn't reproduce any of them, but manually checked it and at least it shouldn't be worse than before)
💬 l0rinc commented on pull request "refactor: Prepare compile-time check of bilingual format strings":
(https://github.com/bitcoin/bitcoin/pull/31295#discussion_r1851812567)
I tried reproducing the tidy warnings with
```bash
$ clang++ --version
Homebrew clang version 19.1.3
$ cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
&& cmake --build build -j$(nproc) \
&& run-clang-tidy -p build -j$(nproc) -checks='-*,modernize-use-emplace' src/wallet/feebumper.cpp
```
But it doesn't give me the mentioned warnings.
The change itself does make sense (forwarding the bilingual_str params directly), I just can'
...
(https://github.com/bitcoin/bitcoin/pull/31295#discussion_r1851812567)
I tried reproducing the tidy warnings with
```bash
$ clang++ --version
Homebrew clang version 19.1.3
$ cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
&& cmake --build build -j$(nproc) \
&& run-clang-tidy -p build -j$(nproc) -checks='-*,modernize-use-emplace' src/wallet/feebumper.cpp
```
But it doesn't give me the mentioned warnings.
The change itself does make sense (forwarding the bilingual_str params directly), I just can'
...