Merge #4329

4329: Look for `cargo`, `rustc`, and `rustup` in standard installation path r=matklad a=cdisselkoen

Discussed in #3118.  This is approximately a 90% fix for the issue described there.

This PR creates a new crate `ra_env` with a function `get_path_for_executable()`; see docs there.  `get_path_for_executable()` improves and generalizes the function `cargo_binary()` which was previously duplicated in the `ra_project_model` and `ra_flycheck` crates.  (Both of those crates now depend on the new `ra_env` crate.)  The new function checks (e.g.) `$CARGO` and `$PATH`, but also falls back on `~/.cargo/bin` manually before erroring out.  This should allow most users to not have to worry about setting the `$CARGO` or `$PATH` variables for VSCode, which can be difficult e.g. on macOS as discussed in #3118.

I've attempted to replace all calls to `cargo`, `rustc`, and `rustup` in rust-analyzer with appropriate invocations of `get_path_for_executable()`; I don't think I've missed any in Rust code, but there is at least one invocation in TypeScript code which I haven't fixed.  (I'm not sure whether it's affected by the same problem or not.) https://github.com/rust-analyzer/rust-analyzer/blob/a4778ddb7a00f552a8e653bbf56ae9fd69cfe1d3/editors/code/src/cargo.ts#L79

I'm sure this PR could be improved a bunch, so I'm happy to take feedback/suggestions on how to solve this problem better, or just bikeshedding variable/function/crate names etc.

cc @Veetaha 

Fixes #3118.

Co-authored-by: Craig Disselkoen <[email protected]>
Co-authored-by: veetaha <[email protected]>

4 files changed, 19 insertions, 17 deletions

diff --git a/crates/ra_project_model/Cargo.toml b/crates/ra_project_model/Cargo.toml
index 5e651fe70..626478468 100644
--- a/crates/ra_project_model/Cargo.toml
+++ b/crates/ra_project_model/Cargo.toml
@@ -14,8 +14,9 @@ rustc-hash = "1.1.0"
 cargo_metadata = "0.9.1"
 
 ra_arena = { path = "../ra_arena" }
-ra_db = { path = "../ra_db" }
 ra_cfg = { path = "../ra_cfg" }
+ra_db = { path = "../ra_db" }
+ra_env = { path = "../ra_env" }
 ra_proc_macro =  { path = "../ra_proc_macro" }
 
 serde = { version = "1.0.106", features = ["derive"] }
diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs
index 16fff9f1c..eb9f33ee8 100644
--- a/crates/ra_project_model/src/cargo_workspace.rs
+++ b/crates/ra_project_model/src/cargo_workspace.rs
@@ -1,7 +1,6 @@
 //! FIXME: write short doc here
 
 use std::{
-    env,
     ffi::OsStr,
     ops,
     path::{Path, PathBuf},
@@ -12,6 +11,7 @@ use anyhow::{Context, Result};
 use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId};
 use ra_arena::{Arena, Idx};
 use ra_db::Edition;
+use ra_env::get_path_for_executable;
 use rustc_hash::FxHashMap;
 
 /// `CargoWorkspace` represents the logical structure of, well, a Cargo
