💬 hebasto commented on issue "depends: fallback server missing Qt downloads":
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3559121324)
> Seems like `https://code.qt.io` also doesn't work with (some?) VPN providers, so we should probably move away from using that.
I can see two options:
1. Switch to https://raw.githubusercontent.com/qt/qt5.
2. Treat the three files as patches.
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3559121324)
> Seems like `https://code.qt.io` also doesn't work with (some?) VPN providers, so we should probably move away from using that.
I can see two options:
1. Switch to https://raw.githubusercontent.com/qt/qt5.
2. Treat the three files as patches.
💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3559158385)
> Since `DBHeightKey` and `DBHashKey` are pretty similar. What do you think about a generic template for both? [furszy@e5dd2b0](https://github.com/furszy/bitcoin-core/commit/e5dd2b02f15f9f68fbe84cc30255573ad23dbf2c). (Only needed to specialize the serialization for the value)
That's clever, but I'm not convinced it is worth the downside of more complex / less readable code - we just have two different Key types, and I don't think it's likely that there will be other added in the future. Thoug
...
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3559158385)
> Since `DBHeightKey` and `DBHashKey` are pretty similar. What do you think about a generic template for both? [furszy@e5dd2b0](https://github.com/furszy/bitcoin-core/commit/e5dd2b02f15f9f68fbe84cc30255573ad23dbf2c). (Only needed to specialize the serialization for the value)
That's clever, but I'm not convinced it is worth the downside of more complex / less readable code - we just have two different Key types, and I don't think it's likely that there will be other added in the future. Thoug
...
💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3559168135)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3559168135)
Rebased.
💬 hebasto commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3559173941)
Friendly ping @davidgumberg @hodlinator :)
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3559173941)
Friendly ping @davidgumberg @hodlinator :)
💬 enirox001 commented on pull request "test: Retry download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/33915#discussion_r2546937787)
nano nit: The single retry seems reasonable to start with. If CI flakiness persists after this change, increasing to 2-3 retries or adding exponential backoff can be considered
(https://github.com/bitcoin/bitcoin/pull/33915#discussion_r2546937787)
nano nit: The single retry seems reasonable to start with. If CI flakiness persists after this change, increasing to 2-3 retries or adding exponential backoff can be considered
✅ fanquake closed an issue: "ci: windows-native-dll-vcpkg-* cache does not work?"
(https://github.com/bitcoin/bitcoin/issues/33685)
(https://github.com/bitcoin/bitcoin/issues/33685)
🚀 fanquake merged a pull request: "ci: Consistenly only cache on the default branch"
(https://github.com/bitcoin/bitcoin/pull/33905)
(https://github.com/bitcoin/bitcoin/pull/33905)
🤔 enirox001 reviewed a pull request: "test: Retry download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3488992622)
ACK fad06f3
This looks good to me. I tested locally by running the script with
valid releases (v25.0, v24.0.1) and an invalid one (v0.1.0) to verify
the retry logic works correctly. The retry mechanism should significantly
reduce CI failures from transient network issues.
optional nit: Consider adding an explicit timeout to `urlopen()`
(e.g., `timeout=60`) to fail faster in case of network hangs, rather
than relying on OS socket defaults. This would make the script more
predict
...
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3488992622)
ACK fad06f3
This looks good to me. I tested locally by running the script with
valid releases (v25.0, v24.0.1) and an invalid one (v0.1.0) to verify
the retry logic works correctly. The retry mechanism should significantly
reduce CI failures from transient network issues.
optional nit: Consider adding an explicit timeout to `urlopen()`
(e.g., `timeout=60`) to fail faster in case of network hangs, rather
than relying on OS socket defaults. This would make the script more
predict
...
💬 enirox001 commented on pull request "test: Retry download in get_previous_releases.py":
(https://github.com/bitcoin/bitcoin/pull/33915#discussion_r2546940887)
nano nit: The single retry seems reasonable to start with. If CI flakiness persists after this change, increasing to 2-3 retries or adding exponential backoff can be considered
(https://github.com/bitcoin/bitcoin/pull/33915#discussion_r2546940887)
nano nit: The single retry seems reasonable to start with. If CI flakiness persists after this change, increasing to 2-3 retries or adding exponential backoff can be considered
✅ fanquake closed an issue: "interface_ipc functional test failing in CI"
(https://github.com/bitcoin/bitcoin/issues/33884)
(https://github.com/bitcoin/bitcoin/issues/33884)
🚀 fanquake merged a pull request: "test: Fix race condition in IPC interface block progation test"
(https://github.com/bitcoin/bitcoin/pull/33880)
(https://github.com/bitcoin/bitcoin/pull/33880)
💬 vasild commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2546914489)
Would it be useful to print the first 5, even if there are more? E.g.
```
Reconstructed block B required tx 111
Reconstructed block B required tx 222
Reconstructed block B required tx 333
Reconstructed block B required tx 444
Reconstructed block B required tx 555 and others
```
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2546914489)
Would it be useful to print the first 5, even if there are more? E.g.
```
Reconstructed block B required tx 111
Reconstructed block B required tx 222
Reconstructed block B required tx 333
Reconstructed block B required tx 444
Reconstructed block B required tx 555 and others
```
💬 vasild commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2546951579)
I am missing some high level context here, but is this guaranteed to iterate over all members of `vtx_missing[]`?
I ask because down there we do iterate the full `vtx_missing[]` to derive `tx_missing_size`.
(https://github.com/bitcoin/bitcoin/pull/33738#discussion_r2546951579)
I am missing some high level context here, but is this guaranteed to iterate over all members of `vtx_missing[]`?
I ask because down there we do iterate the full `vtx_missing[]` to derive `tx_missing_size`.
✅ fanquake closed an issue: "build: broken CMake *flags output"
(https://github.com/bitcoin/bitcoin/issues/31482)
(https://github.com/bitcoin/bitcoin/issues/31482)
💬 fanquake commented on issue "build: broken CMake *flags output":
(https://github.com/bitcoin/bitcoin/issues/31482#issuecomment-3559202651)
Not sure making changes to address this, are worth it. Closing for now.
(https://github.com/bitcoin/bitcoin/issues/31482#issuecomment-3559202651)
Not sure making changes to address this, are worth it. Closing for now.
💬 fanquake commented on pull request "cmake: Fix `-pthread` flags presentation in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-3559205819)
I've closed #31482, as not sure making further changes to address this is worth it.
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-3559205819)
I've closed #31482, as not sure making further changes to address this is worth it.
✅ fanquake closed an issue: "CI: Improve documentation around replicating CI locally"
(https://github.com/bitcoin/bitcoin/issues/31199)
(https://github.com/bitcoin/bitcoin/issues/31199)
🚀 fanquake merged a pull request: "doc: Improve CI docs on env and qemu-user-static"
(https://github.com/bitcoin/bitcoin/pull/33887)
(https://github.com/bitcoin/bitcoin/pull/33887)
⚠️ fanquake reopened an issue: "CI: Improve documentation around replicating CI locally"
(https://github.com/bitcoin/bitcoin/issues/31199)
I've received some feedback that replicating CI jobs locally isn't straightforward. Perhaps the documentation can be improved in this area?
@maflcko I understand you had some improvements in mind? Happy to also work on this.
(https://github.com/bitcoin/bitcoin/issues/31199)
I've received some feedback that replicating CI jobs locally isn't straightforward. Perhaps the documentation can be improved in this area?
@maflcko I understand you had some improvements in mind? Happy to also work on this.
💬 fanquake commented on issue "CI: Improve documentation around replicating CI locally":
(https://github.com/bitcoin/bitcoin/issues/31199#issuecomment-3559214112)
@m3dwards can you advise if this can be closed after #33887. Or clarify in the description what else should be done.
(https://github.com/bitcoin/bitcoin/issues/31199#issuecomment-3559214112)
@m3dwards can you advise if this can be closed after #33887. Or clarify in the description what else should be done.