From 030d78345fa79af07f8ebd89a9d244576fac992b Mon Sep 17 00:00:00 2001 From: veetaha Date: Sat, 23 May 2020 04:58:22 +0300 Subject: Fix invoking cargo without consulting CARGO or standard installation paths --- editors/code/src/cargo.ts | 4 ++-- editors/code/src/tasks.ts | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) (limited to 'editors/code') diff --git a/editors/code/src/cargo.ts b/editors/code/src/cargo.ts index a55b2f860..46cd3d777 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 1366c76d6..c22d69362 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 +} -- cgit v1.2.3 From d605ec9c321392d9c7ee4b440c560e1e405d92e6 Mon Sep 17 00:00:00 2001 From: veetaha Date: Sun, 31 May 2020 05:13:08 +0300 Subject: 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. --- editors/code/src/cargo.ts | 151 ---------------------- 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/toolchain.ts | 174 ++++++++++++++++++++++++++ editors/code/src/util.ts | 18 +++ editors/code/tests/unit/launch_config.test.ts | 14 +-- 8 files changed, 208 insertions(+), 163 deletions(-) delete mode 100644 editors/code/src/cargo.ts create mode 100644 editors/code/src/toolchain.ts (limited to 'editors/code') diff --git a/editors/code/src/cargo.ts b/editors/code/src/cargo.ts deleted file mode 100644 index 46cd3d777..000000000 --- a/editors/code/src/cargo.ts +++ /dev/null @@ -1,151 +0,0 @@ -import * as cp from 'child_process'; -import * as os from 'os'; -import * as path from 'path'; -import * as readline from 'readline'; -import { OutputChannel } from 'vscode'; -import { isValidExecutable } from './util'; - -interface CompilationArtifact { - fileName: string; - name: string; - kind: string; - isTest: boolean; -} - -export interface ArtifactSpec { - cargoArgs: string[]; - 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) { } - - private async getArtifacts(spec: ArtifactSpec): Promise { - const artifacts: CompilationArtifact[] = []; - - try { - await this.runCargo(spec.cargoArgs, - message => { - if (message.reason === 'compiler-artifact' && message.executable) { - const isBinary = message.target.crate_types.includes('bin'); - const isBuildScript = message.target.kind.includes('custom-build'); - if ((isBinary && !isBuildScript) || message.profile.test) { - artifacts.push({ - fileName: message.executable, - name: message.target.name, - kind: message.target.kind[0], - isTest: message.profile.test - }); - } - } else if (message.reason === 'compiler-message') { - this.output.append(message.message.rendered); - } - }, - stderr => this.output.append(stderr), - ); - } catch (err) { - this.output.show(true); - throw new Error(`Cargo invocation has failed: ${err}`); - } - - return spec.filter?.(artifacts) ?? artifacts; - } - - async executableFromArgs(args: readonly string[]): Promise { - const artifacts = await this.getArtifacts(artifactSpec(args)); - - if (artifacts.length === 0) { - throw new Error('No compilation artifacts'); - } else if (artifacts.length > 1) { - throw new Error('Multiple compilation artifacts are not supported.'); - } - - return artifacts[0].fileName; - } - - private runCargo( - cargoArgs: string[], - onStdoutJson: (obj: any) => void, - 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, { - stdio: ['ignore', 'pipe', 'pipe'], - cwd: this.rootFolder - }); - - cargo.on('error', err => reject(new Error(`could not launch cargo: ${err}`))); - - cargo.stderr.on('data', chunk => onStderrString(chunk.toString())); - - const rl = readline.createInterface({ input: cargo.stdout }); - rl.on('line', line => { - const message = JSON.parse(line); - onStdoutJson(message); - }); - - cargo.on('exit', (exitCode, _) => { - if (exitCode === 0) - resolve(exitCode); - else - reject(new Error(`exit code: ${exitCode}.`)); - }); - }); - } -} - -// 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.` - ); -} diff --git a/editors/code/src/debug.ts b/editors/code/src/debug.ts index 027504ecd..bdec5b735 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 4da12eb30..3e0b60699 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 2a7a429cf..401cb76af 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 c22d69362..9748824df 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/toolchain.ts b/editors/code/src/toolchain.ts new file mode 100644 index 000000000..80a7915e9 --- /dev/null +++ b/editors/code/src/toolchain.ts @@ -0,0 +1,174 @@ +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 { log, memoize } from './util'; + +interface CompilationArtifact { + fileName: string; + name: string; + kind: string; + isTest: boolean; +} + +export interface ArtifactSpec { + cargoArgs: string[]; + filter?: (artifacts: CompilationArtifact[]) => CompilationArtifact[]; +} + +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[] = []; + + try { + await this.runCargo(spec.cargoArgs, + message => { + if (message.reason === 'compiler-artifact' && message.executable) { + const isBinary = message.target.crate_types.includes('bin'); + const isBuildScript = message.target.kind.includes('custom-build'); + if ((isBinary && !isBuildScript) || message.profile.test) { + artifacts.push({ + fileName: message.executable, + name: message.target.name, + kind: message.target.kind[0], + isTest: message.profile.test + }); + } + } else if (message.reason === 'compiler-message') { + this.output.append(message.message.rendered); + } + }, + stderr => this.output.append(stderr), + ); + } catch (err) { + this.output.show(true); + throw new Error(`Cargo invocation has failed: ${err}`); + } + + return spec.filter?.(artifacts) ?? artifacts; + } + + async executableFromArgs(args: readonly string[]): Promise { + const artifacts = await this.getArtifacts(Cargo.artifactSpec(args)); + + if (artifacts.length === 0) { + throw new Error('No compilation artifacts'); + } else if (artifacts.length > 1) { + throw new Error('Multiple compilation artifacts are not supported.'); + } + + return artifacts[0].fileName; + } + + private runCargo( + cargoArgs: string[], + onStdoutJson: (obj: any) => void, + onStderrString: (data: string) => void + ): Promise { + return new Promise((resolve, reject) => { + const cargo = cp.spawn(cargoPath(), cargoArgs, { + stdio: ['ignore', 'pipe', 'pipe'], + cwd: this.rootFolder + }); + + cargo.on('error', err => reject(new Error(`could not launch cargo: ${err}`))); + + cargo.stderr.on('data', chunk => onStderrString(chunk.toString())); + + const rl = readline.createInterface({ input: cargo.stdout }); + rl.on('line', line => { + const message = JSON.parse(line); + onStdoutJson(message); + }); + + cargo.on('exit', (exitCode, _) => { + if (exitCode === 0) + resolve(exitCode); + else + reject(new Error(`exit code: ${exitCode}.`)); + }); + }); + } +} + +/** 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 352ef9162..fe3fb71cd 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 d5cf1b74e..68794d53e 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); -- cgit v1.2.3