From e758ed19151638d468612a076812ea6f10f7f071 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 29 Dec 2024 21:10:47 +0100 Subject: [PATCH] Allow using Visual Studio target names in `find_tool` (#1335) * Allow using VS target names in find_tool Also fix a bug in vs15plus_vc_paths due to shadowing of the `target` variable. * code review update --- src/windows/find_tools.rs | 221 ++++++++++++++++++-------------------- 1 file changed, 103 insertions(+), 118 deletions(-) diff --git a/src/windows/find_tools.rs b/src/windows/find_tools.rs index fb7a3541..490e52c1 100644 --- a/src/windows/find_tools.rs +++ b/src/windows/find_tools.rs @@ -28,18 +28,37 @@ use crate::ToolFamily; const MSVC_FAMILY: ToolFamily = ToolFamily::Msvc { clang_cl: false }; -#[derive(Copy, Clone)] -struct TargetArch<'a>(pub &'a str); - -impl PartialEq<&str> for TargetArch<'_> { - fn eq(&self, other: &&str) -> bool { - self.0 == *other - } +/// The target provided by the user. +#[derive(Copy, Clone, PartialEq, Eq)] +enum TargetArch { + X86, + X64, + Arm, + Arm64, + Arm64ec, } +impl TargetArch { + /// Parse the `TargetArch` from a str. Returns `None` if the arch is unrecognized. + fn new(arch: &str) -> Option { + match arch { + "x64" | "x86_64" => Some(Self::X64), + "arm64" | "aarch64" => Some(Self::Arm64), + "arm64ec" => Some(Self::Arm64ec), + "x86" | "i686" | "i586" => Some(Self::X86), + "arm" | "thumbv7a" => Some(Self::Arm), + _ => None, + } + } -impl<'a> From> for &'a str { - fn from(target: TargetArch<'a>) -> Self { - target.0 + #[cfg(windows)] + /// Gets the Visual Studio name for the architecture. + fn as_vs_arch(&self) -> &'static str { + match self { + Self::X64 => "x64", + Self::Arm64 | Self::Arm64ec => "arm64", + Self::X86 => "x86", + Self::Arm => "arm", + } } } @@ -136,7 +155,7 @@ pub(crate) fn find_tool_inner( env_getter: &dyn EnvGetter, ) -> Option { // We only need the arch. - let target = TargetArch(full_arch); + let target = TargetArch::new(full_arch)?; // Looks like msbuild isn't located in the same location as other tools like // cl.exe and lib.exe. @@ -277,7 +296,7 @@ mod impl_ { /// # SAFETY /// /// The caller must ensure that the function signature matches the actual function. - /// The easiest way to do this is to add an entry to windows_sys_no_link.list and use the + /// The easiest way to do this is to add an entry to `windows_sys_no_link.list` and use the /// generated function for `func_signature`. /// /// The function returned cannot be used after the handle is dropped. @@ -355,41 +374,25 @@ mod impl_ { /// Checks to see if the target's arch matches the VS environment. Returns `None` if the /// environment is unknown. - fn is_vscmd_target(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn is_vscmd_target(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { is_vscmd_target_env(target, env_getter).or_else(|| is_vscmd_target_cl(target, env_getter)) } /// Checks to see if the `VSCMD_ARG_TGT_ARCH` environment variable matches the /// given target's arch. Returns `None` if the variable does not exist. - fn is_vscmd_target_env(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn is_vscmd_target_env(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { let vscmd_arch = env_getter.get_env("VSCMD_ARG_TGT_ARCH")?; - if let Some(arch) = vsarch_from_target(target) { - Some(arch == vscmd_arch.as_ref()) - } else { - Some(false) - } + Some(target.as_vs_arch() == vscmd_arch.as_ref()) } /// Checks if the cl.exe target matches the given target's arch. Returns `None` if anything /// fails. - fn is_vscmd_target_cl(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn is_vscmd_target_cl(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { let cmd_target = vscmd_target_cl(env_getter)?; - Some(vsarch_from_target(target) == Some(cmd_target)) + Some(target.as_vs_arch() == cmd_target) } - /// Convert the Rust target arch to its VS arch equivalent. - fn vsarch_from_target(target: TargetArch<'_>) -> Option<&'static str> { - match target.into() { - "x86_64" => Some("x64"), - "aarch64" | "arm64ec" => Some("arm64"), - "i686" | "i586" => Some("x86"), - "thumbv7a" => Some("arm"), - // An unrecognized arch. - _ => None, - } - } - - /// Detect the target architecture of `cl.exe`` in the current path, and return `None` if this + /// Detect the target architecture of `cl.exe` in the current path, and return `None` if this /// fails for any reason. fn vscmd_target_cl(env_getter: &dyn EnvGetter) -> Option<&'static str> { let cl_exe = env_getter.get_env("PATH").and_then(|path| { @@ -421,7 +424,7 @@ mod impl_ { /// Attempt to find the tool using environment variables set by vcvars. pub(super) fn find_msvc_environment( tool: &str, - target: TargetArch<'_>, + target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { // Early return if the environment isn't one that is known to have compiler toolsets in PATH @@ -453,13 +456,13 @@ mod impl_ { } } - fn find_msbuild_vs17(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn find_msbuild_vs17(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { find_tool_in_vs16plus_path(r"MSBuild\Current\Bin\MSBuild.exe", target, "17", env_getter) } #[allow(bare_trait_objects)] fn vs16plus_instances( - target: TargetArch<'_>, + target: TargetArch, version: &'static str, env_getter: &dyn EnvGetter, ) -> Box> { @@ -482,7 +485,7 @@ mod impl_ { fn find_tool_in_vs16plus_path( tool: &str, - target: TargetArch<'_>, + target: TargetArch, version: &'static str, env_getter: &dyn EnvGetter, ) -> Option { @@ -493,10 +496,10 @@ mod impl_ { return None; } let mut tool = Tool::with_family(path, MSVC_FAMILY); - if target == "x86_64" { + if target == TargetArch::X64 { tool.env.push(("Platform".into(), "X64".into())); } - if target == "aarch64" || target == "arm64ec" { + if matches!(target, TargetArch::Arm64 | TargetArch::Arm64ec) { tool.env.push(("Platform".into(), "ARM64".into())); } Some(tool) @@ -504,7 +507,7 @@ mod impl_ { .next() } - fn find_msbuild_vs16(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn find_msbuild_vs16(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { find_tool_in_vs16plus_path(r"MSBuild\Current\Bin\MSBuild.exe", target, "16", env_getter) } @@ -520,10 +523,7 @@ mod impl_ { // // However, on ARM64 this method doesn't work because VS Installer fails to register COM component on ARM64. // Hence, as the last resort we try to use vswhere.exe to list available instances. - fn vs15plus_instances( - target: TargetArch<'_>, - env_getter: &dyn EnvGetter, - ) -> Option { + fn vs15plus_instances(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { vs15plus_instances_using_com() .or_else(|| vs15plus_instances_using_vswhere(target, env_getter)) } @@ -538,7 +538,7 @@ mod impl_ { } fn vs15plus_instances_using_vswhere( - target: TargetArch<'_>, + target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { let program_files_path = env_getter @@ -554,11 +554,10 @@ mod impl_ { return None; } - let tools_arch = match target.into() { - "i586" | "i686" | "x86_64" => Some("x86.x64"), - "arm" | "thumbv7a" => Some("ARM"), - "aarch64" | "arm64ec" => Some("ARM64"), - _ => None, + let tools_arch = match target { + TargetArch::X86 | TargetArch::X64 => Some("x86.x64"), + TargetArch::Arm => Some("ARM"), + TargetArch::Arm64 | TargetArch::Arm64ec => Some("ARM64"), }; let vswhere_output = Command::new(vswhere_path) @@ -593,7 +592,7 @@ mod impl_ { pub(super) fn find_msvc_15plus( tool: &str, - target: TargetArch<'_>, + target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { let iter = vs15plus_instances(target, env_getter)?; @@ -617,7 +616,7 @@ mod impl_ { // [more reliable]: https://github.com/rust-lang/cc-rs/pull/331 fn find_tool_in_vs15_path( tool: &str, - target: TargetArch<'_>, + target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { let mut path = match vs15plus_instances(target, env_getter) { @@ -641,9 +640,9 @@ mod impl_ { path.map(|path| { let mut tool = Tool::with_family(path, MSVC_FAMILY); - if target == "x86_64" { + if target == TargetArch::X64 { tool.env.push(("Platform".into(), "X64".into())); - } else if target == "aarch64" { + } else if matches!(target, TargetArch::Arm64 | TargetArch::Arm64ec) { tool.env.push(("Platform".into(), "ARM64".into())); } tool @@ -652,7 +651,7 @@ mod impl_ { fn tool_from_vs15plus_instance( tool: &str, - target: TargetArch<'_>, + target: TargetArch, instance_path: &Path, env_getter: &dyn EnvGetter, ) -> Option { @@ -683,7 +682,7 @@ mod impl_ { } fn vs15plus_vc_paths( - target: TargetArch<'_>, + target_arch: TargetArch, instance_path: &Path, env_getter: &dyn EnvGetter, ) -> Option<(PathBuf, PathBuf, PathBuf, PathBuf, Option, PathBuf)> { @@ -705,13 +704,13 @@ mod impl_ { } _ => return None, }; - let target = lib_subdir(target)?; + let target_dir = target_arch.as_vs_arch(); // The directory layout here is MSVC/bin/Host$host/$target/ let path = instance_path.join(r"VC\Tools\MSVC").join(version); // We use the first available host architecture that can build for the target let (host_path, host) = hosts.iter().find_map(|&x| { let candidate = path.join("bin").join(format!("Host{}", x)); - if candidate.join(target).exists() { + if candidate.join(target_dir).exists() { Some((candidate, x)) } else { None @@ -719,7 +718,7 @@ mod impl_ { })?; // This is the path to the toolchain for a particular target, running // on a given host - let bin_path = host_path.join(target); + let bin_path = host_path.join(target_dir); // But! we also need PATH to contain the target directory for the host // architecture, because it contains dlls like mspdb140.dll compiled for // the host architecture. @@ -729,8 +728,9 @@ mod impl_ { } else { "lib" }; - let lib_path = path.join(lib_fragment).join(target); - let alt_lib_path = (target == "arm64ec").then(|| path.join(lib_fragment).join("arm64ec")); + let lib_path = path.join(lib_fragment).join(target_dir); + let alt_lib_path = + (target_arch == TargetArch::Arm64ec).then(|| path.join(lib_fragment).join("arm64ec")); let include_path = path.join("include"); Some(( path, @@ -786,9 +786,9 @@ mod impl_ { .unwrap_or_default() } - fn atl_paths(target: TargetArch<'_>, path: &Path) -> Option<(PathBuf, PathBuf)> { + fn atl_paths(target: TargetArch, path: &Path) -> Option<(PathBuf, PathBuf)> { let atl_path = path.join("atlmfc"); - let sub = lib_subdir(target)?; + let sub = target.as_vs_arch(); if atl_path.exists() { Some((atl_path.join("lib").join(sub), atl_path.join("include"))) } else { @@ -800,7 +800,7 @@ mod impl_ { // the Windows 10 SDK or Windows 8.1 SDK. pub(super) fn find_msvc_14( tool: &str, - target: TargetArch<'_>, + target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { let vcdir = get_vc_dir("14.0")?; @@ -809,12 +809,8 @@ mod impl_ { Some(tool.into_tool(env_getter)) } - fn add_sdks( - tool: &mut MsvcTool, - target: TargetArch<'_>, - env_getter: &dyn EnvGetter, - ) -> Option<()> { - let sub = lib_subdir(target)?; + fn add_sdks(tool: &mut MsvcTool, target: TargetArch, env_getter: &dyn EnvGetter) -> Option<()> { + let sub = target.as_vs_arch(); let (ucrt, ucrt_version) = get_ucrt_dir()?; let host = match host_arch() { @@ -871,7 +867,7 @@ mod impl_ { // Given a possible MSVC installation directory, we look for the linker and // then add the MSVC library path. - fn get_tool(tool: &str, path: &Path, target: TargetArch<'_>) -> Option { + fn get_tool(tool: &str, path: &Path, target: TargetArch) -> Option { bin_subdir(target) .into_iter() .map(|(sub, host)| { @@ -887,7 +883,7 @@ mod impl_ { tool }) .filter_map(|mut tool| { - let sub = vc_lib_subdir(target)?; + let sub = vc_lib_subdir(target); tool.libs.push(path.join("lib").join(sub)); tool.include.push(path.join("include")); let atlmfc_path = path.join("atlmfc"); @@ -1009,36 +1005,25 @@ mod impl_ { // linkers that can target the architecture we desire. The 64-bit host // linker is preferred, and hence first, due to 64-bit allowing it more // address space to work with and potentially being faster. - fn bin_subdir(target: TargetArch<'_>) -> Vec<(&'static str, &'static str)> { - match (target.into(), host_arch()) { - ("i586", X86) | ("i686", X86) => vec![("", "")], - ("i586", X86_64) | ("i686", X86_64) => vec![("amd64_x86", "amd64"), ("", "")], - ("x86_64", X86) => vec![("x86_amd64", "")], - ("x86_64", X86_64) => vec![("amd64", "amd64"), ("x86_amd64", "")], - ("arm", X86) | ("thumbv7a", X86) => vec![("x86_arm", "")], - ("arm", X86_64) | ("thumbv7a", X86_64) => vec![("amd64_arm", "amd64"), ("x86_arm", "")], + fn bin_subdir(target: TargetArch) -> Vec<(&'static str, &'static str)> { + match (target, host_arch()) { + (TargetArch::X86, X86) => vec![("", "")], + (TargetArch::X86, X86_64) => vec![("amd64_x86", "amd64"), ("", "")], + (TargetArch::X64, X86) => vec![("x86_amd64", "")], + (TargetArch::X64, X86_64) => vec![("amd64", "amd64"), ("x86_amd64", "")], + (TargetArch::Arm, X86) => vec![("x86_arm", "")], + (TargetArch::Arm, X86_64) => vec![("amd64_arm", "amd64"), ("x86_arm", "")], _ => vec![], } } - fn lib_subdir(target: TargetArch<'_>) -> Option<&'static str> { - match target.into() { - "i586" | "i686" => Some("x86"), - "x86_64" => Some("x64"), - "arm" | "thumbv7a" => Some("arm"), - "aarch64" | "arm64ec" => Some("arm64"), - _ => None, - } - } - // MSVC's x86 libraries are not in a subfolder - fn vc_lib_subdir(target: TargetArch<'_>) -> Option<&'static str> { - match target.into() { - "i586" | "i686" => Some(""), - "x86_64" => Some("amd64"), - "arm" | "thumbv7a" => Some("arm"), - "aarch64" => Some("arm64"), - _ => None, + fn vc_lib_subdir(target: TargetArch) -> &'static str { + match target { + TargetArch::X86 => "", + TargetArch::X64 => "amd64", + TargetArch::Arm => "arm", + TargetArch::Arm64 | TargetArch::Arm64ec => "arm64", } } @@ -1104,19 +1089,19 @@ mod impl_ { pub(super) fn has_msbuild_version(version: &str, env_getter: &dyn EnvGetter) -> bool { match version { "17.0" => { - find_msbuild_vs17(TargetArch("x86_64"), env_getter).is_some() - || find_msbuild_vs17(TargetArch("i686"), env_getter).is_some() - || find_msbuild_vs17(TargetArch("aarch64"), env_getter).is_some() + find_msbuild_vs17(TargetArch::X64, env_getter).is_some() + || find_msbuild_vs17(TargetArch::X86, env_getter).is_some() + || find_msbuild_vs17(TargetArch::Arm64, env_getter).is_some() } "16.0" => { - find_msbuild_vs16(TargetArch("x86_64"), env_getter).is_some() - || find_msbuild_vs16(TargetArch("i686"), env_getter).is_some() - || find_msbuild_vs16(TargetArch("aarch64"), env_getter).is_some() + find_msbuild_vs16(TargetArch::X64, env_getter).is_some() + || find_msbuild_vs16(TargetArch::X86, env_getter).is_some() + || find_msbuild_vs16(TargetArch::Arm64, env_getter).is_some() } "15.0" => { - find_msbuild_vs15(TargetArch("x86_64"), env_getter).is_some() - || find_msbuild_vs15(TargetArch("i686"), env_getter).is_some() - || find_msbuild_vs15(TargetArch("aarch64"), env_getter).is_some() + find_msbuild_vs15(TargetArch::X64, env_getter).is_some() + || find_msbuild_vs15(TargetArch::X86, env_getter).is_some() + || find_msbuild_vs15(TargetArch::Arm64, env_getter).is_some() } "14.0" => LOCAL_MACHINE .open(&OsString::from(format!( @@ -1128,16 +1113,16 @@ mod impl_ { } } - pub(super) fn find_devenv(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + pub(super) fn find_devenv(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { find_devenv_vs15(target, env_getter) } - fn find_devenv_vs15(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn find_devenv_vs15(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { find_tool_in_vs15_path(r"Common7\IDE\devenv.exe", target, env_getter) } // see http://stackoverflow.com/questions/328017/path-to-msbuild - pub(super) fn find_msbuild(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + pub(super) fn find_msbuild(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { // VS 15 (2017) changed how to locate msbuild if let Some(r) = find_msbuild_vs17(target, env_getter) { Some(r) @@ -1150,11 +1135,11 @@ mod impl_ { } } - fn find_msbuild_vs15(target: TargetArch<'_>, env_getter: &dyn EnvGetter) -> Option { + fn find_msbuild_vs15(target: TargetArch, env_getter: &dyn EnvGetter) -> Option { find_tool_in_vs15_path(r"MSBuild\15.0\Bin\MSBuild.exe", target, env_getter) } - fn find_old_msbuild(target: TargetArch<'_>) -> Option { + fn find_old_msbuild(target: TargetArch) -> Option { let key = r"SOFTWARE\Microsoft\MSBuild\ToolsVersions"; LOCAL_MACHINE .open(key.as_ref()) @@ -1166,7 +1151,7 @@ mod impl_ { let mut path = PathBuf::from(path); path.push("MSBuild.exe"); let mut tool = Tool::with_family(path, MSVC_FAMILY); - if target == "x86_64" { + if target == TargetArch::X64 { tool.env.push(("Platform".into(), "X64".into())); } tool @@ -1185,21 +1170,21 @@ mod impl_ { /// Finding msbuild.exe tool under unix system is not currently supported. /// Maybe can check it using an environment variable looks like `MSBUILD_BIN`. #[inline(always)] - pub(super) fn find_msbuild(_target: TargetArch<'_>, _: &dyn EnvGetter) -> Option { + pub(super) fn find_msbuild(_target: TargetArch, _: &dyn EnvGetter) -> Option { None } // Finding devenv.exe tool under unix system is not currently supported. // Maybe can check it using an environment variable looks like `DEVENV_BIN`. #[inline(always)] - pub(super) fn find_devenv(_target: TargetArch<'_>, _: &dyn EnvGetter) -> Option { + pub(super) fn find_devenv(_target: TargetArch, _: &dyn EnvGetter) -> Option { None } /// Attempt to find the tool using environment variables set by vcvars. pub(super) fn find_msvc_environment( tool: &str, - _target: TargetArch<'_>, + _target: TargetArch, env_getter: &dyn EnvGetter, ) -> Option { // Early return if the environment doesn't contain a VC install. @@ -1230,7 +1215,7 @@ mod impl_ { #[inline(always)] pub(super) fn find_msvc_15plus( _tool: &str, - _target: TargetArch<'_>, + _target: TargetArch, _: &dyn EnvGetter, ) -> Option { None @@ -1241,7 +1226,7 @@ mod impl_ { #[inline(always)] pub(super) fn find_msvc_14( _tool: &str, - _target: TargetArch<'_>, + _target: TargetArch, _: &dyn EnvGetter, ) -> Option { None