#36271 Revert #36057 & document mingw-w64 requirement
https://github.com/bitcoin/bitcoin/pull/36271 · · +5/-19 in 6 files, 3 commits · labels: none
Goal
- Clean up redundant build checks now that Mingw-w64 12.0 is the baseline for Windows builds
- Document the minimum toolchain requirement and remove obsolete cross-compilation configurations
Reverts PR #36057's configure-time check for `SetThreadDescription()` now that the `33.x` release branch is split, documents the minimum requirement of Mingw-w64 12.0 in the Windows build instructions, and drops the legacy MSVCRT configuration from the Nix cross-compilation shell.
Problem: Prior to branching 33.x, older Mingw-w64 toolchains lacked `SetThreadDescription()`, requiring configure-time detection. With Mingw-w64 12.0 established as the baseline requirement for master, the check is redundant technical debt.
Category: Build and CI (#34 of 55)
P3 · cleanup
- P3 because it cleans up temporary build checks and workarounds no longer needed on master
- Clarifies minimum toolchain requirements for Windows cross-compilation with minor maintenance leverage
Retires a temporary build workaround following the 33.x branch off and clarifies toolchain requirements. Hebasto confirmed this 'follows the plan' set out during PR #36057.
Membership: Removes CMake configure-time symbol checks and updates Windows toolchain cross-compilation configurations.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1
Reviewability: Ready
- Ready to review with a clean diff, passing CI, and all review suggestions already incorporated
Clean diff with no conflicts, passing CI, and author promptly incorporated all review suggestions.
Author status: active
Resolved concerns:
- maflcko suggested dropping the MSVCRT host branch in `contrib/devtools/shell-win64-cross.nix` and `depends/README.md`, which the author incorporated in a force push.
Agreement: Strong
- Strong consensus to complete the planned post-branch toolchain cleanup
- Concept approval because it follows the agreed plan (hebasto)
- Suggested dropping obsolete Nix MSVCRT references which was adopted (maflcko)
- Code review approval on the updated revision (ViniciusCestarii)
Strong consensus to execute the planned post-branch toolchain cleanup.
Reviewers confirmed the change follows the established plan from #36057, with explicit concept and commit ACKs and no objections.
- hebasto: 'Concept ACK. This follows the plan.'
- ViniciusCestarii: 'ACK ce76c082cead6a107d1c95f584a0eff390b3539f'
- maflcko suggested removing obsolete Nix MSVCRT references, promptly applied by fanquake.
Review verdicts (DrahtBot): 1
- ACK: ViniciusCestarii
- Concept ACK: hebasto
Files
7 lines under test/bench/ci.
- contrib/devtools/shell-win64-cross.nix +1/-6
- src/util/threadnames.cpp +2/-4
- cmake/introspection.cmake +0/-4
- cmake/bitcoin-build-config.h.in +0/-3
- depends/README.md +1/-2
- doc/build-windows.md +1/-0
Card
Reverts PR #36057's temporary configure-time check for SetThreadDescription() now that the 33.x release has branched off, establishing Mingw-w64 12.0 as the minimum Windows build toolchain requirement. It also cleans up obsolete MSVCRT settings from the Nix cross-compilation shell. The change follows the agreed build roadmap and has strong reviewer support with no open issues.