From 6713be0b130670324c61d9deb38b7b6aee6a8bac Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 12:25:36 +0200 Subject: [PATCH 1/3] Rename ra_env -> ra_toolchain --- Cargo.lock | 20 +++++++++---------- crates/ra_flycheck/Cargo.toml | 2 +- crates/ra_flycheck/src/lib.rs | 2 +- crates/ra_project_model/Cargo.toml | 2 +- .../ra_project_model/src/cargo_workspace.rs | 2 +- crates/ra_project_model/src/lib.rs | 2 +- crates/ra_project_model/src/sysroot.rs | 4 ++-- crates/{ra_env => ra_toolchain}/Cargo.toml | 2 +- crates/{ra_env => ra_toolchain}/src/lib.rs | 8 +++++--- 9 files changed, 23 insertions(+), 21 deletions(-) rename crates/{ra_env => ra_toolchain}/Cargo.toml (85%) rename crates/{ra_env => ra_toolchain}/src/lib.rs (97%) diff --git a/Cargo.lock b/Cargo.lock index 36cff6402f..656969c874 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -957,14 +957,6 @@ dependencies = [ "test_utils", ] -[[package]] -name = "ra_env" -version = "0.1.0" -dependencies = [ - "anyhow", - "home", -] - [[package]] name = "ra_flycheck" version = "0.1.0" @@ -975,7 +967,7 @@ dependencies = [ "jod-thread", "log", "lsp-types", - "ra_env", + "ra_toolchain", "serde_json", ] @@ -1180,8 +1172,8 @@ dependencies = [ "ra_arena", "ra_cfg", "ra_db", - "ra_env", "ra_proc_macro", + "ra_toolchain", "rustc-hash", "serde", "serde_json", @@ -1213,6 +1205,14 @@ dependencies = [ "text-size", ] +[[package]] +name = "ra_toolchain" +version = "0.1.0" +dependencies = [ + "anyhow", + "home", +] + [[package]] name = "ra_tt" version = "0.1.0" diff --git a/crates/ra_flycheck/Cargo.toml b/crates/ra_flycheck/Cargo.toml index d0f7fb2dcf..03e5571484 100644 --- a/crates/ra_flycheck/Cargo.toml +++ b/crates/ra_flycheck/Cargo.toml @@ -14,7 +14,7 @@ log = "0.4.8" cargo_metadata = "0.9.1" serde_json = "1.0.48" jod-thread = "0.1.1" -ra_env = { path = "../ra_env" } +ra_toolchain = { path = "../ra_toolchain" } [dev-dependencies] insta = "0.16.0" diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index d8b727b0ea..561657edbd 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -16,7 +16,7 @@ use lsp_types::{ CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; diff --git a/crates/ra_project_model/Cargo.toml b/crates/ra_project_model/Cargo.toml index 6264784689..a32a5daabd 100644 --- a/crates/ra_project_model/Cargo.toml +++ b/crates/ra_project_model/Cargo.toml @@ -16,7 +16,7 @@ cargo_metadata = "0.9.1" ra_arena = { path = "../ra_arena" } ra_cfg = { path = "../ra_cfg" } ra_db = { path = "../ra_db" } -ra_env = { path = "../ra_env" } +ra_toolchain = { path = "../ra_toolchain" } 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 eb9f33ee89..9683bfcc0d 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -11,7 +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 ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; /// `CargoWorkspace` represents the logical structure of, well, a Cargo diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 88a6ffb2a0..4f0b9c77ee 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -14,7 +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 ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; use serde_json::from_reader; diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 11c26ad891..2b628c2a3e 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -1,14 +1,14 @@ //! FIXME: write short doc here -use anyhow::{bail, Context, Result}; use std::{ env, ops, path::{Path, PathBuf}, process::{Command, Output}, }; +use anyhow::{bail, Context, Result}; use ra_arena::{Arena, Idx}; -use ra_env::get_path_for_executable; +use ra_toolchain::get_path_for_executable; #[derive(Default, Debug, Clone)] pub struct Sysroot { diff --git a/crates/ra_env/Cargo.toml b/crates/ra_toolchain/Cargo.toml similarity index 85% rename from crates/ra_env/Cargo.toml rename to crates/ra_toolchain/Cargo.toml index f0a401be53..fbad1073e0 100644 --- a/crates/ra_env/Cargo.toml +++ b/crates/ra_toolchain/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2018" -name = "ra_env" +name = "ra_toolchain" version = "0.1.0" authors = ["rust-analyzer developers"] diff --git a/crates/ra_env/src/lib.rs b/crates/ra_toolchain/src/lib.rs similarity index 97% rename from crates/ra_env/src/lib.rs rename to crates/ra_toolchain/src/lib.rs index 413da19827..110c92c889 100644 --- a/crates/ra_env/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -1,11 +1,13 @@ //! This crate contains a single public function //! [`get_path_for_executable`](fn.get_path_for_executable.html). //! See docs there for more information. +use std::{ + env, + path::{Path, PathBuf}, + process::Command, +}; use anyhow::{bail, Result}; -use std::env; -use std::path::{Path, PathBuf}; -use std::process::Command; /// Return a `PathBuf` to use for the given executable. /// From 7c0409e0c7249fe793b5d05829fcd984d06ec770 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 12:34:39 +0200 Subject: [PATCH 2/3] Cleanup --- crates/ra_toolchain/src/lib.rs | 57 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/crates/ra_toolchain/src/lib.rs b/crates/ra_toolchain/src/lib.rs index 110c92c889..afa76619d8 100644 --- a/crates/ra_toolchain/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -14,7 +14,8 @@ use anyhow::{bail, Result}; /// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that /// gives a valid Cargo executable; or it may return a full path to a valid /// Cargo. -pub fn get_path_for_executable(executable_name: impl AsRef) -> Result { +pub fn get_path_for_executable(executable_name: &'static str) -> Result { + assert!(executable_name == "rustc" || executable_name == "cargo"); // The current implementation checks three places for an executable to use: // 1) Appropriate environment variable (erroring if this is set but not a usable executable) // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc @@ -23,46 +24,46 @@ pub fn get_path_for_executable(executable_name: impl AsRef) -> Result` // example: for cargo, this tries ~/.cargo/bin/cargo // It seems that this is a reasonable place to try for cargo, rustc, and rustup - let executable_name = executable_name.as_ref(); let env_var = executable_name.to_ascii_uppercase(); if let Ok(path) = env::var(&env_var) { - if is_valid_executable(&path) { + return if is_valid_executable(&path) { Ok(path.into()) } else { bail!( "`{}` environment variable points to something that's not a valid executable", env_var ) - } - } else { - if is_valid_executable(executable_name) { - return Ok(executable_name.into()); - } - if let Some(mut path) = ::home::home_dir() { - path.push(".cargo"); - path.push("bin"); - path.push(executable_name); - if is_valid_executable(&path) { - return Ok(path); - } - } - // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly - // for VSCode, even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; - // but launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - bail!( - "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", - executable_name, executable_name, env_var - ) + }; } + + if is_valid_executable(executable_name) { + return Ok(executable_name.into()); + } + + if let Some(mut path) = home::home_dir() { + path.push(".cargo"); + path.push("bin"); + path.push(executable_name); + if is_valid_executable(&path) { + return Ok(path); + } + } + // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly + // for VSCode, even if they are set correctly in a terminal. + // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode + // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; + // but launching VSCode from Dock does not inherit environment variables from a terminal. + // For more discussion, see #3118. + bail!( + "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", + executable_name, executable_name, env_var + ) } /// Does the given `Path` point to a usable executable? /// /// (assumes the executable takes a `--version` switch and writes to stdout, /// which is true for `cargo`, `rustc`, and `rustup`) -fn is_valid_executable(p: impl AsRef) -> bool { - Command::new(p.as_ref()).arg("--version").output().is_ok() +fn is_valid_executable(p: &'static str) -> bool { + Command::new(p).arg("--version").output().is_ok() } From ecff5dc141046c5b9e40639657247a05fb9b0344 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 8 May 2020 14:54:29 +0200 Subject: [PATCH 3/3] Cleanup --- Cargo.lock | 1 - crates/ra_flycheck/src/lib.rs | 7 +- .../ra_project_model/src/cargo_workspace.rs | 5 +- crates/ra_project_model/src/lib.rs | 32 ++++----- crates/ra_project_model/src/sysroot.rs | 49 +++---------- crates/ra_toolchain/Cargo.toml | 1 - crates/ra_toolchain/src/lib.rs | 69 +++++++++---------- 7 files changed, 64 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 656969c874..41855f22e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1209,7 +1209,6 @@ dependencies = [ name = "ra_toolchain" version = "0.1.0" dependencies = [ - "anyhow", "home", ] diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index 561657edbd..68dcee2851 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -16,7 +16,6 @@ use lsp_types::{ CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; -use ra_toolchain::get_path_for_executable; use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; @@ -216,10 +215,10 @@ impl FlycheckThread { let mut cmd = match &self.config { FlycheckConfig::CargoCommand { command, all_targets, all_features, extra_args } => { - let mut cmd = Command::new(get_path_for_executable("cargo").unwrap()); + let mut cmd = Command::new(ra_toolchain::cargo()); cmd.arg(command); - cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]); - cmd.arg(self.workspace_root.join("Cargo.toml")); + cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]) + .arg(self.workspace_root.join("Cargo.toml")); if *all_targets { cmd.arg("--all-targets"); } diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs index 9683bfcc0d..082af4f969 100644 --- a/crates/ra_project_model/src/cargo_workspace.rs +++ b/crates/ra_project_model/src/cargo_workspace.rs @@ -11,7 +11,6 @@ use anyhow::{Context, Result}; use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId}; use ra_arena::{Arena, Idx}; use ra_db::Edition; -use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; /// `CargoWorkspace` represents the logical structure of, well, a Cargo @@ -147,7 +146,7 @@ impl CargoWorkspace { cargo_features: &CargoConfig, ) -> Result { let mut meta = MetadataCommand::new(); - meta.cargo_path(get_path_for_executable("cargo")?); + meta.cargo_path(ra_toolchain::cargo()); meta.manifest_path(cargo_toml); if cargo_features.all_features { meta.features(CargoOpt::AllFeatures); @@ -289,7 +288,7 @@ pub fn load_extern_resources( cargo_toml: &Path, cargo_features: &CargoConfig, ) -> Result { - let mut cmd = Command::new(get_path_for_executable("cargo")?); + let mut cmd = Command::new(ra_toolchain::cargo()); cmd.args(&["check", "--message-format=json", "--manifest-path"]).arg(cargo_toml); if cargo_features.all_features { cmd.arg("--all-features"); diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 4f0b9c77ee..5a0a87ed7d 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -8,13 +8,12 @@ use std::{ fs::{read_dir, File, ReadDir}, io::{self, BufReader}, path::{Path, PathBuf}, - process::Command, + process::{Command, Output}, }; use anyhow::{bail, Context, Result}; use ra_cfg::CfgOptions; use ra_db::{CrateGraph, CrateName, Edition, Env, ExternSource, ExternSourceId, FileId}; -use ra_toolchain::get_path_for_executable; use rustc_hash::FxHashMap; use serde_json::from_reader; @@ -568,25 +567,18 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions { } } - match (|| -> Result { + let rustc_cfgs = || -> Result { // `cfg(test)` and `cfg(debug_assertion)` are handled outside, so we suppress them here. - let mut cmd = Command::new(get_path_for_executable("rustc")?); + let mut cmd = Command::new(ra_toolchain::rustc()); cmd.args(&["--print", "cfg", "-O"]); if let Some(target) = target { cmd.args(&["--target", target.as_str()]); } - let output = cmd.output().context("Failed to get output from rustc --print cfg -O")?; - if !output.status.success() { - bail!( - "rustc --print cfg -O exited with exit code ({})", - output - .status - .code() - .map_or(String::from("no exit code"), |code| format!("{}", code)) - ); - } + let output = output(cmd)?; Ok(String::from_utf8(output.stdout)?) - })() { + }(); + + match rustc_cfgs { Ok(rustc_cfgs) => { for line in rustc_cfgs.lines() { match line.find('=') { @@ -599,8 +591,16 @@ pub fn get_rustc_cfg_options(target: Option<&String>) -> CfgOptions { } } } - Err(e) => log::error!("failed to get rustc cfgs: {}", e), + Err(e) => log::error!("failed to get rustc cfgs: {:#}", e), } cfg_options } + +fn output(mut cmd: Command) -> Result { + let output = cmd.output().with_context(|| format!("{:?} failed", cmd))?; + if !output.status.success() { + bail!("{:?} failed, {}", cmd, output.status) + } + Ok(output) +} diff --git a/crates/ra_project_model/src/sysroot.rs b/crates/ra_project_model/src/sysroot.rs index 2b628c2a3e..a8a196e64c 100644 --- a/crates/ra_project_model/src/sysroot.rs +++ b/crates/ra_project_model/src/sysroot.rs @@ -3,12 +3,13 @@ use std::{ env, ops, path::{Path, PathBuf}, - process::{Command, Output}, + process::Command, }; -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; use ra_arena::{Arena, Idx}; -use ra_toolchain::get_path_for_executable; + +use crate::output; #[derive(Default, Debug, Clone)] pub struct Sysroot { @@ -85,50 +86,22 @@ impl Sysroot { } } -fn create_command_text(program: &str, args: &[&str]) -> String { - format!("{} {}", program, args.join(" ")) -} - -fn run_command_in_cargo_dir( - cargo_toml: impl AsRef, - program: impl AsRef, - args: &[&str], -) -> Result { - let program = program.as_ref().as_os_str().to_str().expect("Invalid Unicode in path"); - let output = Command::new(program) - .current_dir(cargo_toml.as_ref().parent().unwrap()) - .args(args) - .output() - .context(format!("{} failed", create_command_text(program, args)))?; - if !output.status.success() { - match output.status.code() { - Some(code) => bail!( - "failed to run the command: '{}' exited with code {}", - create_command_text(program, args), - code - ), - None => bail!( - "failed to run the command: '{}' terminated by signal", - create_command_text(program, args) - ), - }; - } - Ok(output) -} - fn get_or_install_rust_src(cargo_toml: &Path) -> Result { if let Ok(path) = env::var("RUST_SRC_PATH") { return Ok(path.into()); } - let rustc = get_path_for_executable("rustc")?; - let rustc_output = run_command_in_cargo_dir(cargo_toml, &rustc, &["--print", "sysroot"])?; + let current_dir = cargo_toml.parent().unwrap(); + let mut rustc = Command::new(ra_toolchain::rustc()); + rustc.current_dir(current_dir).args(&["--print", "sysroot"]); + let rustc_output = output(rustc)?; 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() { - let rustup = get_path_for_executable("rustup")?; - run_command_in_cargo_dir(cargo_toml, &rustup, &["component", "add", "rust-src"])?; + let mut rustup = Command::new(ra_toolchain::rustup()); + rustup.current_dir(current_dir).args(&["component", "add", "rust-src"]); + let _output = output(rustup)?; } if !src_path.exists() { bail!( diff --git a/crates/ra_toolchain/Cargo.toml b/crates/ra_toolchain/Cargo.toml index fbad1073e0..1873fbe167 100644 --- a/crates/ra_toolchain/Cargo.toml +++ b/crates/ra_toolchain/Cargo.toml @@ -5,5 +5,4 @@ version = "0.1.0" authors = ["rust-analyzer developers"] [dependencies] -anyhow = "1.0.26" home = "0.5.3" diff --git a/crates/ra_toolchain/src/lib.rs b/crates/ra_toolchain/src/lib.rs index afa76619d8..3c307a0eac 100644 --- a/crates/ra_toolchain/src/lib.rs +++ b/crates/ra_toolchain/src/lib.rs @@ -1,21 +1,26 @@ //! This crate contains a single public function //! [`get_path_for_executable`](fn.get_path_for_executable.html). //! See docs there for more information. -use std::{ - env, - path::{Path, PathBuf}, - process::Command, -}; +use std::{env, iter, path::PathBuf}; -use anyhow::{bail, Result}; +pub fn cargo() -> PathBuf { + get_path_for_executable("cargo") +} + +pub fn rustc() -> PathBuf { + get_path_for_executable("rustc") +} + +pub fn rustup() -> PathBuf { + get_path_for_executable("rustup") +} /// Return a `PathBuf` to use for the given executable. /// /// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that /// gives a valid Cargo executable; or it may return a full path to a valid /// Cargo. -pub fn get_path_for_executable(executable_name: &'static str) -> Result { - assert!(executable_name == "rustc" || executable_name == "cargo"); +fn get_path_for_executable(executable_name: &'static str) -> PathBuf { // The current implementation checks three places for an executable to use: // 1) Appropriate environment variable (erroring if this is set but not a usable executable) // example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc @@ -25,45 +30,35 @@ pub fn get_path_for_executable(executable_name: &'static str) -> Result // example: for cargo, this tries ~/.cargo/bin/cargo // It seems that this is a reasonable place to try for cargo, rustc, and rustup let env_var = executable_name.to_ascii_uppercase(); - if let Ok(path) = env::var(&env_var) { - return if is_valid_executable(&path) { - Ok(path.into()) - } else { - bail!( - "`{}` environment variable points to something that's not a valid executable", - env_var - ) - }; + if let Some(path) = env::var_os(&env_var) { + return path.into(); } - if is_valid_executable(executable_name) { - return Ok(executable_name.into()); + if lookup_in_path(executable_name) { + return executable_name.into(); } if let Some(mut path) = home::home_dir() { path.push(".cargo"); path.push("bin"); path.push(executable_name); - if is_valid_executable(&path) { - return Ok(path); + if path.is_file() { + return path; } } - // This error message may also be caused by $PATH or $CARGO/$RUSTC/etc not being set correctly - // for VSCode, even if they are set correctly in a terminal. - // On macOS in particular, launching VSCode from terminal with `code ` causes VSCode - // to inherit environment variables including $PATH, $CARGO, $RUSTC, etc from that terminal; - // but launching VSCode from Dock does not inherit environment variables from a terminal. - // For more discussion, see #3118. - bail!( - "Failed to find `{}` executable. Make sure `{}` is in `$PATH`, or set `${}` to point to a valid executable.", - executable_name, executable_name, env_var - ) + executable_name.into() } -/// Does the given `Path` point to a usable executable? -/// -/// (assumes the executable takes a `--version` switch and writes to stdout, -/// which is true for `cargo`, `rustc`, and `rustup`) -fn is_valid_executable(p: &'static str) -> bool { - Command::new(p).arg("--version").output().is_ok() +fn lookup_in_path(exec: &str) -> bool { + let paths = env::var_os("PATH").unwrap_or_default(); + let mut candidates = env::split_paths(&paths).flat_map(|path| { + let candidate = path.join(&exec); + let with_exe = if env::consts::EXE_EXTENSION == "" { + None + } else { + Some(candidate.with_extension(env::consts::EXE_EXTENSION)) + }; + iter::once(candidate).chain(with_exe) + }); + candidates.any(|it| it.is_file()) }