Skip to content

fix: Fix runnables being incorrectly constructed #17549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,6 @@
{
"title": "general",
"properties": {
"rust-analyzer.cargoRunner": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also removes this, the custom runner was broken for ages already so clearly no one is depending on it

"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 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 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 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 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 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
8 changes: 4 additions & 4 deletions editors/code/tests/unit/settings.test.ts
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