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

Compiling from msbuild incorrectly uses target build tools for build scripts #1308

Closed
dlon opened this issue Dec 1, 2024 · 8 comments · Fixed by #1310
Closed

Compiling from msbuild incorrectly uses target build tools for build scripts #1308

dlon opened this issue Dec 1, 2024 · 8 comments · Fixed by #1310

Comments

@dlon
Copy link
Contributor

dlon commented Dec 1, 2024

The problem arises when running cargo build from within a VS project and targeting something other than the native arch (e.g., using msbuild.exe myproj.sln /p:Platform=otherarch). When cargo builds a build script, it uses the target linker (added to PATH by Visual Studio) instead of the host linker, which naturally fails.

This is a problem as of Rust 1.83 due to the following fix: #1201. That PR also contains some discussion about this issue.

@dlon
Copy link
Contributor Author

dlon commented Dec 1, 2024

@NobodyXu @ChrisDenton Would you accept a PR based on your suggestion of parsing the output of cl.exe? :)

The odd thing about it is that the msbuild tools will be used for the native target build scripts, but not otherwise. That's a bit counter-intuitive, perhaps.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 2, 2024

Yes I'm willing to review and merge it

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 2, 2024

The odd thing about it is that the msbuild tools will be used for the native target build scripts, but not otherwise. That's a bit counter-intuitive, perhaps.

Well cc doesn't link anything, only compile and then archive.

@ChrisDenton
Copy link
Member

ChrisDenton commented Dec 2, 2024

Looking at the code, for Visual Studio proper (e.g. after vcvars.bat) we use VSCMD_ARG_TGT_ARCH to verify the environment is set up for the target and then VSINSTALLDIR to find the path to Visual Studio, which we then use to set the toolchain to the right target:

if is_vscmd_target(target, env_getter) == Some(false) {
// We will only get here with versions 15+.
let vs_install_dir: PathBuf = env_getter.get_env("VSINSTALLDIR")?.into();
tool_from_vs15plus_instance(tool, target, &vs_install_dir, env_getter)

fn is_vscmd_target(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option<bool> {
let vscmd_arch = env_getter.get_env("VSCMD_ARG_TGT_ARCH")?;
// Convert the Rust target arch to its VS arch equivalent.
let arch = match target.into() {
"x86_64" => "x64",
"aarch64" | "arm64ec" => "arm64",
"i686" | "i586" => "x86",
"thumbv7a" => "arm",
// An unrecognized arch.
_ => return Some(false),
};
Some(vscmd_arch.as_ref() == arch)
}

MSBuild does not set either of those nor does it set anything equivalent. For VSCMD_ARG_TGT_ARCH we can instead parse the header of cl.exe. For VSINSTALLDIR I guess in the case where there's a target mismatch we can instead just return None from find_msvc_environment so that it'll fallback to finding the latest matching tools.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 2, 2024

For VSCMD_ARG_TGT_ARCH we can instead parse the header of cl.exe.

Hmmm how hard would it be?
Would it be portable?

For VSINSTALLDIR I guess in the case where there's a target mismatch we can instead just return None from find_msvc_environment so that it'll fallback to finding the latest matching tools.

👍

@ChrisDenton
Copy link
Member

Hmmm how hard would it be?

I meant the thing I mentioned earlier about parsing the line that appears at the top of stderr from cl.exe (which cl.exe actually calls the "logo").

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 2, 2024

Thanks for clarification that's pretty portable and quite reliable way of doing it, I don't think we have a better way to do it

dlon added a commit to mullvad/mullvadvpn-app that referenced this issue Dec 2, 2024
When running cargo from MSVC, as of Rust 1.83, it uses build tools
for the target MSVC architecture rather than host, when building
build scripts. Unsetting 'VSTEL_MSBuildProjectFullPath' works around
this issue.

This workaround can be removed once the upstream issue has been fixed:
rust-lang/cc-rs#1308
dlon added a commit to mullvad/mullvadvpn-app that referenced this issue Dec 2, 2024
When running cargo from MSVC, as of Rust 1.83, it uses build tools
for the target MSVC architecture rather than host, when building
build scripts. Unsetting 'VSTEL_MSBuildProjectFullPath' works around
this issue.

This workaround can be removed once the upstream issue has been fixed:
rust-lang/cc-rs#1308
@dlon dlon changed the title msbuild uses target linker for build scripts msbuild uses target build tools for build scripts Dec 2, 2024
@dlon
Copy link
Contributor Author

dlon commented Dec 2, 2024

I've now opened #1310 to fix this issue. Thank you both for the help!

@dlon dlon changed the title msbuild uses target build tools for build scripts Compiling from msbuild incorrectly uses target build tools for build scripts Dec 2, 2024
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 a pull request may close this issue.

3 participants