💬 maflcko commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407190445)
If this really was an issue, adding back the build patches would probably be easier than backporting all bugfixes for years: https://github.com/bitcoin/bitcoin/commit/0c57a798b50bf03a0b69f632c691932d608254ef#diff-42e88f6b2dc3f513ccde184c74d1853a01b2bfc3ee64debf98c52efe316b04c1L456
However, I still don't understand why this is an issue.
Maybe I am missing something obvious, but at least locally I can call `./configure` on the `28.x` branch just fine on Alma8:
```
dnf install gcc-toolset
...
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407190445)
If this really was an issue, adding back the build patches would probably be easier than backporting all bugfixes for years: https://github.com/bitcoin/bitcoin/commit/0c57a798b50bf03a0b69f632c691932d608254ef#diff-42e88f6b2dc3f513ccde184c74d1853a01b2bfc3ee64debf98c52efe316b04c1L456
However, I still don't understand why this is an issue.
Maybe I am missing something obvious, but at least locally I can call `./configure` on the `28.x` branch just fine on Alma8:
```
dnf install gcc-toolset
...
📝 ryanofsky opened a pull request: "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))"
(https://github.com/bitcoin/bitcoin/pull/31072)
This PR is just a scripted-diff replacing strprintf calls like:
```c++
strprintf(Untranslated("Error parsing command line arguments: %s"), error))
```
with:
```c++
Untranslated(strprintf("Error parsing command line arguments: %s", error))
```
Currently code is inconsistent and uses a mix of both styles, but second style is better because:
1. It scans more cleanly. It is easier to read the strprintf call, and easier to think about because the whole string, not just one part, is
...
(https://github.com/bitcoin/bitcoin/pull/31072)
This PR is just a scripted-diff replacing strprintf calls like:
```c++
strprintf(Untranslated("Error parsing command line arguments: %s"), error))
```
with:
```c++
Untranslated(strprintf("Error parsing command line arguments: %s", error))
```
Currently code is inconsistent and uses a mix of both styles, but second style is better because:
1. It scans more cleanly. It is easier to read the strprintf call, and easier to think about because the whole string, not just one part, is
...
💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1796886224)
Thanks! Reworked per feedback.
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1796886224)
Thanks! Reworked per feedback.
💬 hebasto commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407322051)
> Are you saying you think we shouldn't be able to include system headers when building using depends?
Yes, for headers-only packages built in depends, such as Boost and USDT. t's for the same reason, why we got rid of the `ALLOW_HOST_PACKAGES` variable.
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407322051)
> Are you saying you think we shouldn't be able to include system headers when building using depends?
Yes, for headers-only packages built in depends, such as Boost and USDT. t's for the same reason, why we got rid of the `ALLOW_HOST_PACKAGES` variable.
💬 fanquake commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407334296)
> Yes, for headers-only packages built in depends, such as Boost and USDT.
Headers-only shouldn't make any difference here, and this should already be the case for Boost. If it's not, that should be fixed. See my point above re USDT, I don't understand how you propose to do this, given that the USDT headers exist amongst the libc headers.
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2407334296)
> Yes, for headers-only packages built in depends, such as Boost and USDT.
Headers-only shouldn't make any difference here, and this should already be the case for Boost. If it's not, that should be fixed. See my point above re USDT, I don't understand how you propose to do this, given that the USDT headers exist amongst the libc headers.
👍 fanquake approved a pull request: "build: Add missing USDT header dependency to kernel"
(https://github.com/bitcoin/bitcoin/pull/30970#pullrequestreview-2362721686)
ACK ccd10fdb97f9b8268a5cd60d7461967cfe536f16
(https://github.com/bitcoin/bitcoin/pull/30970#pullrequestreview-2362721686)
ACK ccd10fdb97f9b8268a5cd60d7461967cfe536f16
💬 ArmchairCryptologist commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407340807)
> Do you have any further information on how you conclude that more than half of Bitcoin Core installations run on these OSes (and their derivatives)? This number doesn't feel right to me, even as a (high) estimate.
> This report: https://repology.org/project/glibc/badges shows that glibc >=2.31 is supported by:
It is mostly a hunch based on my own experience with the server hosting industry. Most of the distros mentioned here are not generally considered stable for server usage, and are onl
...
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407340807)
> Do you have any further information on how you conclude that more than half of Bitcoin Core installations run on these OSes (and their derivatives)? This number doesn't feel right to me, even as a (high) estimate.
> This report: https://repology.org/project/glibc/badges shows that glibc >=2.31 is supported by:
It is mostly a hunch based on my own experience with the server hosting industry. Most of the distros mentioned here are not generally considered stable for server usage, and are onl
...
🚀 fanquake merged a pull request: "build: Add missing USDT header dependency to kernel"
(https://github.com/bitcoin/bitcoin/pull/30970)
(https://github.com/bitcoin/bitcoin/pull/30970)
💬 sipa commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407347013)
*If* there is significant demand for running Bitcoin Core on RH8, I think it would make sense for us to consider adding the glibc workarounds necessary so that guix-built binaries support glibc 2.28. I suspect that is easier than backporting fixes to 27.x for longer, and doesn't preclude RH8 users from getting the features added in 28.x and later.
I don't think the possibility of self-compiling on RH8 is a full replacement for project binaries that just work. Presumably people who use RH8 want
...
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407347013)
*If* there is significant demand for running Bitcoin Core on RH8, I think it would make sense for us to consider adding the glibc workarounds necessary so that guix-built binaries support glibc 2.28. I suspect that is easier than backporting fixes to 27.x for longer, and doesn't preclude RH8 users from getting the features added in 28.x and later.
I don't think the possibility of self-compiling on RH8 is a full replacement for project binaries that just work. Presumably people who use RH8 want
...
💬 hebasto commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2407347771)
Concept ACK. May be useful for switching to [Qt 6.8 or newer.](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346)
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2407347771)
Concept ACK. May be useful for switching to [Qt 6.8 or newer.](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346)
💬 fanquake commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407347872)
> if it still trivially compiles against it.
Having to maintain support for compiling well EOL glibcs is not trivial, and continues to get harder as the compilers and toolchains used by the project modernise. It's already the case that on the next update to our release (Guix) build environment, our current glibc (2.31), will stop compiling without further patching, because the newer binutils (2.41), can't compile it when targeting riscv.
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407347872)
> if it still trivially compiles against it.
Having to maintain support for compiling well EOL glibcs is not trivial, and continues to get harder as the compilers and toolchains used by the project modernise. It's already the case that on the next update to our release (Guix) build environment, our current glibc (2.31), will stop compiling without further patching, because the newer binutils (2.41), can't compile it when targeting riscv.
💬 sipa commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407368227)
@ArmchairCryptologist There is a difference between (1) our source code being able to be compiled against an old glibc, and then working on that old glibc and (2) the release process being able to run on modern platforms (and making use of the features that offers, including targetting modern hardware) and the resulting binaries working on old systems.
Of course, one possibility would be to have multiple release builds, for example a separate one targetting old-glibc x86_64 systems only. So far
...
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407368227)
@ArmchairCryptologist There is a difference between (1) our source code being able to be compiled against an old glibc, and then working on that old glibc and (2) the release process being able to run on modern platforms (and making use of the features that offers, including targetting modern hardware) and the resulting binaries working on old systems.
Of course, one possibility would be to have multiple release builds, for example a separate one targetting old-glibc x86_64 systems only. So far
...
💬 maflcko commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407391721)
> for example a separate one targetting old-glibc x86_64
Looking at the patches, it may be possible to do a x86_64 guix build (maybe even aarch64) with glibc1.28, without adding any patches back, as they relate to other architectures. However, I haven't tried this and don't know how involved it would be.
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407391721)
> for example a separate one targetting old-glibc x86_64
Looking at the patches, it may be possible to do a x86_64 guix build (maybe even aarch64) with glibc1.28, without adding any patches back, as they relate to other architectures. However, I haven't tried this and don't know how involved it would be.
💬 jonatack commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2407408687)
Thanks!
> I think we should just unify `settxfee` and your `setfeerate` into one RPC?
I'm not sure. Point 3 and the rationales in https://github.com/bitcoin/bitcoin/pull/20484#issuecomment-734786305 suggest a possibly safer way (perhaps you have a workaround in mind I'm overlooking).
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2407408687)
Thanks!
> I think we should just unify `settxfee` and your `setfeerate` into one RPC?
I'm not sure. Point 3 and the rationales in https://github.com/bitcoin/bitcoin/pull/20484#issuecomment-734786305 suggest a possibly safer way (perhaps you have a workaround in mind I'm overlooking).
💬 sipa commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407411552)
@maflcko I assume you mean glibc 2.28 and 2.27? What would be the downside of only having glibc 2.27 release binaries for x86_64 (and/or aarch64)?
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407411552)
@maflcko I assume you mean glibc 2.28 and 2.27? What would be the downside of only having glibc 2.27 release binaries for x86_64 (and/or aarch64)?
💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2407431612)
> from C++ one has greater control and observability - the test can read any global or member variable and can call arbitrary functions
> Also, when used from within C++ one does not have to re-implement the transport code in Python. From C++ we can use the existent code to parse or craft the socket data.
For these reasons, along with the speed of running the tests and the simplicity and robustness of using the same language for tests and code, I prefer to write test code in C++ when I can
...
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2407431612)
> from C++ one has greater control and observability - the test can read any global or member variable and can call arbitrary functions
> Also, when used from within C++ one does not have to re-implement the transport code in Python. From C++ we can use the existent code to parse or craft the socket data.
For these reasons, along with the speed of running the tests and the simplicity and robustness of using the same language for tests and code, I prefer to write test code in C++ when I can
...
💬 jonatack commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2407438275)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2407438275)
Concept ACK
💬 maflcko commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2407451391)
> Wrt "how useful is this": the first part of this PR (also isolated in #30205) is already used in Sv2Transport tests.
I followed the links but only found a closed pull requests: https://github.com/bitcoin/bitcoin/pull/30332#event-13580183288
I think it is easy to say that something will be used in the future, but I think it would be easier if the framework commits were added with non-redundant tests at the same time.
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-2407451391)
> Wrt "how useful is this": the first part of this PR (also isolated in #30205) is already used in Sv2Transport tests.
I followed the links but only found a closed pull requests: https://github.com/bitcoin/bitcoin/pull/30332#event-13580183288
I think it is easy to say that something will be used in the future, but I think it would be easier if the framework commits were added with non-redundant tests at the same time.
👍 ryanofsky approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2362745062)
Code review ACK fae6acacc0216aa07f5a4a0ee75fbade620d7023. This is a nice improvement overall, and definitely better than the status quo due to extra type checking it provides. I do think there is extra complexity here and extra changes that could go away if we adopt #31072 and stop passing untranslated format strings to strprintf.
I also think it probably would be nicer if `_()` would continue to look up translations right away, instead of delaying them until conversion to bilingual str. I t
...
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2362745062)
Code review ACK fae6acacc0216aa07f5a4a0ee75fbade620d7023. This is a nice improvement overall, and definitely better than the status quo due to extra type checking it provides. I do think there is extra complexity here and extra changes that could go away if we adopt #31072 and stop passing untranslated format strings to strprintf.
I also think it probably would be nicer if `_()` would continue to look up translations right away, instead of delaying them until conversion to bilingual str. I t
...
💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1796975768)
In commit "refactor: Delay translation of Untranslated() or _() literals" (fa62737ae398bd56ef9e68b8d0a4d217cfcc532b)
Thinking about this more it seems like if `_` function does not be consteval, and could just a template type that inherits from `bilingual_str`, and included the number of format specifiers included in the string in its type information. This would be nice because runtime semantics of `_` would be unchanged, it would still do the translation at time it is called, and there woul
...
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1796975768)
In commit "refactor: Delay translation of Untranslated() or _() literals" (fa62737ae398bd56ef9e68b8d0a4d217cfcc532b)
Thinking about this more it seems like if `_` function does not be consteval, and could just a template type that inherits from `bilingual_str`, and included the number of format specifiers included in the string in its type information. This would be nice because runtime semantics of `_` would be unchanged, it would still do the translation at time it is called, and there woul
...