💬 maflcko commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248896883)
> Also, those builds still break.
That is expected. https://github.com/bitcoin/bitcoin/commit/ed95f4f095fecb0a6e5601199959db21e811dd71 must not interfere with unrelated issues. The reason why your build fails is because you are adding the targets twice. (You can see this by reading the log)
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248896883)
> Also, those builds still break.
That is expected. https://github.com/bitcoin/bitcoin/commit/ed95f4f095fecb0a6e5601199959db21e811dd71 must not interfere with unrelated issues. The reason why your build fails is because you are adding the targets twice. (You can see this by reading the log)
👍 hodlinator approved a pull request: "macdeploy: avoid use of `Bitcoin Core` in Linux cross build"
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3180244945)
ACK 921c6ba251494b4970025166a3e25cf20789da66
Good to rename `osx_volname` to `macos_zip` since it's more modern naming of the OS.
Guix output:
```
[100%] Generating dist/bitcoin-macos-app.zip
adding: Bitcoin-Qt.app/ (stored 0%)
...
[100%] Built target deploy
```
Hashes:
```
f15abfa32b63a5b007d52fb31ac148980570b77cc3be12674debdca65d092706 guix-build-921c6ba25149/output/arm64-apple-darwin/SHA256SUMS.part
b94a1863fff232cdd81f2ad4b729eae9242f80d6493afea6c7c02e871658629f guix-
...
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3180244945)
ACK 921c6ba251494b4970025166a3e25cf20789da66
Good to rename `osx_volname` to `macos_zip` since it's more modern naming of the OS.
Guix output:
```
[100%] Generating dist/bitcoin-macos-app.zip
adding: Bitcoin-Qt.app/ (stored 0%)
...
[100%] Built target deploy
```
Hashes:
```
f15abfa32b63a5b007d52fb31ac148980570b77cc3be12674debdca65d092706 guix-build-921c6ba25149/output/arm64-apple-darwin/SHA256SUMS.part
b94a1863fff232cdd81f2ad4b729eae9242f80d6493afea6c7c02e871658629f guix-
...
💬 hodlinator commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#discussion_r2318703303)
nit: Please remove empty newline at the beginning of the block if you retouch.
(https://github.com/bitcoin/bitcoin/pull/33158#discussion_r2318703303)
nit: Please remove empty newline at the beginning of the block if you retouch.
💬 fanquake commented on issue "GUI (?): Copying output from console causes large mem usage/OOM":
(https://github.com/bitcoin/bitcoin/issues/33285#issuecomment-3248931861)
I guess if it's known and has just never been fixed, maybe we should add a warning that copying from the console (too much?) might crash your node.
(https://github.com/bitcoin/bitcoin/issues/33285#issuecomment-3248931861)
I guess if it's known and has just never been fixed, maybe we should add a warning that copying from the console (too much?) might crash your node.
💬 fanquake commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#discussion_r2318743803)
Dropped.
(https://github.com/bitcoin/bitcoin/pull/33158#discussion_r2318743803)
Dropped.
👍 hodlinator approved a pull request: "macdeploy: avoid use of `Bitcoin Core` in Linux cross build"
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3180311911)
ACK 8e434a84999c473a7295772a346cbce27888d28e
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3180311911)
ACK 8e434a84999c473a7295772a346cbce27888d28e
💬 jmoik commented on issue "build: secp256k1 warnings not turned into errors in MSAN job":
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3248965317)
I will take a look at this
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3248965317)
I will take a look at this
💬 maflcko commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3249007687)
tested ACK c0d28c8f5b150a03de75155a0961b3d9b2695ed6 💳
<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: tested ACK c0d28c8f
...
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3249007687)
tested ACK c0d28c8f5b150a03de75155a0961b3d9b2695ed6 💳
<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: tested ACK c0d28c8f
...
💬 maflcko commented on issue "build: secp256k1 warnings not turned into errors in MSAN job":
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249024368)
The reason is that Werror is not passed down, so I guess the question is: What flags should be passed down under what circumstances?
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249024368)
The reason is that Werror is not passed down, so I guess the question is: What flags should be passed down under what circumstances?
📝 Sjors opened a pull request: "ci: cd into BASE_BUILD_DIR for GetCMakeLogFiles"
(https://github.com/bitcoin/bitcoin/pull/33291)
When a bug is introduced in cmake, we render its logs, which was broken:
https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248645770
(https://github.com/bitcoin/bitcoin/pull/33291)
When a bug is introduced in cmake, we render its logs, which was broken:
https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248645770
💬 Sjors commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249033527)
@maflcko the log is very verbose, but in indeed useful, so I opened #33291.
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249033527)
@maflcko the log is very verbose, but in indeed useful, so I opened #33291.
📝 Sjors converted_to_draft a pull request: "build: suggest -DENABLE_IPC=OFF when missing capnp"
(https://github.com/bitcoin/bitcoin/pull/33290)
Since #31802, when existing users upgrade to a recent version of master, or the upcoming v30 release, they'll be treated by an error that CapnProto is missing. Unless they read the release notes :-)
This error is generated by `src/ipc/libmultiprocess/CMakeLists.txt` which is a git subtree and so it doesn't have context of our project, and doesn't know about the `-DENABLE_IPC` option.
This pull request adds a simple pre-check in own CMake file to see if Cap'n Proto is missing. For ease of m
...
(https://github.com/bitcoin/bitcoin/pull/33290)
Since #31802, when existing users upgrade to a recent version of master, or the upcoming v30 release, they'll be treated by an error that CapnProto is missing. Unless they read the release notes :-)
This error is generated by `src/ipc/libmultiprocess/CMakeLists.txt` which is a git subtree and so it doesn't have context of our project, and doesn't know about the `-DENABLE_IPC` option.
This pull request adds a simple pre-check in own CMake file to see if Cap'n Proto is missing. For ease of m
...
💬 fanquake commented on issue "build: secp256k1 warnings not turned into errors in MSAN job":
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249065390)
My expectation would be that the CI shouldn't produce any spurious output, and any unexpected output would be an error. That way, any output is either fixed, or documented/suppressed. Otherwise anyone looking at the CI can't know what is expected, what should be fixed, or what doesn't matter, and we avoid issues like this, #33256, etc.
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249065390)
My expectation would be that the CI shouldn't produce any spurious output, and any unexpected output would be an error. That way, any output is either fixed, or documented/suppressed. Otherwise anyone looking at the CI can't know what is expected, what should be fixed, or what doesn't matter, and we avoid issues like this, #33256, etc.
💬 maflcko commented on pull request "ci: cd into BASE_BUILD_DIR for GetCMakeLogFiles":
(https://github.com/bitcoin/bitcoin/pull/33291#issuecomment-3249067521)
I haven't checked, but it seems plausible that I introduced this bug in commit fad191ff48b15832a90c19d560a7c0525c146be3, which fixed two other bugs.
Regardless, it seems better to specify the dir explicitly, than to silently/implicitly depend on it.
lgtm ACK 9b76eef2d2b42703e2a30952d4c3474b533e360a 💽
<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}" -
...
(https://github.com/bitcoin/bitcoin/pull/33291#issuecomment-3249067521)
I haven't checked, but it seems plausible that I introduced this bug in commit fad191ff48b15832a90c19d560a7c0525c146be3, which fixed two other bugs.
Regardless, it seems better to specify the dir explicitly, than to silently/implicitly depend on it.
lgtm ACK 9b76eef2d2b42703e2a30952d4c3474b533e360a 💽
<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}" -
...
🤔 janb84 reviewed a pull request: "build: set ENABLE_IPC to OFF when fuzzing"
(https://github.com/bitcoin/bitcoin/pull/33235#pullrequestreview-3180445284)
ACK af4156ab75565acc5a71b68e70da5e2cf407df80
PR sets the option `ENABLE_IPC` to `OFF` when building for fuzzing. This seems reasonable for now, because we do not have any fuzz testing on IPC code. Setting ENABLE_IPC` to `OFF` will reduce a dependency (capnp).
- build and tested ✅
(https://github.com/bitcoin/bitcoin/pull/33235#pullrequestreview-3180445284)
ACK af4156ab75565acc5a71b68e70da5e2cf407df80
PR sets the option `ENABLE_IPC` to `OFF` when building for fuzzing. This seems reasonable for now, because we do not have any fuzz testing on IPC code. Setting ENABLE_IPC` to `OFF` will reduce a dependency (capnp).
- build and tested ✅
💬 jmoik commented on issue "build: secp256k1 warnings not turned into errors in MSAN job":
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249078560)
from CmakeLists.txt:
# The core_interface library aims to encapsulate common build flags.
# It is a usage requirement for all targets except for secp256k1, which
# gets its flags by other means.
We can simply add a section for WERROR to the cmake file just like for other flags like BUILD_TESTS, to inherit the WERROR setting.
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3249078560)
from CmakeLists.txt:
# The core_interface library aims to encapsulate common build flags.
# It is a usage requirement for all targets except for secp256k1, which
# gets its flags by other means.
We can simply add a section for WERROR to the cmake file just like for other flags like BUILD_TESTS, to inherit the WERROR setting.
💬 jesterhodl commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3249083029)
> checkpoints
Don't see it yet, I suppose it needs a sync?
<img width="587" height="187" alt="image" src="https://github.com/user-attachments/assets/9efcb5a1-8b8c-4544-ab53-af574f2784c7" />
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3249083029)
> checkpoints
Don't see it yet, I suppose it needs a sync?
<img width="587" height="187" alt="image" src="https://github.com/user-attachments/assets/9efcb5a1-8b8c-4544-ab53-af574f2784c7" />
💬 maflcko commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249086372)
> @maflcko the log is very verbose, but in indeed useful, so I opened #33291.
for reference, this specific warning was also printed without the verbose log: https://cirrus-ci.com/task/4516741763563520?logs=ci#L894 :
```
...
[03:43:58.798] CMake Error at /usr/lib/x86_64-linux-gnu/cmake/CapnProto/CapnProtoTargets.cmake:213 (add_executable):
[03:43:58.798] add_executable cannot create imported target "CapnProto::capnpc_cpp"
[03:43:58.798] because another target with the same name alre
...
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249086372)
> @maflcko the log is very verbose, but in indeed useful, so I opened #33291.
for reference, this specific warning was also printed without the verbose log: https://cirrus-ci.com/task/4516741763563520?logs=ci#L894 :
```
...
[03:43:58.798] CMake Error at /usr/lib/x86_64-linux-gnu/cmake/CapnProto/CapnProtoTargets.cmake:213 (add_executable):
[03:43:58.798] add_executable cannot create imported target "CapnProto::capnpc_cpp"
[03:43:58.798] because another target with the same name alre
...
💬 theStack commented on issue "tracing: test `interface_usdt_net.py` fails due to garbage message type in `net:outbound_message` tracepoint":
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3249097001)
> > ```
> > Arguments: -8@x21 8@x24 8@[sp, 136] 8@[x0] 8@x1 8@x2
> > ```
>
> Are these the same if you build with `-DAPPEND_CXXFLAGS='-fno-inline'`?
They are quite different on a `-fno-inline` build:
```
Name: outbound_message
Location: 0x000000000012ed28, Base: 0x000000000111c6d8, Semaphore: 0x00000000016594d0
Arguments: -8@x23 8@x24 8@x25 8@x26 8@x27 8@x0
```
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3249097001)
> > ```
> > Arguments: -8@x21 8@x24 8@[sp, 136] 8@[x0] 8@x1 8@x2
> > ```
>
> Are these the same if you build with `-DAPPEND_CXXFLAGS='-fno-inline'`?
They are quite different on a `-fno-inline` build:
```
Name: outbound_message
Location: 0x000000000012ed28, Base: 0x000000000111c6d8, Semaphore: 0x00000000016594d0
Arguments: -8@x23 8@x24 8@x25 8@x26 8@x27 8@x0
```
💬 Sjors commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249117834)
Using `find_program` instead of `find_package` as suggested by an AI overlord, seems to do the trick.
Another issue was that the depends build also threw this warning, which I added a check for.
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249117834)
Using `find_program` instead of `find_package` as suggested by an AI overlord, seems to do the trick.
Another issue was that the depends build also threw this warning, which I added a check for.