diff --git a/src/toolchain.rs b/src/toolchain.rs index 1fa1275956..8384a186bf 100644 --- a/src/toolchain.rs +++ b/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); diff --git a/tests/suite/cli_misc.rs b/tests/suite/cli_misc.rs index 46d748ada4..7d4c4af956 100644 --- a/tests/suite/cli_misc.rs +++ b/tests/suite/cli_misc.rs @@ -1,7 +1,14 @@ //! Test cases of the rustup command that do not depend on the //! dist server, mostly derived from multirust/test-v2.sh -use std::{env::consts::EXE_SUFFIX, fs, path::Path, str}; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt as _; +use std::{ + env::{self, consts::EXE_SUFFIX}, + fs, + path::Path, + str, +}; use itertools::Itertools; use rustup::{ @@ -410,7 +417,7 @@ async fn rustup_doesnt_prepend_path_unnecessarily() { let assert_ok_with_paths = |assert: &Assert, data| { assert.is_ok(); - let stderr = std::env::split_paths(&assert.output.stderr) + let stderr = env::split_paths(&assert.output.stderr) .format_with("\n", |p, f| f(&p.display())) .to_string(); let stderr = assert.redact(&stderr); @@ -468,7 +475,7 @@ async fn rustup_doesnt_prepend_path_unnecessarily() { ["cargo", "--echo-env", "PATH"], [( "PATH", - std::env::join_paths([&cx.config.exedir, &cargo_home_bin]) + env::join_paths([&cx.config.exedir, &cargo_home_bin]) .unwrap() .to_str() .unwrap(), @@ -1762,6 +1769,15 @@ async fn rust_analyzer_proxy_uses_external_without_hosted() { .await; let real_path = Path::new(real_path.output.stderr.trim()); fs::copy(real_path, extern_path).unwrap(); + // Copy `.version` and `.version-hash` files as well so that the `--version` probe works. This + // is needed for rustup to detect valid external `rust-analyzer` fallbacks. + for suffix in [".version", ".version-hash"] { + fs::copy( + format!("{}{suffix}", real_path.display()), + format!("{}{suffix}", extern_path.display()), + ) + .unwrap(); + } cx.config .expect(["rustup", "component", "remove", "rust-analyzer"]) .await @@ -1783,6 +1799,85 @@ info: falling back to "[EXTERN_PATH]" .is_ok(); } +#[cfg(unix)] +#[tokio::test] +async fn rust_analyzer_proxy_skips_external_redirecting_to_rustup() { + let cx = CliTestContext::new(Scenario::RustAnalyzer).await; + cx.config.expect(RUST_ANALYZER_SETUP_CMD).await.is_ok(); + + let tempdir = tempfile::Builder::new().prefix("rustup").tempdir().unwrap(); + + // Creates the "good" external fallback under `extern_path` by copying from the `rust-analyzer` + // mock binary under `real_path`. + let extern_dir = tempdir.path(); + let extern_path = &extern_dir.join("rust-analyzer"); + let real_path = cx + .config + .expect(["rust-analyzer", "--echo-current-exe"]) + .await; + let real_path = Path::new(real_path.output.stderr.trim()); + fs::copy(real_path, extern_path).unwrap(); + // Copy `.version` and `.version-hash` files as well so that the `--version` probe works. This + // is needed for rustup to detect valid external `rust-analyzer` fallbacks. + for suffix in [".version", ".version-hash"] { + fs::copy( + format!("{}{suffix}", real_path.display()), + format!("{}{suffix}", extern_path.display()), + ) + .unwrap(); + } + cx.config + .expect(["rustup", "component", "remove", "rust-analyzer"]) + .await + .is_ok(); + + // Creates the "bad" external fallback under `bad_extern_dir`, which is itself a shell script + // that calls the rustup proxy, causing infinite recursion if used. + let bad_extern_dir = extern_dir.join("bad"); + fs::create_dir_all(&bad_extern_dir).unwrap(); + let bad_extern_path = &bad_extern_dir.join("rust-analyzer"); + fs::write( + bad_extern_path, + r#"#!/bin/sh +exec "$RUSTUP_TEST_RUST_ANALYZER_PROXY" "$@" +"#, + ) + .unwrap(); + let mut permissions = fs::metadata(bad_extern_path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(bad_extern_path, permissions).unwrap(); + + // When setting up the env override, we put the bad fallback before the good one. + let proxy_path = cx.config.exedir.join(format!("rust-analyzer{EXE_SUFFIX}")); + let env_override = &[ + ( + "PATH", + &*env::join_paths([&bad_extern_dir, extern_dir]) + .unwrap() + .display() + .to_string(), + ), + ( + "RUSTUP_TEST_RUST_ANALYZER_PROXY", + &*proxy_path.display().to_string(), + ), + ]; + + // If rustup is able to detect that the bad fallback is redirecting to itself, it should skip it + // and use the good fallback instead. + cx.config + .expect_with_env(["rust-analyzer", "--echo-current-exe"], env_override) + .await + .extend_redactions([("[EXTERN_PATH]", extern_path)]) + .is_ok() + .with_stderr(snapbox::str![[r#" +info: `rust-analyzer` is unavailable for the active toolchain +info: falling back to "[EXTERN_PATH]" +[EXTERN_PATH] + +"#]]); +} + const RUST_ANALYZER_SETUP_CMD: &[&str] = &[ "rustup", "toolchain",