From 030d78345fa79af07f8ebd89a9d244576fac992b Mon Sep 17 00:00:00 2001 From: veetaha Date: Sat, 23 May 2020 04:58:22 +0300 Subject: [PATCH 1/3] Fix invoking cargo without consulting CARGO or standard installation paths --- Cargo.lock | 1 + crates/rust-analyzer/Cargo.toml | 1 + crates/rust-analyzer/src/main_loop/handlers.rs | 8 +++++++- editors/code/src/cargo.ts | 4 ++-- editors/code/src/tasks.ts | 7 +++++-- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index def4ed45e0..7de784c1cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1364,6 +1364,7 @@ dependencies = [ "ra_syntax", "ra_text_edit", "ra_tt", + "ra_toolchain", "ra_vfs", "rand", "relative-path", diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index 65b487db3b..2e49448cc6 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -48,6 +48,7 @@ hir = { path = "../ra_hir", package = "ra_hir" } hir_def = { path = "../ra_hir_def", package = "ra_hir_def" } hir_ty = { path = "../ra_hir_ty", package = "ra_hir_ty" } ra_proc_macro_srv = { path = "../ra_proc_macro_srv" } +ra_toolchain = { path = "../ra_toolchain" } [target.'cfg(windows)'.dependencies] winapi = "0.3.8" diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 1f910ff82b..1b5b3325c0 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -40,6 +40,7 @@ use crate::{ world::WorldSnapshot, LspError, Result, }; +use anyhow::Context; pub fn handle_analyzer_status(world: WorldSnapshot, _: ()) -> Result { let _p = profile("handle_analyzer_status"); @@ -982,10 +983,15 @@ fn to_lsp_runnable( target.map_or_else(|| "run binary".to_string(), |t| format!("run {}", t)) } }; + let cargo_path = ra_toolchain::cargo() + .to_str() + .context("Path to cargo executable contains invalid UTF8 characters")? + .to_owned(); + Ok(lsp_ext::Runnable { range: to_proto::range(&line_index, runnable.range), label, - bin: "cargo".to_string(), + bin: cargo_path, args, extra_args, env: { diff --git a/editors/code/src/cargo.ts b/editors/code/src/cargo.ts index a55b2f860f..46cd3d7778 100644 --- a/editors/code/src/cargo.ts +++ b/editors/code/src/cargo.ts @@ -126,8 +126,8 @@ export class Cargo { } } -// Mirrors `ra_env::get_path_for_executable` implementation -function getCargoPathOrFail(): string { +// Mirrors `ra_toolchain::cargo()` implementation +export function getCargoPathOrFail(): string { const envVar = process.env.CARGO; const executableName = "cargo"; diff --git a/editors/code/src/tasks.ts b/editors/code/src/tasks.ts index 1366c76d6b..c22d693623 100644 --- a/editors/code/src/tasks.ts +++ b/editors/code/src/tasks.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import { getCargoPathOrFail } from "./cargo"; // This ends up as the `type` key in tasks.json. RLS also uses `cargo` and // our configuration should be compatible with it so use the same key. @@ -24,6 +25,8 @@ class CargoTaskProvider implements vscode.TaskProvider { // set of tasks that always exist. These tasks cannot be removed in // tasks.json - only tweaked. + const cargoPath = getCargoPathOrFail(); + return [ { command: 'build', group: vscode.TaskGroup.Build }, { command: 'check', group: vscode.TaskGroup.Build }, @@ -46,7 +49,7 @@ class CargoTaskProvider implements vscode.TaskProvider { `cargo ${command}`, 'rust', // What to do when this command is executed. - new vscode.ShellExecution('cargo', [command]), + new vscode.ShellExecution(cargoPath, [command]), // Problem matchers. ['$rustc'], ); @@ -80,4 +83,4 @@ class CargoTaskProvider implements vscode.TaskProvider { export function activateTaskProvider(target: vscode.WorkspaceFolder): vscode.Disposable { const provider = new CargoTaskProvider(target); return vscode.tasks.registerTaskProvider(TASK_TYPE, provider); -} \ No newline at end of file +} From a419cedb1cc661349a022262c8b03993e063252f Mon Sep 17 00:00:00 2001 From: veetaha Date: Sat, 23 May 2020 16:31:56 +0300 Subject: [PATCH 2/3] Fix tests, apply code review proposals --- Cargo.lock | 1 + crates/ra_proc_macro_srv/Cargo.toml | 1 + crates/ra_proc_macro_srv/src/tests/utils.rs | 3 +-- crates/rust-analyzer/src/main_loop/handlers.rs | 17 ++++++++++------- crates/rust-analyzer/tests/heavy_tests/main.rs | 14 +++++++++----- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7de784c1cd..e1da335f5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1119,6 +1119,7 @@ dependencies = [ "memmap", "ra_mbe", "ra_proc_macro", + "ra_toolchain", "ra_tt", "serde_derive", "test_utils", diff --git a/crates/ra_proc_macro_srv/Cargo.toml b/crates/ra_proc_macro_srv/Cargo.toml index bb30032785..5821029450 100644 --- a/crates/ra_proc_macro_srv/Cargo.toml +++ b/crates/ra_proc_macro_srv/Cargo.toml @@ -22,3 +22,4 @@ cargo_metadata = "0.10.0" difference = "2.0.0" # used as proc macro test target serde_derive = "1.0.106" +ra_toolchain = { path = "../ra_toolchain" } diff --git a/crates/ra_proc_macro_srv/src/tests/utils.rs b/crates/ra_proc_macro_srv/src/tests/utils.rs index 84348b5def..8d85f2d8a7 100644 --- a/crates/ra_proc_macro_srv/src/tests/utils.rs +++ b/crates/ra_proc_macro_srv/src/tests/utils.rs @@ -2,7 +2,6 @@ use crate::dylib; use crate::ProcMacroSrv; -pub use difference::Changeset as __Changeset; use ra_proc_macro::ListMacrosTask; use std::str::FromStr; use test_utils::assert_eq_text; @@ -13,7 +12,7 @@ mod fixtures { // Use current project metadata to get the proc-macro dylib path pub fn dylib_path(crate_name: &str, version: &str) -> std::path::PathBuf { - let command = Command::new("cargo") + let command = Command::new(ra_toolchain::cargo()) .args(&["check", "--message-format", "json"]) .output() .unwrap() diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 1b5b3325c0..d42cfa3000 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -427,7 +427,7 @@ pub fn handle_runnables( res.push(lsp_ext::Runnable { range: Default::default(), label: format!("cargo {} -p {}", cmd, spec.package), - bin: "cargo".to_string(), + bin: cargo_path()?, args: vec![cmd.to_string(), "--package".to_string(), spec.package.clone()], extra_args: Vec::new(), env: FxHashMap::default(), @@ -439,7 +439,7 @@ pub fn handle_runnables( res.push(lsp_ext::Runnable { range: Default::default(), label: "cargo check --workspace".to_string(), - bin: "cargo".to_string(), + bin: cargo_path()?, args: vec!["check".to_string(), "--workspace".to_string()], extra_args: Vec::new(), env: FxHashMap::default(), @@ -450,6 +450,13 @@ pub fn handle_runnables( Ok(res) } +fn cargo_path() -> Result { + Ok(ra_toolchain::cargo() + .to_str() + .context("Path to `cargo` executable contains invalid UTF8 characters")? + .to_owned()) +} + pub fn handle_completion( world: WorldSnapshot, params: lsp_types::CompletionParams, @@ -983,15 +990,11 @@ fn to_lsp_runnable( target.map_or_else(|| "run binary".to_string(), |t| format!("run {}", t)) } }; - let cargo_path = ra_toolchain::cargo() - .to_str() - .context("Path to cargo executable contains invalid UTF8 characters")? - .to_owned(); Ok(lsp_ext::Runnable { range: to_proto::range(&line_index, runnable.range), label, - bin: cargo_path, + bin: cargo_path()?, args, extra_args, env: { diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index 405ddb3629..a31580c867 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -58,6 +58,10 @@ use std::collections::Spam; eprintln!("completion took {:?}", completion_start.elapsed()); } +fn cargo_path() -> String { + ra_toolchain::cargo().to_str().unwrap().to_owned() +} + #[test] fn test_runnables_no_project() { if skip_slow_tests() { @@ -79,7 +83,7 @@ fn foo() { { "args": [ "test" ], "extraArgs": [ "foo", "--nocapture" ], - "bin": "cargo", + "bin": cargo_path(), "env": { "RUST_BACKTRACE": "short" }, "cwd": null, "label": "test foo", @@ -91,7 +95,7 @@ fn foo() { { "args": ["check", "--workspace"], "extraArgs": [], - "bin": "cargo", + "bin": cargo_path(), "env": {}, "cwd": null, "label": "cargo check --workspace", @@ -141,7 +145,7 @@ fn main() {} { "args": [ "test", "--package", "foo", "--test", "spam" ], "extraArgs": [ "test_eggs", "--exact", "--nocapture" ], - "bin": "cargo", + "bin": cargo_path(), "env": { "RUST_BACKTRACE": "short" }, "label": "test test_eggs", "range": { @@ -153,7 +157,7 @@ fn main() {} { "args": [ "check", "--package", "foo" ], "extraArgs": [], - "bin": "cargo", + "bin": cargo_path(), "env": {}, "label": "cargo check -p foo", "range": { @@ -165,7 +169,7 @@ fn main() {} { "args": [ "test", "--package", "foo" ], "extraArgs": [], - "bin": "cargo", + "bin": cargo_path(), "env": {}, "label": "cargo test -p foo", "range": { From d605ec9c321392d9c7ee4b440c560e1e405d92e6 Mon Sep 17 00:00:00 2001 From: veetaha Date: Sun, 31 May 2020 05:13:08 +0300 Subject: [PATCH 3/3] Change Runnable.bin -> Runnable.kind As per matklad, we now pass the responsibility for finding the binary to the frontend. Also, added caching for finding the binary path to reduce the amount of filesystem interactions. --- Cargo.lock | 1 - crates/rust-analyzer/Cargo.toml | 1 - crates/rust-analyzer/src/lsp_ext.rs | 11 +- .../rust-analyzer/src/main_loop/handlers.rs | 14 +- .../rust-analyzer/tests/heavy_tests/main.rs | 14 +- docs/dev/lsp-extensions.md | 2 +- editors/code/src/debug.ts | 2 +- editors/code/src/lsp_ext.ts | 5 +- editors/code/src/run.ts | 3 +- editors/code/src/tasks.ts | 4 +- editors/code/src/{cargo.ts => toolchain.ts} | 137 ++++++++++-------- editors/code/src/util.ts | 18 +++ editors/code/tests/unit/launch_config.test.ts | 14 +- 13 files changed, 133 insertions(+), 93 deletions(-) rename editors/code/src/{cargo.ts => toolchain.ts} (53%) diff --git a/Cargo.lock b/Cargo.lock index e1da335f5f..07f18c7607 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1365,7 +1365,6 @@ dependencies = [ "ra_syntax", "ra_text_edit", "ra_tt", - "ra_toolchain", "ra_vfs", "rand", "relative-path", diff --git a/crates/rust-analyzer/Cargo.toml b/crates/rust-analyzer/Cargo.toml index 2e49448cc6..65b487db3b 100644 --- a/crates/rust-analyzer/Cargo.toml +++ b/crates/rust-analyzer/Cargo.toml @@ -48,7 +48,6 @@ hir = { path = "../ra_hir", package = "ra_hir" } hir_def = { path = "../ra_hir_def", package = "ra_hir_def" } hir_ty = { path = "../ra_hir_ty", package = "ra_hir_ty" } ra_proc_macro_srv = { path = "../ra_proc_macro_srv" } -ra_toolchain = { path = "../ra_toolchain" } [target.'cfg(windows)'.dependencies] winapi = "0.3.8" diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index acb1dacb6b..173c23b9e5 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -121,12 +121,21 @@ pub struct RunnablesParams { pub position: Option, } +// Must strictly correspond to the executable name +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "lowercase")] +pub enum RunnableKind { + Cargo, + Rustc, + Rustup, +} + #[derive(Deserialize, Serialize, Debug)] #[serde(rename_all = "camelCase")] pub struct Runnable { pub range: Range, pub label: String, - pub bin: String, + pub kind: RunnableKind, pub args: Vec, pub extra_args: Vec, pub env: FxHashMap, diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index d42cfa3000..bc7c7f1ef5 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -40,7 +40,6 @@ use crate::{ world::WorldSnapshot, LspError, Result, }; -use anyhow::Context; pub fn handle_analyzer_status(world: WorldSnapshot, _: ()) -> Result { let _p = profile("handle_analyzer_status"); @@ -427,7 +426,7 @@ pub fn handle_runnables( res.push(lsp_ext::Runnable { range: Default::default(), label: format!("cargo {} -p {}", cmd, spec.package), - bin: cargo_path()?, + kind: lsp_ext::RunnableKind::Cargo, args: vec![cmd.to_string(), "--package".to_string(), spec.package.clone()], extra_args: Vec::new(), env: FxHashMap::default(), @@ -439,7 +438,7 @@ pub fn handle_runnables( res.push(lsp_ext::Runnable { range: Default::default(), label: "cargo check --workspace".to_string(), - bin: cargo_path()?, + kind: lsp_ext::RunnableKind::Cargo, args: vec!["check".to_string(), "--workspace".to_string()], extra_args: Vec::new(), env: FxHashMap::default(), @@ -450,13 +449,6 @@ pub fn handle_runnables( Ok(res) } -fn cargo_path() -> Result { - Ok(ra_toolchain::cargo() - .to_str() - .context("Path to `cargo` executable contains invalid UTF8 characters")? - .to_owned()) -} - pub fn handle_completion( world: WorldSnapshot, params: lsp_types::CompletionParams, @@ -994,7 +986,7 @@ fn to_lsp_runnable( Ok(lsp_ext::Runnable { range: to_proto::range(&line_index, runnable.range), label, - bin: cargo_path()?, + kind: lsp_ext::RunnableKind::Cargo, args, extra_args, env: { diff --git a/crates/rust-analyzer/tests/heavy_tests/main.rs b/crates/rust-analyzer/tests/heavy_tests/main.rs index a31580c867..8b473ff74c 100644 --- a/crates/rust-analyzer/tests/heavy_tests/main.rs +++ b/crates/rust-analyzer/tests/heavy_tests/main.rs @@ -58,10 +58,6 @@ use std::collections::Spam; eprintln!("completion took {:?}", completion_start.elapsed()); } -fn cargo_path() -> String { - ra_toolchain::cargo().to_str().unwrap().to_owned() -} - #[test] fn test_runnables_no_project() { if skip_slow_tests() { @@ -83,7 +79,7 @@ fn foo() { { "args": [ "test" ], "extraArgs": [ "foo", "--nocapture" ], - "bin": cargo_path(), + "kind": "cargo", "env": { "RUST_BACKTRACE": "short" }, "cwd": null, "label": "test foo", @@ -95,7 +91,7 @@ fn foo() { { "args": ["check", "--workspace"], "extraArgs": [], - "bin": cargo_path(), + "kind": "cargo", "env": {}, "cwd": null, "label": "cargo check --workspace", @@ -145,7 +141,7 @@ fn main() {} { "args": [ "test", "--package", "foo", "--test", "spam" ], "extraArgs": [ "test_eggs", "--exact", "--nocapture" ], - "bin": cargo_path(), + "kind": "cargo", "env": { "RUST_BACKTRACE": "short" }, "label": "test test_eggs", "range": { @@ -157,7 +153,7 @@ fn main() {} { "args": [ "check", "--package", "foo" ], "extraArgs": [], - "bin": cargo_path(), + "kind": "cargo", "env": {}, "label": "cargo check -p foo", "range": { @@ -169,7 +165,7 @@ fn main() {} { "args": [ "test", "--package", "foo" ], "extraArgs": [], - "bin": cargo_path(), + "kind": "cargo", "env": {}, "label": "cargo test -p foo", "range": { diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index dbc95be387..d06da355d3 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -417,7 +417,7 @@ interface Runnable { /// The label to show in the UI. label: string; /// The following fields describe a process to spawn. - bin: string; + kind: "cargo" | "rustc" | "rustup"; args: string[]; /// Args for cargo after `--`. extraArgs: string[]; diff --git a/editors/code/src/debug.ts b/editors/code/src/debug.ts index 027504ecd2..bdec5b7357 100644 --- a/editors/code/src/debug.ts +++ b/editors/code/src/debug.ts @@ -3,7 +3,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; import * as ra from './lsp_ext'; -import { Cargo } from './cargo'; +import { Cargo } from './toolchain'; import { Ctx } from "./ctx"; const debugOutput = vscode.window.createOutputChannel("Debug"); diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 4da12eb309..3e0b606997 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -45,10 +45,13 @@ export interface RunnablesParams { textDocument: lc.TextDocumentIdentifier; position: lc.Position | null; } + +export type RunnableKind = "cargo" | "rustc" | "rustup"; + export interface Runnable { range: lc.Range; label: string; - bin: string; + kind: RunnableKind; args: string[]; extraArgs: string[]; env: { [key: string]: string }; diff --git a/editors/code/src/run.ts b/editors/code/src/run.ts index 2a7a429cfa..401cb76af6 100644 --- a/editors/code/src/run.ts +++ b/editors/code/src/run.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import * as lc from 'vscode-languageclient'; import * as ra from './lsp_ext'; +import * as toolchain from "./toolchain"; import { Ctx, Cmd } from './ctx'; import { startDebugSession, getDebugConfiguration } from './debug'; @@ -175,7 +176,7 @@ export function createTask(spec: ra.Runnable): vscode.Task { const definition: CargoTaskDefinition = { type: 'cargo', label: spec.label, - command: spec.bin, + command: toolchain.getPathForExecutable(spec.kind), args: spec.extraArgs ? [...spec.args, '--', ...spec.extraArgs] : spec.args, env: spec.env, }; diff --git a/editors/code/src/tasks.ts b/editors/code/src/tasks.ts index c22d693623..9748824df3 100644 --- a/editors/code/src/tasks.ts +++ b/editors/code/src/tasks.ts @@ -1,5 +1,5 @@ import * as vscode from 'vscode'; -import { getCargoPathOrFail } from "./cargo"; +import * as toolchain from "./toolchain"; // This ends up as the `type` key in tasks.json. RLS also uses `cargo` and // our configuration should be compatible with it so use the same key. @@ -25,7 +25,7 @@ class CargoTaskProvider implements vscode.TaskProvider { // set of tasks that always exist. These tasks cannot be removed in // tasks.json - only tweaked. - const cargoPath = getCargoPathOrFail(); + const cargoPath = toolchain.cargoPath(); return [ { command: 'build', group: vscode.TaskGroup.Build }, diff --git a/editors/code/src/cargo.ts b/editors/code/src/toolchain.ts similarity index 53% rename from editors/code/src/cargo.ts rename to editors/code/src/toolchain.ts index 46cd3d7778..80a7915e90 100644 --- a/editors/code/src/cargo.ts +++ b/editors/code/src/toolchain.ts @@ -1,9 +1,10 @@ import * as cp from 'child_process'; import * as os from 'os'; import * as path from 'path'; +import * as fs from 'fs'; import * as readline from 'readline'; import { OutputChannel } from 'vscode'; -import { isValidExecutable } from './util'; +import { log, memoize } from './util'; interface CompilationArtifact { fileName: string; @@ -17,34 +18,35 @@ export interface ArtifactSpec { filter?: (artifacts: CompilationArtifact[]) => CompilationArtifact[]; } -export function artifactSpec(args: readonly string[]): ArtifactSpec { - const cargoArgs = [...args, "--message-format=json"]; - - // arguments for a runnable from the quick pick should be updated. - // see crates\rust-analyzer\src\main_loop\handlers.rs, handle_code_lens - switch (cargoArgs[0]) { - case "run": cargoArgs[0] = "build"; break; - case "test": { - if (!cargoArgs.includes("--no-run")) { - cargoArgs.push("--no-run"); - } - break; - } - } - - const result: ArtifactSpec = { cargoArgs: cargoArgs }; - if (cargoArgs[0] === "test") { - // for instance, `crates\rust-analyzer\tests\heavy_tests\main.rs` tests - // produce 2 artifacts: {"kind": "bin"} and {"kind": "test"} - result.filter = (artifacts) => artifacts.filter(it => it.isTest); - } - - return result; -} - export class Cargo { constructor(readonly rootFolder: string, readonly output: OutputChannel) { } + // Made public for testing purposes + static artifactSpec(args: readonly string[]): ArtifactSpec { + const cargoArgs = [...args, "--message-format=json"]; + + // arguments for a runnable from the quick pick should be updated. + // see crates\rust-analyzer\src\main_loop\handlers.rs, handle_code_lens + switch (cargoArgs[0]) { + case "run": cargoArgs[0] = "build"; break; + case "test": { + if (!cargoArgs.includes("--no-run")) { + cargoArgs.push("--no-run"); + } + break; + } + } + + const result: ArtifactSpec = { cargoArgs: cargoArgs }; + if (cargoArgs[0] === "test") { + // for instance, `crates\rust-analyzer\tests\heavy_tests\main.rs` tests + // produce 2 artifacts: {"kind": "bin"} and {"kind": "test"} + result.filter = (artifacts) => artifacts.filter(it => it.isTest); + } + + return result; + } + private async getArtifacts(spec: ArtifactSpec): Promise { const artifacts: CompilationArtifact[] = []; @@ -77,7 +79,7 @@ export class Cargo { } async executableFromArgs(args: readonly string[]): Promise { - const artifacts = await this.getArtifacts(artifactSpec(args)); + const artifacts = await this.getArtifacts(Cargo.artifactSpec(args)); if (artifacts.length === 0) { throw new Error('No compilation artifacts'); @@ -94,14 +96,7 @@ export class Cargo { onStderrString: (data: string) => void ): Promise { return new Promise((resolve, reject) => { - let cargoPath; - try { - cargoPath = getCargoPathOrFail(); - } catch (err) { - return reject(err); - } - - const cargo = cp.spawn(cargoPath, cargoArgs, { + const cargo = cp.spawn(cargoPath(), cargoArgs, { stdio: ['ignore', 'pipe', 'pipe'], cwd: this.rootFolder }); @@ -126,26 +121,54 @@ export class Cargo { } } -// Mirrors `ra_toolchain::cargo()` implementation -export function getCargoPathOrFail(): string { - const envVar = process.env.CARGO; - const executableName = "cargo"; - - if (envVar) { - if (isValidExecutable(envVar)) return envVar; - - throw new Error(`\`${envVar}\` environment variable points to something that's not a valid executable`); - } - - if (isValidExecutable(executableName)) return executableName; - - const standardLocation = path.join(os.homedir(), '.cargo', 'bin', executableName); - - if (isValidExecutable(standardLocation)) return standardLocation; - - throw new Error( - `Failed to find \`${executableName}\` executable. ` + - `Make sure \`${executableName}\` is in \`$PATH\`, ` + - `or set \`${envVar}\` to point to a valid executable.` - ); +/** Mirrors `ra_toolchain::cargo()` implementation */ +export function cargoPath(): string { + return getPathForExecutable("cargo"); +} + +/** Mirrors `ra_toolchain::get_path_for_executable()` implementation */ +export const getPathForExecutable = memoize( + // We apply caching to decrease file-system interactions + (executableName: "cargo" | "rustc" | "rustup"): string => { + { + const envVar = process.env[executableName.toUpperCase()]; + if (envVar) return envVar; + } + + if (lookupInPath(executableName)) return executableName; + + try { + // hmm, `os.homedir()` seems to be infallible + // it is not mentioned in docs and cannot be infered by the type signature... + const standardPath = path.join(os.homedir(), ".cargo", "bin", executableName); + + if (isFile(standardPath)) return standardPath; + } catch (err) { + log.error("Failed to read the fs info", err); + } + return executableName; + } +); + +function lookupInPath(exec: string): boolean { + const paths = process.env.PATH ?? "";; + + const candidates = paths.split(path.delimiter).flatMap(dirInPath => { + const candidate = path.join(dirInPath, exec); + return os.type() === "Windows_NT" + ? [candidate, `${candidate}.exe`] + : [candidate]; + }); + + return candidates.some(isFile); +} + +function isFile(suspectPath: string): boolean { + // It is not mentionned in docs, but `statSync()` throws an error when + // the path doesn't exist + try { + return fs.statSync(suspectPath).isFile(); + } catch { + return false; + } } diff --git a/editors/code/src/util.ts b/editors/code/src/util.ts index 352ef9162f..fe3fb71cd7 100644 --- a/editors/code/src/util.ts +++ b/editors/code/src/util.ts @@ -99,3 +99,21 @@ export function isValidExecutable(path: string): boolean { export function setContextValue(key: string, value: any): Thenable { return vscode.commands.executeCommand('setContext', key, value); } + +/** + * Returns a higher-order function that caches the results of invoking the + * underlying function. + */ +export function memoize(func: (this: TThis, arg: Param) => Ret) { + const cache = new Map(); + + return function(this: TThis, arg: Param) { + const cached = cache.get(arg); + if (cached) return cached; + + const result = func.call(this, arg); + cache.set(arg, result); + + return result; + }; +} diff --git a/editors/code/tests/unit/launch_config.test.ts b/editors/code/tests/unit/launch_config.test.ts index d5cf1b74ea..68794d53ed 100644 --- a/editors/code/tests/unit/launch_config.test.ts +++ b/editors/code/tests/unit/launch_config.test.ts @@ -1,25 +1,25 @@ import * as assert from 'assert'; -import * as cargo from '../../src/cargo'; +import { Cargo } from '../../src/toolchain'; suite('Launch configuration', () => { suite('Lens', () => { test('A binary', async () => { - const args = cargo.artifactSpec(["build", "--package", "pkg_name", "--bin", "pkg_name"]); + const args = Cargo.artifactSpec(["build", "--package", "pkg_name", "--bin", "pkg_name"]); assert.deepEqual(args.cargoArgs, ["build", "--package", "pkg_name", "--bin", "pkg_name", "--message-format=json"]); assert.deepEqual(args.filter, undefined); }); test('One of Multiple Binaries', async () => { - const args = cargo.artifactSpec(["build", "--package", "pkg_name", "--bin", "bin1"]); + const args = Cargo.artifactSpec(["build", "--package", "pkg_name", "--bin", "bin1"]); assert.deepEqual(args.cargoArgs, ["build", "--package", "pkg_name", "--bin", "bin1", "--message-format=json"]); assert.deepEqual(args.filter, undefined); }); test('A test', async () => { - const args = cargo.artifactSpec(["test", "--package", "pkg_name", "--lib", "--no-run"]); + const args = Cargo.artifactSpec(["test", "--package", "pkg_name", "--lib", "--no-run"]); assert.deepEqual(args.cargoArgs, ["test", "--package", "pkg_name", "--lib", "--no-run", "--message-format=json"]); assert.notDeepEqual(args.filter, undefined); @@ -28,7 +28,7 @@ suite('Launch configuration', () => { suite('QuickPick', () => { test('A binary', async () => { - const args = cargo.artifactSpec(["run", "--package", "pkg_name", "--bin", "pkg_name"]); + const args = Cargo.artifactSpec(["run", "--package", "pkg_name", "--bin", "pkg_name"]); assert.deepEqual(args.cargoArgs, ["build", "--package", "pkg_name", "--bin", "pkg_name", "--message-format=json"]); assert.deepEqual(args.filter, undefined); @@ -36,14 +36,14 @@ suite('Launch configuration', () => { test('One of Multiple Binaries', async () => { - const args = cargo.artifactSpec(["run", "--package", "pkg_name", "--bin", "bin2"]); + const args = Cargo.artifactSpec(["run", "--package", "pkg_name", "--bin", "bin2"]); assert.deepEqual(args.cargoArgs, ["build", "--package", "pkg_name", "--bin", "bin2", "--message-format=json"]); assert.deepEqual(args.filter, undefined); }); test('A test', async () => { - const args = cargo.artifactSpec(["test", "--package", "pkg_name", "--lib"]); + const args = Cargo.artifactSpec(["test", "--package", "pkg_name", "--lib"]); assert.deepEqual(args.cargoArgs, ["test", "--package", "pkg_name", "--lib", "--message-format=json", "--no-run"]); assert.notDeepEqual(args.filter, undefined);