📝 fqlx opened a pull request: "rpc/deprecateboolverbose Boolean verbosity is deprecated"
(https://github.com/bitcoin/bitcoin/pull/33288)
(https://github.com/bitcoin/bitcoin/pull/33288)
💬 ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3247577007)
Maybe it would make sense to have cmake symlink `.clang-tidy` files from the source directory to the build directory the same way it seems to symlink functional test files
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3247577007)
Maybe it would make sense to have cmake symlink `.clang-tidy` files from the source directory to the build directory the same way it seems to symlink functional test files
✅ maflcko closed an issue: "Increasing self-hosted runner raw performance"
(https://github.com/bitcoin/bitcoin/issues/30852)
(https://github.com/bitcoin/bitcoin/issues/30852)
💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-3247801501)
Closing for now. I think we'll go with https://github.com/bitcoin/bitcoin/issues/31965, which may also increase performance, so there is no longer a need to upgrade the self-hosted runners.
Feel free to continue discussion below. Also, this can be re-opened, or a new issue can be opened.
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-3247801501)
Closing for now. I think we'll go with https://github.com/bitcoin/bitcoin/issues/31965, which may also increase performance, so there is no longer a need to upgrade the self-hosted runners.
Feel free to continue discussion below. Also, this can be re-opened, or a new issue can be opened.
⚠️ maflcko opened an issue: "check_translations: Provide glossary as context"
(https://github.com/bitcoin/bitcoin/issues/33289)
> One false positive group of issues I noticed is the disagreement on translating terms.
> For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues don't show up, but fwiw I found Transifex's Glossary very interesting to help have long-running async consistency on term translations, but I don't see any option to export them i
...
(https://github.com/bitcoin/bitcoin/issues/33289)
> One false positive group of issues I noticed is the disagreement on translating terms.
> For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues don't show up, but fwiw I found Transifex's Glossary very interesting to help have long-running async consistency on term translations, but I don't see any option to export them i
...
✅ maflcko closed an issue: "check_translations: Provide glossary as context"
(https://github.com/bitcoin/bitcoin/issues/33289)
(https://github.com/bitcoin/bitcoin/issues/33289)
💬 maflcko commented on issue "check_translations: Provide glossary as context":
(https://github.com/bitcoin/bitcoin/issues/33289#issuecomment-3247818755)
sry
(https://github.com/bitcoin/bitcoin/issues/33289#issuecomment-3247818755)
sry
💬 maflcko commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3247874109)
BDB has many known and unknown issues, so clinging to it when one is worried about wallet issues, seems backward? The backend hasn't been maintained for years (decades?) upstream and many issues in Bitcoin Core weren't fixed either, due to the planned removal.
> In any case, it is understandable that users may not want to mess with upgrading their wallets.
I think it is beneficial to be cautious and deliberate around actions involving the wallet. For example, it can help to create backups to
...
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3247874109)
BDB has many known and unknown issues, so clinging to it when one is worried about wallet issues, seems backward? The backend hasn't been maintained for years (decades?) upstream and many issues in Bitcoin Core weren't fixed either, due to the planned removal.
> In any case, it is understandable that users may not want to mess with upgrading their wallets.
I think it is beneficial to be cautious and deliberate around actions involving the wallet. For example, it can help to create backups to
...
💬 TheCharlatan commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3247919097)
> did you mean that this was implemented? "Support could also mean an automatic migration to sqlite with an internal dependency free converter"
Yes.
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3247919097)
> did you mean that this was implemented? "Support could also mean an automatic migration to sqlite with an internal dependency free converter"
Yes.
💬 purpleKarrot commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2317931795)
I think this should be a [dependent option](https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html). Also, we should start prefixing all options to allow the project to be used as a subproject:
```cmake
cmake_dependent_option(BTCCORE_ENABLE_FUNCTIONAL_TESTS "Enable the functional tests" ON BUILD_TESTS OFF)
```
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2317931795)
I think this should be a [dependent option](https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html). Also, we should start prefixing all options to allow the project to be used as a subproject:
```cmake
cmake_dependent_option(BTCCORE_ENABLE_FUNCTIONAL_TESTS "Enable the functional tests" ON BUILD_TESTS OFF)
```
📝 Sjors opened 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.
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 maintenance it doesn't check the version
...
(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.
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 maintenance it doesn't check the version
...
💬 purpleKarrot commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318074319)
Here, you leave an empty `set_target_properties` call and remove it in the next commit. Just remove it directly.
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318074319)
Here, you leave an empty `set_target_properties` call and remove it in the next commit. Just remove it directly.
💬 purpleKarrot commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318069759)
You modify the TODO in one commit and then delete it in another. You way want to rewrite the git history so that the todo is removed directly.
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318069759)
You modify the TODO in one commit and then delete it in another. You way want to rewrite the git history so that the todo is removed directly.
💬 purpleKarrot commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318071503)
Same here. You modify the TODO in one commit and then delete it in the next one. Just delete it directly.
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2318071503)
Same here. You modify the TODO in one commit and then delete it in the next one. Just delete it directly.
💬 stickies-v commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2318090336)
> I think this should be a dependent option.
Why? Imo it's not unreasonable to want to build with only one of unit tests and functional tests, they don't depend on each other.
> Also, we should start prefixing all options to allow the project to be used as a subproject:
That sounds like a good idea, but pragmatically I think it makes sense to first open a separate issue, to discuss and get consensus on the goal and approach (e.g. prefix name), so I'm going to hold off on that here.
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2318090336)
> I think this should be a dependent option.
Why? Imo it's not unreasonable to want to build with only one of unit tests and functional tests, they don't depend on each other.
> Also, we should start prefixing all options to allow the project to be used as a subproject:
That sounds like a good idea, but pragmatically I think it makes sense to first open a separate issue, to discuss and get consensus on the goal and approach (e.g. prefix name), so I'm going to hold off on that here.
💬 Sjors commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248102026)
Very strange CI errors:
```
03:45:32.525] -- Configuring incomplete, errors occurred!
[03:45:32.543] ++ cmake -P /ci_container_base/ci/test/GetCMakeLogFiles.cmake
[03:45:32.586] + cat CMakeFiles/CMakeConfigureLog.yaml
[03:45:32.587] cat: CMakeFiles/CMakeConfigureLog.yaml: No such file or directory
[03:45:32.621] Command '['./ci/test/02_run_container.sh']' returned non-zero exit status 1.
```
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3248102026)
Very strange CI errors:
```
03:45:32.525] -- Configuring incomplete, errors occurred!
[03:45:32.543] ++ cmake -P /ci_container_base/ci/test/GetCMakeLogFiles.cmake
[03:45:32.586] + cat CMakeFiles/CMakeConfigureLog.yaml
[03:45:32.587] cat: CMakeFiles/CMakeConfigureLog.yaml: No such file or directory
[03:45:32.621] Command '['./ci/test/02_run_container.sh']' returned non-zero exit status 1.
```
💬 purpleKarrot commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2318148580)
> Imo it's not unreasonable to want to build with only one of unit tests and functional tests, they don't depend on each other.
They don't? `${BUILD_TESTS}` as the default value of the `BUILD_FUNCTIONAL_TESTS` option implies a dependency. If they are independent, maybe give them independent default values?
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2318148580)
> Imo it's not unreasonable to want to build with only one of unit tests and functional tests, they don't depend on each other.
They don't? `${BUILD_TESTS}` as the default value of the `BUILD_FUNCTIONAL_TESTS` option implies a dependency. If they are independent, maybe give them independent default values?
💬 0xB10C 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-3248190718)
> 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'`?
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3248190718)
> 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'`?
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248203281)
@cryptomeow
> Great! Regarding the Greek translation, I've gone one round and did some fixes based on the review and it certainly is helpful and provides actionable issues.
>
> One false positive group of issues I noticed is the disagreement on translating terms.
>
> For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248203281)
@cryptomeow
> Great! Regarding the Greek translation, I've gone one round and did some fixes based on the review and it certainly is helpful and provides actionable issues.
>
> One false positive group of issues I noticed is the disagreement on translating terms.
>
> For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues
...
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248219824)
@l0rinc
> @cryptomeow you can check out this PR having the translations in an XML format and feed the whole file to an LLM. http://aistudio.google.com let's you work with multiple complete translations. You could ask it to extract the key/value pairs you want and feed _that_ to the other LLM of yours choice.
This is an interesting idea.
However, I don’t think it’s reasonable to ask volunteer coordinators to review the entire PR.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3248219824)
@l0rinc
> @cryptomeow you can check out this PR having the translations in an XML format and feed the whole file to an LLM. http://aistudio.google.com let's you work with multiple complete translations. You could ask it to extract the key/value pairs you want and feed _that_ to the other LLM of yours choice.
This is an interesting idea.
However, I don’t think it’s reasonable to ask volunteer coordinators to review the entire PR.