search

corrosion-rs / corrosion

Marrying Rust and CMake - Easy Rust and C/C++ Integration!
https://corrosion-rs.github.io/corrosion/
MIT License
1.11k stars 106 forks source link

Leave choosing `msvcrt` up to the C/C++ CMake code #537

Closed jschwe closed 5 months ago

jschwe commented 5 months ago

It's difficult for Corrosion to correctly choose the right msvcrt version. CMake has a target property for it, and we also don't know which build configurations the user CMake code expectes to be Debug related.

The previous code actually already removed the linking against msvcrt in all cases except "Debug" builds. This was not intentional, but indicates that removing the library entirely from the link list, and leaving it up to the C/C++ code to link it in, should work well in practice.

jschwe commented 5 months ago

@Billyzou0741326 could you test this PR? I haven't tested this on a windows machine, but I believe we don't even need to specify msvcrt on the link line, since we should be able to rely on CMake adding msvcrt to the link line of the C/++ executable or shared library, that we link our Rust static library into.

Billyzou0741326 commented 5 months ago

I'm no longer seeing either msvcrt or msvcrtd being specified at all in the generated build scripts (checked both Ninja and the VS generator).

Not sure if this change will have other unintended effects, as I only tested via a trivial add method in the extern "C" style. At least that built just fine with no warnings