-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Cleanup VCRuntime140.dll dependency from native components #4381
Conversation
| set(CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO} /LTCG /OPT:REF /OPT:ICF ${NO_INCREMENTAL_LINKER_FLAGS}") | ||
|
|
||
| # Force uCRT to be dynamically linked for Release build | ||
| set(CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO "${CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO} /NODEFAULTLIB:libucrt.lib /DEFAULTLIB:ucrt.lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is overriding default lib the blessed (by C++) way to mix the static and dynamic linking? Similar things were hard to get right in the past - had subtle bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This is what they suggested. This is possible now because runtime pieces remain statically linked while General library functions come from ucrt dynamically.
|
LGTM |
| # Define the uCRT lib reference | ||
| set(STATIC_MT_UCRT_LIB "libucrt$<$<OR:$<CONFIG:Debug>,$<CONFIG:Checked>>:d>.lib") | ||
| set(STATIC_UCRT_LIB "libucrt$<$<OR:$<CONFIG:Debug>,$<CONFIG:Checked>>:d>.lib") | ||
| set(DYNAMIC_UCRT_LIB "ucrt$<$<OR:$<CONFIG:Debug>,$<CONFIG:Checked>>:d>.lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are these two getting used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet. Defined for any future use for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. LGTM then. I'm working on the CoreFX version now.
Cleanup VCRuntime140.dll dependency from native components Commit migrated from dotnet/coreclr@1caf7b9
|
Was this made to no longer be static linked since soon Windows 7 support will be dropped which means that .NET could default to use the OS's UCRT copy through dynamic (and not static) link? |
This removes the dependency on VCRuntime140.dll from the native components built in CoreCLR repo. By default, we static link all CRT pieces. However, for the Release build, we link into uCRT DLL while continuing to static link the CRT pieces corresponding to vcruntime140.dll. I have confirmed that the Release binaries only have dependency on the uCRT APISets that will be pushed down via WU.
Fixes https://github.com/dotnet/coreclr/issues/4334.
@jkotas @russellhadley PTAL
@weshaggard @ericstj - FYI