fix(toolchain): don't fall back to external RA redirecting to rustup

src/toolchain.rs

@@ -1,5 +1,3 @@
-#[cfg(unix)]
-use std::os::unix::fs::PermissionsExt as _;
 use std::{
     env::{self, consts::EXE_SUFFIX},
     ffi::{OsStr, OsString},
@@ -385,19 +383,39 @@ impl<'a> Toolchain<'a> {
 
         let me = env::current_exe()?;
 
-        // Try to find the first `rust-analyzer` under the `$PATH` that is both
-        // an existing file and not the same file as `me`, i.e. not a rustup proxy.
+        // Try to find the first `rust-analyzer` under the `$PATH` that:
+        // - Is an existing file.
+        // - Is not the same file as `me`, i.e. not a rustup proxy.
+        // - Can be executed with `--version` while `RUST_RECURSION_COUNT` is set to
+        //   `RUST_RECURSION_COUNT_MAX + 1`, i.e. doesn't redirect to rustup.
         for mut p in env::split_paths(&path) {
             p.push(binary);
-            let is_external_ra = p.is_file()
+            let mut is_external_ra = p.is_file()
                 // We report `true` on `is_same_file()` error to prevent an invalid `p`
                 // from becoming the candidate.
                 && !is_same_file(&me, &p).unwrap_or(true);
-            // On Unix, we additionally check if the file is executable.
-            #[cfg(unix)]
-            let is_external_ra = is_external_ra
-                && p.metadata()
-                    .is_ok_and(|meta| meta.permissions().mode() & 0o111 != 0);
+
+            if is_external_ra {
+                let mut ra = Command::new(&p);
+                self.set_env(&mut ra);
+                ra.arg("--version")
+                    .env(
+                        "RUST_RECURSION_COUNT",
+                        (env_var::RUST_RECURSION_COUNT_MAX + 1).to_string(),
+                    )
+                    .stdout(Stdio::null())
+                    .stderr(Stdio::null())
+                    .stdin(Stdio::null());
+
+                is_external_ra = match ra
+                    .spawn()
+                    .and_then(|mut c| c.wait_timeout(Duration::from_secs(10)))
+                {
+                    Ok(Some(s)) => s.success(),
+                    Ok(None) | Err(_) => false,
+                }
+            }
+
             if is_external_ra {
                 let mut ra = Command::new(p);
                 self.set_env(&mut ra);

tests/suite/cli_misc.rs

@@ -1,6 +1,8 @@
 //! Test cases of the rustup command that do not depend on the
 //! dist server, mostly derived from multirust/test-v2.sh
 
+#[cfg(unix)]
+use std::os::unix::fs::PermissionsExt as _;
 use std::{env::consts::EXE_SUFFIX, fs, path::Path, str};
 
 use itertools::Itertools;
@@ -1753,7 +1755,18 @@ async fn rust_analyzer_proxy_falls_back_external() {
     let tempdir = tempfile::Builder::new().prefix("rustup").tempdir().unwrap();
     let extern_dir = tempdir.path();
     let extern_path = &extern_dir.join("rust-analyzer");
+    // HACK: The toolchain we are using in this test has `rls` instead of `rust-analyzer` but we
+    // need to test the latter. Thus, we copy the `rls` mock binary and its version files to the
+    // external directory to look like `rust-analyzer` so that `rust-analyzer` and `rust-analyzer
+    // --version` continue to work.
     fs::copy(real_path, extern_path).unwrap();
+    for suffix in [".version", ".version-hash"] {
+        fs::copy(
+            bindir.join(format!("rls{EXE_SUFFIX}{suffix}")),
+            format!("{}{suffix}", extern_path.display()),
+        )
+        .unwrap();
+    }
 
     // First case: rustup-hosted and external RA both installed,
     // prioritize the former.
@@ -1786,6 +1799,43 @@ info: falling back to "[EXTERN_PATH]"
 
 "#]])
         .is_ok();
+
+    // Third case: rustup-hosted RA unavailable, external RA redirects to rustup proxy, fallback fails.
+    #[cfg(unix)]
+    {
+        fs::write(
+            extern_path,
+            r#"#!/bin/sh
+exec "$RUSTUP_TEST_RUST_ANALYZER_PROXY" "$@"
+"#,
+        )
+        .unwrap();
+        let mut permissions = fs::metadata(extern_path).unwrap().permissions();
+        permissions.set_mode(0o755);
+        fs::set_permissions(extern_path, permissions).unwrap();
+
+        let proxy_path = exedir.join(&ra);
+        cx.config
+            .expect_with_env(
+                ["rust-analyzer", "--version"],
+                [
+                    ("PATH", &*extern_dir.display().to_string()),
+                    (
+                        "RUSTUP_TEST_RUST_ANALYZER_PROXY",
+                        &*proxy_path.display().to_string(),
+                    ),
+                ],
+            )
+            .await
+            .is_err()
+            // NOTE: The below "unknown binary" error message is shown because the toolchain we are
+            // testing only has `rls` and we need to test `rust-analyzer` on it. As such the below
+            // snapshot only asserts that the infinite recursion behavior is gone.
+            .with_stderr(snapbox::str![[r#"
+error: Unknown binary 'rust-analyzer' in official toolchain 'stable-[HOST_TUPLE]'.
+
+"#]]);
+    };
 }
 
 #[tokio::test]