-
Notifications
You must be signed in to change notification settings - Fork 131
Don't ignore Debug profile on MSVC #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Trying to remember what's going on here, my guess is that the debug profile linked with I'm pretty sure this'll break some projects, but if it works for Cargo that at least means a good number should continue to work. In that sense I'm curious to see what the breakage is here, and we can always back it out if there's too much breakage! |
Oh yeah I had forgotten what a mess Windows CRT is. Anyway, since cargo is statically linked that explains why cargo just works with this change. IIUC there would only ever be a problem when dynamically linking multiple build artifacts. In those cases however you'd want finer-grained control over what CRT is used anyway. |
Since CMake 3.15 the linked CRT can be controlled with the CMAKE_MSVC_RUNTIME_LIBRARY variable. Considering that AFAIK, Rust still uses exclusively non-debug variants of the MSVC CRT, it would be a good idea to bring back this behavior. In fact I ran into this issue today, resulting in this linker error
(While trying to Interestingly, setting cmake_minimum_required to 3.1 fixes the issue, so I assume some CMake behavior must have changed in a later version? |
Currently you need to pass The difficulty is that rustc itself only supports using msvcrt or libcm (when static linking). So if you want to use a debug CRT it's necessary to tell the linker to ignore what rust says. |
Well, the thing is that I do not want to use the debug CRT. This is CMake configuring the project to link against it. [UPDATE] So from what I understand, what For reference, the workaround I'm using right now is: let build_dir = cmake::Config::new(path)
.define("CMAKE_MSVC_RUNTIME_LIBRARY", "MultiThreadedDLL")
.build(); |
Fixes #19
@alexcrichton Do you remember why you added this check?
I tested building cargo with this change and it seems to work fine.