@@ -146,12 +146,8 @@ impl CargoWorkspace {
         cargo_toml: &Path,
         cargo_features: &CargoConfig,
     ) -> Result<CargoWorkspace> {
-        let _ = Command::new(cargo_binary())
-            .arg("--version")
-            .output()
-            .context("failed to run `cargo --version`, is `cargo` in PATH?")?;
-
         let mut meta = MetadataCommand::new();
+        meta.cargo_path(get_path_for_executable("cargo")?);
         meta.manifest_path(cargo_toml);
         if cargo_features.all_features {
             meta.features(CargoOpt::AllFeatures);
@@ -293,7 +289,7 @@ pub fn load_extern_resources(
     cargo_toml: &Path,
     cargo_features: &CargoConfig,
 ) -> Result<ExternResources> {
-    let mut cmd = Command::new(cargo_binary());
+    let mut cmd = Command::new(get_path_for_executable("cargo")?);
     cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml);
     if cargo_features.all_features {
         cmd.arg("--all-features");
@@ -347,7 +343,3 @@ fn is_dylib(path: &Path) -> bool {
         Some(ext) => matches!(ext.as_str(), "dll" | "dylib" | "so"),
     }
 }
-
-fn cargo_binary() -> String {
-    env::var("CARGO").unwrap_or_else(|_| "cargo".to_string())
-}
diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs
index c226ffa57..88a6ffb2a 100644
--- a/crates/ra_project_model/src/lib.rs
+++ b/crates/ra_project_model/src/lib.rs
@@ -14,6 +14,7 @@ use std::{
 use anyhow::{bail, Context, Result};
 use ra_cfg::CfgOptions;
 use ra_db::{CrateGraph, CrateName, Edition, Env, ExternSource, ExternSourceId, FileId};
+use ra_env::get_path_for_executable;
 use rustc_hash::FxHashMap;
 use serde_json::from_reader;
 
@@ -569,7 +570,7 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions {
 
     match (|| -> Result<String> {
         // `cfg(test)` and `cfg(debug_assertion)` are handled outside, so we suppress them here.
-        let mut cmd = Command::new("rustc");
+        let mut cmd = Command::new(get_path_for_executable("rustc")?);
         cmd.args(&["--print", "cfg", "-O"]);
         if let Some(target) = target {
             cmd.args(&["--target", target.as_str()]);
diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs
index 55ff5ad80..11c26ad89 100644
--- a/crates/ra_project_model/src/sysroot.rs
+++ b/crates/ra_project_model/src/sysroot.rs
@@ -8,6 +8,7 @@ use std::{
 };
 
 use ra_arena::{Arena, Idx};
+use ra_env::get_path_for_executable;
 
 #[derive(Default, Debug, Clone)]
 pub struct Sysroot {
@@ -88,9 +89,14 @@ fn create_command_text(program: &str, args: &[&str]) -> String {
     format!("{} {}", program, args.join(" "))
 }
 
-fn run_command_in_cargo_dir(cargo_toml: &Path, program: &str, args: &[&str]) -> Result<Output> {
+fn run_command_in_cargo_dir(
+    cargo_toml: impl AsRef<Path>,
+    program: impl AsRef<Path>,
+    args: &[&str],
+) -> Result<Output> {
+    let program = program.as_ref().as_os_str().to_str().expect("Invalid Unicode in path");
     let output = Command::new(program)
-        .current_dir(cargo_toml.parent().unwrap())
+        .current_dir(cargo_toml.as_ref().parent().unwrap())
         .args(args)
         .output()
         .context(format!("{} failed", create_command_text(program, args)))?;
@@ -114,13 +120,15 @@ fn get_or_install_rust_src(cargo_toml: &Path) -> Result<PathBuf> {
     if let Ok(path) = env::var("RUST_SRC_PATH") {
         return Ok(path.into());
     }
-    let rustc_output = run_command_in_cargo_dir(cargo_toml, "rustc", &["--print", "sysroot"])?;
+    let rustc = get_path_for_executable("rustc")?;
+    let rustc_output = run_command_in_cargo_dir(cargo_toml, &rustc, &["--print", "sysroot"])?;
     let stdout = String::from_utf8(rustc_output.stdout)?;
     let sysroot_path = Path::new(stdout.trim());
     let src_path = sysroot_path.join("lib/rustlib/src/rust/src");
 
     if !src_path.exists() {
-        run_command_in_cargo_dir(cargo_toml, "rustup", &["component", "add", "rust-src"])?;
+        let rustup = get_path_for_executable("rustup")?;
+        run_command_in_cargo_dir(cargo_toml, &rustup, &["component", "add", "rust-src"])?;
     }
     if !src_path.exists() {
         bail!(