Repository navigation
Add hybrid CRT link for Windows binaries - #40890
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the top-level CMake configuration to adopt the “hybrid CRT” linking model for Windows binaries: keep /MT (static C++ runtime/STL) while forcing the Universal CRT to be dynamically linked (ucrt.dll) via linker default-lib overrides.
Changes:
- Adds linker flags to suppress
libucrt(.lib|d.lib)and explicitly link againstucrt(.lib|d.lib)in Release/Debug builds. - Documents the hybrid CRT intent and links to the reference guidance.
Blue (OneBlue)
left a comment
Blue (OneBlue)
left a comment
There was a problem hiding this comment.
I think this is a great change, and we should definitely merge it.
Agreed that we should wait after the initial release though. One thing that we should validate before merging this is whether this causes us any issues on WS 2022 (specifically on the ServerCore SKU), since we've had runtime dependencies issues in this SKU in the past.
If this runs without issues on WS 2022 ServerCore, I think we should merge it (after the 2.9 release).
Blocking for now, but will unblock after 2.9 is out.
|
There is certainly a reduction in size when compared to another build from the same time, but it isn't as much as I would have hoped for. |
| # but dynamic-link the Universal CRT (ucrt.dll) which is an OS component on Windows 10+. | ||
| # This eliminates the VCRUNTIME/MSVCP redistributable dependency while avoiding the size | ||
| # cost of statically linking the UCRT. See https://aka.ms/hybridcrt for details. | ||
| foreach(config RELEASE RELWITHDEBINFO MINSIZEREL) |
There was a problem hiding this comment.
nit: We don't support RELWITHDEBINFO and MINSIZEREL, so we can probably remove the loop here.
There was a problem hiding this comment.
Actually we can probably just write to CMAKE_SHARED_LINKER_FLAGS and CMAKE_EXE_LINKER_FLAGS here
There was a problem hiding this comment.
Also, should we apply this to STATIC_LIBRARY_FLAGS as well ?
Dismissing review since 2.9.3 has shipped
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CMakeLists.txt:340
CMAKE_STATIC_LINKER_FLAGS_*configures the librarian for.libcreation (lib.exe), not the linker for producing DLL/EXE outputs./NODEFAULTLIBand/DEFAULTLIBare link.exe options, so applying them viaCMAKE_STATIC_LINKER_FLAGS_*is at best ignored and at worst can break static library builds. These flags should only be applied to EXE/SHARED link steps.
set(CMAKE_STATIC_LINKER_FLAGS_RELEASE "${CMAKE_STATIC_LINKER_FLAGS_RELEASE} /NODEFAULTLIB:libucrt.lib /defaultlib:ucrt.lib")
set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG} /NODEFAULTLIB:libucrtd.lib /defaultlib:ucrtd.lib")
set(CMAKE_EXE_LINKER_FLAGS_DEBUG "${CMAKE_EXE_LINKER_FLAGS_DEBUG} /NODEFAULTLIB:libucrtd.lib /defaultlib:ucrtd.lib")
set(CMAKE_STATIC_LINKER_FLAGS_DEBUG "${CMAKE_STATIC_LINKER_FLAGS_DEBUG} /NODEFAULTLIB:libucrtd.lib /defaultlib:ucrtd.lib")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CMakeLists.txt:338
- These linker flags are MSVC-specific (/NODEFAULTLIB, /DEFAULTLIB). As written they are set unconditionally, which can break configuring this CMake project with a non-MSVC toolchain (or any future non-Windows build). Also, the comment claims it “eliminates the VCRUNTIME/MSVCP redistributable dependency”, but the project already sets CMAKE_MSVC_RUNTIME_LIBRARY to /MT|/MTd (MultiThreaded*), so the main value here is the binary size reduction from dynamically linking the UCRT.
# Hybrid CRT: static-link the C++ runtime and STL (/MT|/MTd from CMAKE_MSVC_RUNTIME_LIBRARY),
# but dynamic-link the Universal CRT (ucrt.dll) which is an OS component on Windows 10+.
# This eliminates the VCRUNTIME/MSVCP redistributable dependency while avoiding the size
# cost of statically linking the UCRT. See https://aka.ms/hybridcrt for details.
set(CMAKE_SHARED_LINKER_FLAGS_RELEASE "${CMAKE_SHARED_LINKER_FLAGS_RELEASE} /NODEFAULTLIB:libucrt.lib /defaultlib:ucrt.lib")
Blue (OneBlue)
left a comment
Blue (OneBlue)
left a comment
There was a problem hiding this comment.
LGTM, please make sure that this runs on the ServerCore SKU of Windows Server 2019 before merging.
Just validating that a simple container starts should be sufficient
Validated both install and image pull on Windows ServerCore 2022. |
Summary of the Pull Request
Configures hybrid static linkage for Windows binaries in the build. See https://aka.ms/hybridcrt for details on this model.
PR Checklist
Detailed Description of the Pull Request / Additional comments
The Windows binaries are all already statically linking the runtime. Switching to the hybrid model will cut non-trivial size from every binary, without adding any new dependency requirements.
I also understand if we want to wait on taking this until after a significant release so it may have more time to stabilize.
Validation Steps Performed
Built debug locally, ran an arbitrary test.