Skip to content
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

MSVC: Don't use environment variables to find tools if the targets differ #603

Merged
merged 1 commit into from
May 24, 2021

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented May 22, 2021

On Windows, when using a "Developer" powershell or command prompt, the "VCINSTALLDIR" environment variable is set. Currently if this is detected by cc-rs, then it will unconditionally assume the environment has already been correctly set up.

This PR adds a check to make sure the Visual Studio target matches the requested Rust target. If not then it falls back to using automatic detection.

Fixes: rust-lang/rust#43468

@ChrisDenton ChrisDenton force-pushed the vscmd-target-conflict branch from b5e526c to 6db1713 Compare May 22, 2021 06:50
@ChrisDenton ChrisDenton marked this pull request as draft May 23, 2021 08:31
@ChrisDenton
Copy link
Member Author

After feedback, I've converted this to a draft until I have a moment to update it.

There are concerns with my current approach and how it relies on an environment variable that may not be set for older versions of Visual Studio.

@ChrisDenton ChrisDenton force-pushed the vscmd-target-conflict branch from 6db1713 to c342210 Compare May 23, 2021 17:06
@ChrisDenton ChrisDenton changed the title MSVC: Don't use "VCINSTALLDIR" if its target differs from the Rust target MSVC: Don't use environment variables to find tools if the targets differ May 23, 2021
@ChrisDenton ChrisDenton marked this pull request as ready for review May 23, 2021 17:09
@ChrisDenton
Copy link
Member Author

This PR now adds a find_msvc_environment function that's intended to be used the same way as the other find_msvc_* probe functions. It currently only makes a difference with VS 2017 and 2019 but will fallback to the old behaviour on older versions of Visual Studio. A future PR could expand the function to better support more versions.

This PR shouldn't impact situations where cc was already working properly, but it will help in some cases where it used to give libs for the wrong arch.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me!

src/windows_registry.rs Show resolved Hide resolved
src/windows_registry.rs Show resolved Hide resolved
@alexcrichton
Copy link
Member

Thanks for this!

@alexcrichton alexcrichton merged commit 2428dec into rust-lang:master May 24, 2021
@ChrisDenton ChrisDenton deleted the vscmd-target-conflict branch May 24, 2021 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module Type 'X86' conflicts with machine type 'X64'
2 participants