Implement Hybrid CRT and consolidate Windows build logic #256
Conversation
| set(MSVC_CXX_FLAGS "${MSVC_CXX_FLAGS} /wd4275") | ||
| # C4646: function declared with 'noreturn' has non-void return type | ||
| set(MSVC_CXX_FLAGS "${MSVC_CXX_FLAGS} /wd4646") | ||
| # C4312: 'reinterpret_cast': conversion from 'X' to 'hermes::vm::GCCell *' of greater size |
There was a problem hiding this comment.
seems scary to suppress this one?
There was a problem hiding this comment.
Agree, but I had no choice before. I cannot affect the code from Meta. I can try to remove it to see how it behaves today, Or at least change it to a warning instead of error.
|
|
||
| ### Hybrid CRT Configuration | ||
|
|
||
| A major feature of this fork is the **Hybrid CRT configuration**, which ensures |
There was a problem hiding this comment.
Is this for non-Office usage? I would hope the binaries we ship in Office aren't statically linking the CRT at all?
There was a problem hiding this comment.
This is exactly the reason for the PR. Tiago said that currently the hermes.dll causes issues with Office because of the imports. Thus, as he suggested and pointed to your PR we statically link the C++ runtime and dynamically the UCRT.
There was a problem hiding this comment.
Oh. So, Office is presumably consuming a hermes.dll that statically links the CRT. This will help that case. I would guess dynamically linking the CRT is too hard to synchronize the versions with Office.
There was a problem hiding this comment.
I just checked the DLL size and updated the PR description. The size increase is just about 163K or 3.18%.
vmoroz
force-pushed
the
PR/fix-dll-dependencies
branch
from
b95ae9a to
d7a00cd
Compare
|
lgtm |
4 participants
This PR introduces two main improvements to the
hermes-windowsfork:Hybrid CRT Implementation (Win32 targets):
The Win32 build now follows the “Hybrid CRT” model from the Windows App SDK Coding Guidelines: the static C++ runtime (
/MTdor/MT) is linked while the Universal CRT (UCRT) remains dynamically linked. This removes the dependency on the Visual C++ Redistributable (MSVCP140.dll, etc.), so Hermes-based apps only require the inbox UCRT.Note: UWP (
WindowsStore) builds continue to use the default dynamic CRT, because the platform forbids statically linking the MSVC runtime. CMake’sCMAKE_MSVC_RUNTIME_LIBRARYis therefore only forced for Win32 configurations.Windows Build Logic Consolidation:
All Windows-specific build logic, patches, and workarounds now live in
HermesWindows.cmakeinstead of being scattered through the upstream CMakeLists.txt files. This keeps our fork closer tofacebook/hermes, easing future merges.Key Changes
hermes.dll(Win32 Release):MSVCP140.dllVCRUNTIME140.dllVCRUNTIME140_1.dllKERNEL32.dllKERNEL32.dllADVAPI32.dllADVAPI32.dllWINMM.dllWINMM.dllicuuc.dllicuuc.dllicuin.dllicuin.dllapi-ms-win-crt-stdio-l1-1-0.dllapi-ms-win-crt-stdio-l1-1-0.dllapi-ms-win-crt-heap-l1-1-0.dllapi-ms-win-crt-heap-l1-1-0.dllapi-ms-win-crt-runtime-l1-1-0.dllapi-ms-win-crt-runtime-l1-1-0.dllapi-ms-win-crt-math-l1-1-0.dllapi-ms-win-crt-math-l1-1-0.dllapi-ms-win-crt-time-l1-1-0.dllapi-ms-win-crt-time-l1-1-0.dllapi-ms-win-crt-convert-l1-1-0.dllapi-ms-win-crt-convert-l1-1-0.dllapi-ms-win-crt-string-l1-1-0.dllapi-ms-win-crt-string-l1-1-0.dllapi-ms-win-crt-utility-l1-1-0.dllapi-ms-win-crt-utility-l1-1-0.dllapi-ms-win-crt-locale-l1-1-0.dllapi-ms-win-crt-filesystem-l1-1-0.dllhermes.dllsize change (Win32 Release):Build Logic Consolidation
HermesWindows.cmakecentralizes the Windows configuration.CMAKE_MSVC_RUNTIME_LIBRARYis set there only for non-UWP builds so Win32 uses Hybrid CRT while UWP keeps the supported dynamic CRT.target_compile_options.Documentation
HermesWindowsFork.mdto explain the fork’s principles, Hybrid CRT approach, and merge strategy.Cleanup
test/node-hermesdirectory so our tree matches upstream.Microsoft Reviewers: Open in CodeFlow