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.
This commit is contained in:
veetaha 2020-05-31 05:13:08 +03:00
parent a419cedb1c
commit d605ec9c32
13 changed files with 133 additions and 93 deletions

1
Cargo.lock generated
View File

@ -1365,7 +1365,6 @@ dependencies = [
"ra_syntax",
"ra_text_edit",
"ra_tt",
"ra_toolchain",
"ra_vfs",
"rand",
"relative-path",

View File

@ -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"

View File

@ -121,12 +121,21 @@ pub struct RunnablesParams {
pub position: Option<Position>,
}
// 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<String>,
pub extra_args: Vec<String>,
pub env: FxHashMap<String, String>,

View File

@ -40,7 +40,6 @@
world::WorldSnapshot,
LspError, Result,
};
use anyhow::Context;
pub fn handle_analyzer_status(world: WorldSnapshot, _: ()) -> Result<String> {
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<String> {
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: {

View File

@ -58,10 +58,6 @@ fn completes_items_from_standard_library() {
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": {

View File

@ -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[];

View File

@ -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");

View File

@ -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 };

View File

@ -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,
};

View File

@ -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 },

View File

@ -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<CompilationArtifact[]> {
const artifacts: CompilationArtifact[] = [];
@ -77,7 +79,7 @@ export class Cargo {
}
async executableFromArgs(args: readonly string[]): Promise<string> {
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<number> {
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;
}
}

View File

@ -99,3 +99,21 @@ export function isValidExecutable(path: string): boolean {
export function setContextValue(key: string, value: any): Thenable<void> {
return vscode.commands.executeCommand('setContext', key, value);
}
/**
* Returns a higher-order function that caches the results of invoking the
* underlying function.
*/
export function memoize<Ret, TThis, Param extends string>(func: (this: TThis, arg: Param) => Ret) {
const cache = new Map<string, Ret>();
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;
};
}

View File

@ -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);