Skip to content

Commit

Permalink
Auto merge of rust-lang#17549 - Veykril:runnables-fix, r=Veykril
Browse files Browse the repository at this point in the history
fix: Fix runnables being incorrectly constructed

I've misunderstood parts of the code here which caused runnables to arbitrarily break :) (I have yet to understand the conditions that made them break though, there is some odd caching involved I feel like ...)
Fixes rust-lang/rust-analyzer#17402
  • Loading branch information
bors committed Jul 6, 2024
2 parents 162eced + 807fdd6 commit e907f81
Show file tree
Hide file tree
Showing 8 changed files with 223 additions and 113 deletions.
8 changes: 0 additions & 8 deletions src/tools/rust-analyzer/editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,6 @@
{
"title": "general",
"properties": {
"rust-analyzer.cargoRunner": {
"type": [
"null",
"string"
],
"default": null,
"description": "Custom cargo runner extension ID."
},
"rust-analyzer.restartServerOnConfigChange": {
"markdownDescription": "Whether to restart the server automatically when certain settings that require a restart are changed.",
"default": false,
Expand Down
28 changes: 21 additions & 7 deletions src/tools/rust-analyzer/editors/code/src/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,20 @@ async function getServer(
config: Config,
state: PersistentState,
): Promise<string | undefined> {
const packageJson: {
version: string;
releaseTag: string | null;
enableProposedApi: boolean | undefined;
} = context.extension.packageJSON;

const explicitPath = process.env["__RA_LSP_SERVER_DEBUG"] ?? config.serverPath;
if (explicitPath) {
if (explicitPath.startsWith("~/")) {
return os.homedir() + explicitPath.slice("~".length);
}
return explicitPath;
}
if (config.package.releaseTag === null) return "rust-analyzer";
if (packageJson.releaseTag === null) return "rust-analyzer";

const ext = process.platform === "win32" ? ".exe" : "";
const bundled = vscode.Uri.joinPath(context.extensionUri, "server", `rust-analyzer${ext}`);
Expand All @@ -54,8 +60,15 @@ async function getServer(
if (bundledExists) {
let server = bundled;
if (await isNixOs()) {
server = await getNixOsServer(config, ext, state, bundled, server);
await state.updateServerVersion(config.package.version);
server = await getNixOsServer(
context.globalStorageUri,
packageJson.version,
ext,
state,
bundled,
server,
);
await state.updateServerVersion(packageJson.version);
}
return server.fsPath;
}
Expand Down Expand Up @@ -86,19 +99,20 @@ export function isValidExecutable(path: string, extraEnv: Env): boolean {
}

async function getNixOsServer(
config: Config,
globalStorageUri: vscode.Uri,
version: string,
ext: string,
state: PersistentState,
bundled: vscode.Uri,
server: vscode.Uri,
) {
await vscode.workspace.fs.createDirectory(config.globalStorageUri).then();
const dest = vscode.Uri.joinPath(config.globalStorageUri, `rust-analyzer${ext}`);
await vscode.workspace.fs.createDirectory(globalStorageUri).then();
const dest = vscode.Uri.joinPath(globalStorageUri, `rust-analyzer${ext}`);
let exists = await vscode.workspace.fs.stat(dest).then(
() => true,
() => false,
);
if (exists && config.package.version !== state.serverVersion) {
if (exists && version !== state.serverVersion) {
await vscode.workspace.fs.delete(dest);
exists = false;
}
Expand Down
27 changes: 7 additions & 20 deletions src/tools/rust-analyzer/editors/code/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as path from "path";
import * as vscode from "vscode";
import { type Env, log, unwrapUndefinable, expectNotUndefined } from "./util";
import type { JsonProject } from "./rust_project";
import type { Disposable } from "./ctx";

export type RunnableEnvCfgItem = {
mask?: string;
Expand All @@ -29,22 +30,9 @@ export class Config {
(opt) => `${this.rootSection}.${opt}`,
);

readonly package: {
version: string;
releaseTag: string | null;
enableProposedApi: boolean | undefined;
} = vscode.extensions.getExtension(this.extensionId)!.packageJSON;

readonly globalStorageUri: vscode.Uri;

constructor(ctx: vscode.ExtensionContext) {
this.globalStorageUri = ctx.globalStorageUri;
constructor(disposables: Disposable[]) {
this.discoveredWorkspaces = [];
vscode.workspace.onDidChangeConfiguration(
this.onDidChangeConfiguration,
this,
ctx.subscriptions,
);
vscode.workspace.onDidChangeConfiguration(this.onDidChangeConfiguration, this, disposables);
this.refreshLogging();
this.configureLanguage();
}
Expand All @@ -55,7 +43,10 @@ export class Config {

private refreshLogging() {
log.setEnabled(this.traceExtension ?? false);
log.info("Extension version:", this.package.version);
log.info(
"Extension version:",
vscode.extensions.getExtension(this.extensionId)!.packageJSON.version,
);

const cfg = Object.entries(this.cfg).filter(([_, val]) => !(val instanceof Function));
log.info("Using configuration", Object.fromEntries(cfg));
Expand Down Expand Up @@ -277,10 +268,6 @@ export class Config {
return this.get<string[]>("runnables.problemMatcher") || [];
}

get cargoRunner() {
return this.get<string | undefined>("cargoRunner");
}

get testExplorer() {
return this.get<boolean | undefined>("testExplorer");
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rust-analyzer/editors/code/src/ctx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class Ctx implements RustAnalyzerExtensionApi {
extCtx.subscriptions.push(this);
this.version = extCtx.extension.packageJSON.version ?? "<unknown>";
this._serverVersion = "<not running>";
this.config = new Config(extCtx);
this.config = new Config(extCtx.subscriptions);
this.statusBar = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left);
if (this.config.testExplorer) {
this.testController = vscode.tests.createTestController(
Expand Down
22 changes: 14 additions & 8 deletions src/tools/rust-analyzer/editors/code/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,26 +111,31 @@ export async function createTaskFromRunnable(
runnable: ra.Runnable,
config: Config,
): Promise<vscode.Task> {
let definition: tasks.RustTargetDefinition;
const target = vscode.workspace.workspaceFolders?.[0];

let definition: tasks.TaskDefinition;
let options;
let cargo;
if (runnable.kind === "cargo") {
const runnableArgs = runnable.args;
let args = createCargoArgs(runnableArgs);

let program: string;
if (runnableArgs.overrideCargo) {
// Split on spaces to allow overrides like "wrapper cargo".
const cargoParts = runnableArgs.overrideCargo.split(" ");

program = unwrapUndefinable(cargoParts[0]);
cargo = unwrapUndefinable(cargoParts[0]);
args = [...cargoParts.slice(1), ...args];
} else {
program = await toolchain.cargoPath();
cargo = await toolchain.cargoPath();
}

definition = {
type: tasks.CARGO_TASK_TYPE,
command: program,
args,
command: unwrapUndefinable(args[0]),
args: args.slice(1),
};
options = {
cwd: runnableArgs.workspaceRoot || ".",
env: prepareEnv(runnable.label, runnableArgs, config.runnablesExtraEnv),
};
Expand All @@ -140,13 +145,14 @@ export async function createTaskFromRunnable(
type: tasks.SHELL_TASK_TYPE,
command: runnableArgs.program,
args: runnableArgs.args,
};
options = {
cwd: runnableArgs.cwd,
env: prepareBaseEnv(),
};
}

const target = vscode.workspace.workspaceFolders?.[0];
const exec = await tasks.targetToExecution(definition, config.cargoRunner, true);
const exec = await tasks.targetToExecution(definition, options, cargo);
const task = await tasks.buildRustTask(
target,
definition,
Expand Down
102 changes: 37 additions & 65 deletions src/tools/rust-analyzer/editors/code/src/tasks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as vscode from "vscode";
import type { Config } from "./config";
import { log, unwrapUndefinable } from "./util";
import * as toolchain from "./toolchain";

// This ends up as the `type` key in tasks.json. RLS also uses `cargo` and
Expand All @@ -10,21 +9,21 @@ export const SHELL_TASK_TYPE = "shell";

export const RUST_TASK_SOURCE = "rust";

export type RustTargetDefinition = {
export type TaskDefinition = vscode.TaskDefinition & {
readonly type: typeof CARGO_TASK_TYPE | typeof SHELL_TASK_TYPE;
} & vscode.TaskDefinition &
RustTarget;
export type RustTarget = {
// The command to run, usually `cargo`.
command: string;
// Additional arguments passed to the command.
args?: string[];
// The working directory to run the command in.
cwd?: string;
// The shell environment.
env?: { [key: string]: string };
command: string;
};

export type CargoTaskDefinition = {
env?: Record<string, string>;
type: typeof CARGO_TASK_TYPE;
} & TaskDefinition;

function isCargoTask(definition: vscode.TaskDefinition): definition is CargoTaskDefinition {
return definition.type === CARGO_TASK_TYPE;
}

class RustTaskProvider implements vscode.TaskProvider {
private readonly config: Config;

Expand Down Expand Up @@ -58,13 +57,13 @@ class RustTaskProvider implements vscode.TaskProvider {
for (const workspaceTarget of vscode.workspace.workspaceFolders) {
for (const def of defs) {
const definition = {
command: cargo,
args: [def.command],
};
const exec = await targetToExecution(definition, this.config.cargoRunner);
command: def.command,
type: CARGO_TASK_TYPE,
} as const;
const exec = await targetToExecution(definition, {}, cargo);
const vscodeTask = await buildRustTask(
workspaceTarget,
{ ...definition, type: CARGO_TASK_TYPE },
definition,
`cargo ${def.command}`,
this.config.problemMatcher,
exec,
Expand All @@ -81,23 +80,13 @@ class RustTaskProvider implements vscode.TaskProvider {
// VSCode calls this for every cargo task in the user's tasks.json,
// we need to inform VSCode how to execute that command by creating
// a ShellExecution for it.
if (task.definition.type === CARGO_TASK_TYPE) {
const taskDefinition = task.definition as RustTargetDefinition;
const cargo = await toolchain.cargoPath();
const exec = await targetToExecution(
{
command: cargo,
args: [taskDefinition.command].concat(taskDefinition.args || []),
cwd: taskDefinition.cwd,
env: taskDefinition.env,
},
this.config.cargoRunner,
);
return await buildRustTask(
if (isCargoTask(task.definition)) {
const exec = await targetToExecution(task.definition, { env: task.definition.env });
return buildRustTask(
task.scope,
taskDefinition,
task.definition,
task.name,
this.config.problemMatcher,
task.problemMatchers,
exec,
);
}
Expand All @@ -108,7 +97,7 @@ class RustTaskProvider implements vscode.TaskProvider {

export async function buildRustTask(
scope: vscode.WorkspaceFolder | vscode.TaskScope | undefined,
definition: RustTargetDefinition,
definition: TaskDefinition,
name: string,
problemMatcher: string[],
exec: vscode.ProcessExecution | vscode.ShellExecution,
Expand All @@ -126,40 +115,23 @@ export async function buildRustTask(
}

export async function targetToExecution(
definition: RustTarget,
customRunner?: string,
throwOnError: boolean = false,
definition: TaskDefinition,
options?: {
env?: { [key: string]: string };
cwd?: string;
},
cargo?: string,
): Promise<vscode.ProcessExecution | vscode.ShellExecution> {
if (customRunner) {
const runnerCommand = `${customRunner}.buildShellExecution`;

try {
const runnerArgs = {
kind: CARGO_TASK_TYPE,
args: definition.args,
cwd: definition.cwd,
env: definition.env,
};
const customExec = await vscode.commands.executeCommand(runnerCommand, runnerArgs);
if (customExec) {
if (customExec instanceof vscode.ShellExecution) {
return customExec;
} else {
log.debug("Invalid cargo ShellExecution", customExec);
throw "Invalid cargo ShellExecution.";
}
}
// fallback to default processing
} catch (e) {
if (throwOnError) throw `Cargo runner '${customRunner}' failed! ${e}`;
// fallback to default processing
}
let command, args;
if (isCargoTask(definition)) {
// FIXME: The server should provide cargo
command = cargo || (await toolchain.cargoPath());
args = [definition.command].concat(definition.args || []);
} else {
command = definition.command;
args = definition.args || [];
}
const args = unwrapUndefinable(definition.args);
return new vscode.ProcessExecution(definition.command, args, {
cwd: definition.cwd,
env: definition.env,
});
return new vscode.ProcessExecution(command, args, options);
}

export function activateTaskProvider(config: Config): vscode.Disposable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export async function getTests(ctx: Context) {
USING_MY_VAR: "test test test",
MY_VAR: "test",
};
const actualEnv = await substituteVariablesInEnv(envJson);
const actualEnv = substituteVariablesInEnv(envJson);
assert.deepStrictEqual(actualEnv, expectedEnv);
});

Expand All @@ -34,7 +34,7 @@ export async function getTests(ctx: Context) {
E_IS_ISOLATED: "test",
F_USES_E: "test",
};
const actualEnv = await substituteVariablesInEnv(envJson);
const actualEnv = substituteVariablesInEnv(envJson);
assert.deepStrictEqual(actualEnv, expectedEnv);
});

Expand All @@ -47,7 +47,7 @@ export async function getTests(ctx: Context) {
USING_EXTERNAL_VAR: "test test test",
};

const actualEnv = await substituteVariablesInEnv(envJson);
const actualEnv = substituteVariablesInEnv(envJson);
assert.deepStrictEqual(actualEnv, expectedEnv);
delete process.env["TEST_VARIABLE"];
});
Expand All @@ -56,7 +56,7 @@ export async function getTests(ctx: Context) {
const envJson = {
USING_VSCODE_VAR: "${workspaceFolderBasename}",
};
const actualEnv = await substituteVariablesInEnv(envJson);
const actualEnv = substituteVariablesInEnv(envJson);
assert.deepStrictEqual(actualEnv["USING_VSCODE_VAR"], "code");
});
});
Expand Down
Loading

0 comments on commit e907f81

Please sign in to comment.