Auto merge of #17275 - roife:fix-issue-17012, r=Veykril

Fix inconsistent cwd of `run` and `debug` command in client

Fix #17012. Also related to #13022 and #15993.

When the `kind` of runnable is `bin`, Cargo would use the workspace root as the cwd for the `run` command; otherwise, Cargo defaults to the package root as the cwd for `run`.

Initially, r-a assumed the workspace root as the cwd for all runnables in `debug` command, which led to issue #13022. In this case, during unit testing, the `run` command would use the package root while `debug` would use the workspace root, causing inconsistency.

PR #15993 addressed this problem by using the package root as the cwd for `debug` command. However, it also resulted in an inconsistency: when executing the `run` command within the main fn of a package (whose target is `bin`), Cargo would use the workspace root, whereas `debug` would use the package root, leading to issue #17012.

The preferable approach is to determine the cwd based on the runnable's type. To resolve this, this PR introduces a new `cwd` field within `CargoRunnable`, allowing r-a to decide the appropriate cwd depending on the specific kind of the runnable.
This commit is contained in:
bors 2024-05-24 17:43:35 +00:00
commit a55e8bf09c
8 changed files with 122 additions and 33 deletions

View file

@ -860,6 +860,11 @@ pub(crate) fn handle_runnables(
if cmd == "run" && spec.target_kind != TargetKind::Bin {
continue;
}
let cwd = if cmd != "test" || spec.target_kind == TargetKind::Bin {
spec.workspace_root.clone()
} else {
spec.cargo_toml.parent().to_path_buf()
};
let mut cargo_args =
vec![cmd.to_owned(), "--package".to_owned(), spec.package.clone()];
let all_targets = cmd != "run" && !is_crate_no_std;
@ -876,6 +881,7 @@ pub(crate) fn handle_runnables(
kind: lsp_ext::RunnableKind::Cargo,
args: lsp_ext::CargoRunnable {
workspace_root: Some(spec.workspace_root.clone().into()),
cwd: Some(cwd.into()),
override_cargo: config.override_cargo.clone(),
cargo_args,
cargo_extra_args: config.cargo_extra_args.clone(),
@ -893,6 +899,7 @@ pub(crate) fn handle_runnables(
kind: lsp_ext::RunnableKind::Cargo,
args: lsp_ext::CargoRunnable {
workspace_root: None,
cwd: None,
override_cargo: config.override_cargo,
cargo_args: vec!["check".to_owned(), "--workspace".to_owned()],
cargo_extra_args: config.cargo_extra_args,

View file

@ -441,6 +441,8 @@ pub struct CargoRunnable {
pub override_cargo: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub workspace_root: Option<PathBuf>,
#[serde(skip_serializing_if = "Option::is_none")]
pub cwd: Option<PathBuf>,
// command, --package and --lib stuff
pub cargo_args: Vec<String>,
// user-specified additional cargo args, like `--release`.

View file

@ -1360,6 +1360,10 @@ pub(crate) fn runnable(
let config = snap.config.runnables();
let spec = CargoTargetSpec::for_file(snap, runnable.nav.file_id)?;
let workspace_root = spec.as_ref().map(|it| it.workspace_root.clone());
let cwd = match runnable.kind {
ide::RunnableKind::Bin { .. } => workspace_root.clone().map(|it| it.into()),
_ => spec.as_ref().map(|it| it.cargo_toml.parent().into()),
};
let target = spec.as_ref().map(|s| s.target.clone());
let (cargo_args, executable_args) =
CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg);
@ -1372,6 +1376,7 @@ pub(crate) fn runnable(
kind: lsp_ext::RunnableKind::Cargo,
args: lsp_ext::CargoRunnable {
workspace_root: workspace_root.map(|it| it.into()),
cwd,
override_cargo: config.override_cargo,
cargo_args,
cargo_extra_args: config.cargo_extra_args,

View file

@ -260,6 +260,7 @@ fn main() {}
"executableArgs": ["test_eggs", "--exact", "--show-output"],
"cargoExtraArgs": [],
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo")
},
"kind": "cargo",
@ -279,6 +280,7 @@ fn main() {}
{
"args": {
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo"),
"cargoArgs": [
"test",
@ -325,6 +327,7 @@ fn main() {}
"executableArgs": [],
"cargoExtraArgs": [],
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo")
},
"kind": "cargo",
@ -336,6 +339,7 @@ fn main() {}
"executableArgs": [],
"cargoExtraArgs": [],
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo")
},
"kind": "cargo",
@ -415,6 +419,7 @@ mod tests {
"args": {
"overrideCargo": null,
"workspaceRoot": server.path().join(runnable),
"cwd": server.path().join(runnable),
"cargoArgs": [
"test",
"--package",
@ -432,6 +437,94 @@ mod tests {
}
}
// The main fn in packages should be run from the workspace root
#[test]
fn test_runnables_cwd() {
if skip_slow_tests() {
return;
}
let server = Project::with_fixture(
r#"
//- /foo/Cargo.toml
[workspace]
members = ["mainpkg", "otherpkg"]
//- /foo/mainpkg/Cargo.toml
[package]
name = "mainpkg"
version = "0.1.0"
//- /foo/mainpkg/src/main.rs
fn main() {}
//- /foo/otherpkg/Cargo.toml
[package]
name = "otherpkg"
version = "0.1.0"
//- /foo/otherpkg/src/lib.rs
#[test]
fn otherpkg() {}
"#,
)
.root("foo")
.server()
.wait_until_workspace_is_loaded();
server.request::<Runnables>(
RunnablesParams { text_document: server.doc_id("foo/mainpkg/src/main.rs"), position: None },
json!([
"{...}",
{
"label": "cargo test -p mainpkg --all-targets",
"kind": "cargo",
"args": {
"overrideCargo": null,
"workspaceRoot": server.path().join("foo"),
"cwd": server.path().join("foo"),
"cargoArgs": [
"test",
"--package",
"mainpkg",
"--all-targets"
],
"cargoExtraArgs": [],
"executableArgs": []
},
},
"{...}",
"{...}"
]),
);
server.request::<Runnables>(
RunnablesParams { text_document: server.doc_id("foo/otherpkg/src/lib.rs"), position: None },
json!([
"{...}",
{
"label": "cargo test -p otherpkg --all-targets",
"kind": "cargo",
"args": {
"overrideCargo": null,
"workspaceRoot": server.path().join("foo"),
"cwd": server.path().join("foo").join("otherpkg"),
"cargoArgs": [
"test",
"--package",
"otherpkg",
"--all-targets"
],
"cargoExtraArgs": [],
"executableArgs": []
},
},
"{...}",
"{...}"
]),
);
}
#[test]
fn test_format_document() {
if skip_slow_tests() {

View file

@ -1,5 +1,5 @@
<!---
lsp/ext.rs hash: 422dcc22c2e56166
lsp/ext.rs hash: 1babf76a3c2cef3b
If you need to change the above hash to make the test pass, please check if you
need to adjust this doc as well and ping this issue:
@ -377,6 +377,7 @@ rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look
```typescript
{
workspaceRoot?: string;
cwd?: string;
cargoArgs: string[];
cargoExtraArgs: string[];
executableArgs: string[];

View file

@ -3,7 +3,7 @@ import * as vscode from "vscode";
import * as path from "path";
import type * as ra from "./lsp_ext";
import { Cargo, type ExecutableInfo, getRustcId, getSysroot } from "./toolchain";
import { Cargo, getRustcId, getSysroot } from "./toolchain";
import type { Ctx } from "./ctx";
import { prepareEnv } from "./run";
import { unwrapUndefinable } from "./undefinable";
@ -12,7 +12,6 @@ const debugOutput = vscode.window.createOutputChannel("Debug");
type DebugConfigProvider = (
config: ra.Runnable,
executable: string,
cargoWorkspace: string,
env: Record<string, string>,
sourceFileMap?: Record<string, string>,
) => vscode.DebugConfiguration;
@ -134,7 +133,7 @@ async function getDebugConfiguration(
}
const env = prepareEnv(runnable, ctx.config.runnablesExtraEnv);
const { executable, workspace: cargoWorkspace } = await getDebugExecutableInfo(runnable, env);
const executable = await getDebugExecutable(runnable, env);
let sourceFileMap = debugOptions.sourceFileMap;
if (sourceFileMap === "auto") {
// let's try to use the default toolchain
@ -148,13 +147,7 @@ async function getDebugConfiguration(
}
const provider = unwrapUndefinable(knownEngines[debugEngine.id]);
const debugConfig = provider(
runnable,
simplifyPath(executable),
cargoWorkspace,
env,
sourceFileMap,
);
const debugConfig = provider(runnable, simplifyPath(executable), env, sourceFileMap);
if (debugConfig.type in debugOptions.engineSettings) {
const settingsMap = (debugOptions.engineSettings as any)[debugConfig.type];
for (var key in settingsMap) {
@ -176,21 +169,20 @@ async function getDebugConfiguration(
return debugConfig;
}
async function getDebugExecutableInfo(
async function getDebugExecutable(
runnable: ra.Runnable,
env: Record<string, string>,
): Promise<ExecutableInfo> {
): Promise<string> {
const cargo = new Cargo(runnable.args.workspaceRoot || ".", debugOutput, env);
const executableInfo = await cargo.executableInfoFromArgs(runnable.args.cargoArgs);
const executable = await cargo.executableFromArgs(runnable.args.cargoArgs);
// if we are here, there were no compilation errors.
return executableInfo;
return executable;
}
function getCCppDebugConfig(
runnable: ra.Runnable,
executable: string,
cargoWorkspace: string,
env: Record<string, string>,
sourceFileMap?: Record<string, string>,
): vscode.DebugConfiguration {
@ -200,7 +192,7 @@ function getCCppDebugConfig(
name: runnable.label,
program: executable,
args: runnable.args.executableArgs,
cwd: cargoWorkspace || runnable.args.workspaceRoot,
cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".",
sourceFileMap,
env,
// See https://github.com/rust-lang/rust-analyzer/issues/16901#issuecomment-2024486941
@ -213,7 +205,6 @@ function getCCppDebugConfig(
function getCodeLldbDebugConfig(
runnable: ra.Runnable,
executable: string,
cargoWorkspace: string,
env: Record<string, string>,
sourceFileMap?: Record<string, string>,
): vscode.DebugConfiguration {
@ -223,7 +214,7 @@ function getCodeLldbDebugConfig(
name: runnable.label,
program: executable,
args: runnable.args.executableArgs,
cwd: cargoWorkspace || runnable.args.workspaceRoot,
cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".",
sourceMap: sourceFileMap,
sourceLanguages: ["rust"],
env,
@ -233,7 +224,6 @@ function getCodeLldbDebugConfig(
function getNativeDebugConfig(
runnable: ra.Runnable,
executable: string,
cargoWorkspace: string,
env: Record<string, string>,
_sourceFileMap?: Record<string, string>,
): vscode.DebugConfiguration {
@ -244,7 +234,7 @@ function getNativeDebugConfig(
target: executable,
// See https://github.com/WebFreak001/code-debug/issues/359
arguments: quote(runnable.args.executableArgs),
cwd: cargoWorkspace || runnable.args.workspaceRoot,
cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".",
env,
valuesFormatting: "prettyPrinters",
};

View file

@ -226,6 +226,7 @@ export type Runnable = {
kind: "cargo";
args: {
workspaceRoot?: string;
cwd?: string;
cargoArgs: string[];
cargoExtraArgs: string[];
executableArgs: string[];

View file

@ -9,17 +9,11 @@ import { unwrapUndefinable } from "./undefinable";
interface CompilationArtifact {
fileName: string;
workspace: string;
name: string;
kind: string;
isTest: boolean;
}
export interface ExecutableInfo {
executable: string;
workspace: string;
}
export interface ArtifactSpec {
cargoArgs: string[];
filter?: (artifacts: CompilationArtifact[]) => CompilationArtifact[];
@ -74,7 +68,6 @@ export class Cargo {
artifacts.push({
fileName: message.executable,
name: message.target.name,
workspace: path.dirname(message.manifest_path),
kind: message.target.kind[0],
isTest: message.profile.test,
});
@ -93,7 +86,7 @@ export class Cargo {
return spec.filter?.(artifacts) ?? artifacts;
}
async executableInfoFromArgs(args: readonly string[]): Promise<ExecutableInfo> {
async executableFromArgs(args: readonly string[]): Promise<string> {
const artifacts = await this.getArtifacts(Cargo.artifactSpec(args));
if (artifacts.length === 0) {
@ -103,10 +96,7 @@ export class Cargo {
}
const artifact = unwrapUndefinable(artifacts[0]);
return {
executable: artifact.fileName,
workspace: artifact.workspace,
};
return artifact.fileName;
}
private async runCargo(