Auto merge of #149987 - Zalathar:ambient-cdb, r=<try>

Move ambient cdb discovery from compiletest to bootstrap

try-job: x86_64-msvc-1
try-job: i686-msvc-1
try-job: aarch64-msvc-1
try-job: x86_64-mingw-1

Author: rust-bors[bot]

src/bootstrap/src/core/build_steps/test.rs

@@ -2181,6 +2181,10 @@ Please disable assertions with `rust.debug-assertions = false`.
         }
 
         if mode == CompiletestMode::Debuginfo {
+            if let Some(debuggers::Cdb { cdb }) = debuggers::discover_cdb(target) {
+                cmd.arg("--cdb").arg(cdb);
+            }
+
             if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder, android.as_ref())
             {
                 cmd.arg("--gdb").arg(gdb.as_ref());

src/bootstrap/src/core/debuggers/cdb.rs

@@ -0,0 +1,41 @@
+use std::env;
+use std::path::PathBuf;
+
+use crate::core::config::TargetSelection;
+
+pub(crate) struct Cdb {
+    pub(crate) cdb: PathBuf,
+}
+
+/// FIXME: This CDB discovery code was very questionable when it was in
+/// compiletest, and it's just as questionable now that it's in bootstrap.
+pub(crate) fn discover_cdb(target: TargetSelection) -> Option<Cdb> {
+    if !cfg!(windows) || !target.ends_with("-pc-windows-msvc") {
+        return None;
+    }
+
+    let pf86 =
+        PathBuf::from(env::var_os("ProgramFiles(x86)").or_else(|| env::var_os("ProgramFiles"))?);
+    let cdb_arch = if cfg!(target_arch = "x86") {
+        "x86"
+    } else if cfg!(target_arch = "x86_64") {
+        "x64"
+    } else if cfg!(target_arch = "aarch64") {
+        "arm64"
+    } else if cfg!(target_arch = "arm") {
+        "arm"
+    } else {
+        return None; // No compatible CDB.exe in the Windows 10 SDK
+    };
+
+    let mut path = pf86;
+    path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too?
+    path.push(cdb_arch);
+    path.push(r"cdb.exe");
+
+    if !path.exists() {
+        return None;
+    }
+
+    Some(Cdb { cdb: path })
+}

src/bootstrap/src/core/debuggers/mod.rs

@@ -1,8 +1,10 @@
 //! Code for discovering debuggers and debugger-related configuration, so that
 //! it can be passed to compiletest when running debuginfo tests.
 
+pub(crate) use self::cdb::{Cdb, discover_cdb};
 pub(crate) use self::gdb::{Gdb, discover_gdb};
 pub(crate) use self::lldb::{Lldb, discover_lldb};
 
+mod cdb;
 mod gdb;
 mod lldb;

src/tools/compiletest/src/debuggers.rs

@@ -2,7 +2,7 @@ use std::env;
 use std::process::Command;
 use std::sync::Arc;
 
-use camino::{Utf8Path, Utf8PathBuf};
+use camino::Utf8Path;
 
 use crate::common::{Config, Debugger};
 
@@ -54,51 +54,6 @@ pub(crate) fn configure_lldb(config: &Config) -> Option<Arc<Config>> {
     Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() }))
 }
 
-/// Returns `true` if the given target is a MSVC target for the purposes of CDB testing.
-fn is_pc_windows_msvc_target(target: &str) -> bool {
-    target.ends_with("-pc-windows-msvc")
-}
-
-/// FIXME: this is very questionable...
-fn find_cdb(target: &str) -> Option<Utf8PathBuf> {
-    if !(cfg!(windows) && is_pc_windows_msvc_target(target)) {
-        return None;
-    }
-
-    let pf86 = Utf8PathBuf::from_path_buf(
-        env::var_os("ProgramFiles(x86)").or_else(|| env::var_os("ProgramFiles"))?.into(),
-    )
-    .unwrap();
-    let cdb_arch = if cfg!(target_arch = "x86") {
-        "x86"
-    } else if cfg!(target_arch = "x86_64") {
-        "x64"
-    } else if cfg!(target_arch = "aarch64") {
-        "arm64"
-    } else if cfg!(target_arch = "arm") {
-        "arm"
-    } else {
-        return None; // No compatible CDB.exe in the Windows 10 SDK
-    };
-
-    let mut path = pf86;
-    path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too?
-    path.push(cdb_arch);
-    path.push(r"cdb.exe");
-
-    if !path.exists() {
-        return None;
-    }
-
-    Some(path)
-}
-
-/// Returns Path to CDB
-pub(crate) fn discover_cdb(cdb: Option<String>, target: &str) -> Option<Utf8PathBuf> {
-    let cdb = cdb.map(Utf8PathBuf::from).or_else(|| find_cdb(target));
-    cdb
-}
-
 pub(crate) fn query_cdb_version(cdb: &Utf8Path) -> Option<[u16; 4]> {
     let mut version = None;
     if let Ok(output) = Command::new(cdb).arg("/version").output() {

src/tools/compiletest/src/lib.rs

@@ -263,7 +263,7 @@ fn parse_config(args: Vec<String>) -> Config {
     let adb_device_status = target.contains("android") && adb_test_dir.is_some();
 
     // FIXME: `cdb_version` is *derived* from cdb, but it's *not* technically a config!
-    let cdb = debuggers::discover_cdb(matches.opt_str("cdb"), &target);
+    let cdb = matches.opt_str("cdb").map(Utf8PathBuf::from);
     let cdb_version = cdb.as_deref().and_then(debuggers::query_cdb_version);
     // FIXME: `gdb_version` is *derived* from gdb, but it's *not* technically a config!
     let gdb = matches.opt_str("gdb").map(Utf8PathBuf::from);