Conversation
|
Hello and thank you for the pr ! Could you give me some context about what this does? Is it meant to add support for building static executables on windows? If so, then could you also add this to the ci matrix, so that static windows binaries are tested and this does not regress in the future? |
|
Hi, thank you for the review. Rust exposes So this change is mainly about keeping the Rust side and the CMake-built native library consistent, and avoiding CRT mismatch during linking. This patch simply makes that choice explicit:
As for CI, I’d prefer to keep this PR scoped to the runtime-selection fix itself for now. Since this change mainly aligns the native build with Rust’s existing target configuration, I’m not sure adding a dedicated CI entry is necessary at this stage. But if you insist, I'm good to add. |
|
Understood ! For reading the feature, is crt-static special in some way, or can we read it like all other features in the build script ? Could we simplify this to .define("CMAKE_MSVC_RUNTIME_LIBRARY", if cfg!(feature = "crt-static") { "MultiThreaded" } else { "MultiThreadedDLL" }) |
This pull request updates the build script to select the appropriate MSVC runtime library based on whether the
crt-statictarget feature is enabled. This ensures that the correct runtime linkage is used for static or dynamic CRT builds.