Skip to content
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

VSCode should honor toolchain overrides that contain a rust-analyzer component for server spawning #17663

Closed
Veykril opened this issue Jul 22, 2024 · 11 comments · Fixed by #17667 or #17705
Assignees
Labels
A-vscode vscode plugin issues C-feature Category: feature request

Comments

@Veykril
Copy link
Member

Veykril commented Jul 22, 2024

We tend to drop support for stable releases somewhat frequently as its difficult to really support more than a handful of rust versions with a rolling release model. This means it becomes the job of the user to pin their rust-analyzer somehow to some version that fits their current roolchain install, that's not ideal. We should offer a setting that when enabled checks for a toolchain override in the workspace and attempts to use its rust-analyzer component if installed when spawning the server.

Addtionally we should probably attempt to let the server know its MSRV and inform the user if their toolchain looks too old.

@Veykril Veykril added A-vscode vscode plugin issues C-feature Category: feature request labels Jul 22, 2024
@Veykril Veykril self-assigned this Jul 22, 2024
@huntc
Copy link
Contributor

huntc commented Jul 22, 2024

Thanks for raising this.

Addtionally we should probably attempt to let the server know its MSRV and inform the user if their toolchain looks too old

Just on the subject of "old toolchains", it is of course quite common for projects to pin their toolchains, particularly once deployed to production. My point is that past toolchain usage is a fact of life and so pointing out to the developer that their toolchain is old is probably not useful. :-)

@Veykril
Copy link
Member Author

Veykril commented Jul 22, 2024

Too old for the current rust-analyzer server that is running, it is useful to point that out, because otherwise we just get issue reports for issues caused by us not supporting something wrt to that old toolchain. If we have a way to let people pin their version to an older compatible toolchain then we can just point them to that and everything is fine.

@huntc
Copy link
Contributor

huntc commented Jul 25, 2024

I don't see that this issue is resolved. I've updated by VSC environment to use the nightly RA. Here's its version info:

rust-analyzer version: 0.4.2048-standalone (eeb192b79 2024-07-24) [/Users/huntc/.vscode/extensions/rust-lang.rust-analyzer-0.4.2048-darwin-arm64/server/rust-analyzer]

So the PR commit is part of this. Yet, if I now add RA to my toolchain files e.g.:

[toolchain]
channel = "nightly-2022-11-22"
components = [ "rustfmt", "clippy", "rust-analyzer" ]
targets = [ "thumbv7em-none-eabihf" ]

...then I'm still seeing errors in the RA output:

2024-07-25T01:38:27.294031Z ERROR Flycheck failed to run the following command: CommandHandle { program: "/Users/huntc/.cargo/bin/cargo", arguments: ["check", "--workspace", "--message-format=json-diagnostic-rendered-ansi", "--manifest-path", "/Users/huntc/Projects/titanclass/matte-ev-charger/firmware/nrf-monitor/Cargo.toml", "--keep-going", "--all-targets"], current_dir: Some("/Users/huntc/Projects/titanclass/matte-ev-charger/firmware/nrf-monitor") }, error=Cargo watcher failed, the command produced no valid metadata (exit code: ExitStatus(unix_wait_status(25856))):
error: the `--keep-going` flag is unstable, pass `-Z unstable-options` to enable it
See https://github.com/rust-lang/cargo/issues/10496 for more information about the `--keep-going` flag.

As a consequence, I continue to get:

proc macro `task` not expanded: No proc-macros present for crate

I've also tried cleaning project folders and restarting RA.

Additional info: My project has a VSC workspace file and each project within is relatively independent of the other. Some of the projects have a toolchain file, some do not. There's a blend of embedded and non-embedded Rust projects here.

@huntc
Copy link
Contributor

huntc commented Jul 25, 2024

Is it that the VSC integration design only facilitates one RA per workspace? If I open a specific project i.e. avoid opening up my workspace, then I think the changes in the associated PR are working.

This is the code that leads me to think that there's a restriction on the RA in that it is just one per workspace: https://github.com/rust-lang/rust-analyzer/pull/17667/files#diff-4023aad9450babbfcc30b89d5bf1fe604d569d67ff8c3232b783e8666c41eb0dR55

Perhaps an improvement on that would be to iterate through the workspaces and use the oldest RA found. WDYT? The way it stands now is that opening up my workspace is no longer viable.

@Veykril
Copy link
Member Author

Veykril commented Jul 25, 2024

Additional info: My project has a VSC workspace file and each project within is relatively independent of the other. Some of the projects have a toolchain file, some do not. There's a blend of embedded and non-embedded Rust projects here.

This is the reason. The toolchain pickup only works when there is a single folder opened within a VSCode workspace. The reason for that is that extensions are loaded per VSCode workspace and the toolchain override happens per cargo workspace/folder within a VSCode workspace.

Perhaps an improvement on that would be to iterate through the workspaces and use the oldest RA found. WDYT? The way it stands now is that opening up my workspace is no longer viable.

Ye I suppose this would work?

@Veykril Veykril reopened this Jul 25, 2024
@Veykril
Copy link
Member Author

Veykril commented Jul 25, 2024

Would also be good to attach a "reason" to why we chose the current running server version when telling the user about the running server then.

@huntc
Copy link
Contributor

huntc commented Jul 25, 2024

Ye I suppose this would work?

Well, it’s better than the current situation. ;-)

Do you want me to have a go?

Would also be good to attach a "reason" to why we chose the current running server version when telling the user about the running server then.

Sure. Where would that be displayed? In the RA output?

@Veykril
Copy link
Member Author

Veykril commented Jul 25, 2024

We have a status bar item that shows some info on hover, we should probably show that there
image

private updateStatusBarItem() {
let icon = "";
const status = this.lastStatus;
const statusBar = this.statusBar;
statusBar.show();
statusBar.tooltip = new vscode.MarkdownString("", true);
statusBar.tooltip.isTrusted = true;
switch (status.health) {
case "ok":
statusBar.color = undefined;
statusBar.backgroundColor = undefined;
if (this.config.statusBarClickAction === "stopServer") {
statusBar.command = "rust-analyzer.stopServer";
} else {
statusBar.command = "rust-analyzer.openLogs";
}
this.dependencies?.refresh();
break;
case "warning":
statusBar.color = new vscode.ThemeColor("statusBarItem.warningForeground");
statusBar.backgroundColor = new vscode.ThemeColor(
"statusBarItem.warningBackground",
);
statusBar.command = "rust-analyzer.openLogs";
icon = "$(warning) ";
break;
case "error":
statusBar.color = new vscode.ThemeColor("statusBarItem.errorForeground");
statusBar.backgroundColor = new vscode.ThemeColor("statusBarItem.errorBackground");
statusBar.command = "rust-analyzer.openLogs";
icon = "$(error) ";
break;
case "stopped":
statusBar.tooltip.appendText("Server is stopped");
statusBar.tooltip.appendMarkdown(
"\n\n[Start server](command:rust-analyzer.startServer)",
);
statusBar.color = new vscode.ThemeColor("statusBarItem.warningForeground");
statusBar.backgroundColor = new vscode.ThemeColor(
"statusBarItem.warningBackground",
);
statusBar.command = "rust-analyzer.startServer";
statusBar.text = "$(stop-circle) rust-analyzer";
return;
}
if (status.message) {
statusBar.tooltip.appendText(status.message);
}
if (statusBar.tooltip.value) {
statusBar.tooltip.appendMarkdown("\n\n---\n\n");
}
const toggleCheckOnSave = this.config.checkOnSave ? "Disable" : "Enable";
statusBar.tooltip.appendMarkdown(
`[Extension Info](command:rust-analyzer.serverVersion "Show version and server binary info"): Version ${this.version}, Server Version ${this._serverVersion}` +
"\n\n---\n\n" +
'[$(terminal) Open Logs](command:rust-analyzer.openLogs "Open the server logs")' +
"\n\n" +
`[$(settings) ${toggleCheckOnSave} Check on Save](command:rust-analyzer.toggleCheckOnSave "Temporarily ${toggleCheckOnSave.toLowerCase()} check on save functionality")` +
"\n\n" +
'[$(refresh) Reload Workspace](command:rust-analyzer.reloadWorkspace "Reload and rediscover workspaces")' +
"\n\n" +
'[$(symbol-property) Rebuild Build Dependencies](command:rust-analyzer.rebuildProcMacros "Rebuild build scripts and proc-macros")' +
"\n\n" +
'[$(stop-circle) Stop server](command:rust-analyzer.stopServer "Stop the server")' +
"\n\n" +
'[$(debug-restart) Restart server](command:rust-analyzer.restartServer "Restart the server")',
);
if (!status.quiescent) icon = "$(loading~spin) ";
statusBar.text = `${icon}rust-analyzer`;
}
pushExtCleanup(d: Disposable) {
this.extCtx.subscriptions.push(d);
}
pushClientCleanup(d: Disposable) {
this.clientSubscriptions.push(d);
}
}

(we also have a command but the notification that pops up from that isn't really nice for that).

@huntc
Copy link
Contributor

huntc commented Jul 26, 2024

@Veykril I've taken a stab at selecting a more suitable RA across a workspace. Please lemme know if this is on the right track and then I'll write some tests and take it further. Thanks.

#17705

RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 1, 2024
…kril

Use rustup rust-analyzer component when there is a toolchain file override for the opened workspace

Fixes rust-lang/rust-analyzer#17663
@huntc
Copy link
Contributor

huntc commented Aug 1, 2024

Is this really closed given my PR, or is my PR now obsolete?

@Veykril
Copy link
Member Author

Veykril commented Aug 1, 2024 via email

bors added a commit that referenced this issue Aug 2, 2024
feat: Use oldest rustup rust-analyzer when toolchain override is present

Selects a rust-toolchain declared RA based on its date. The earliest (oldest) RA wins and becomes the one that the workspace uses as a whole.

In terms of precedence:

nightly > stable-with-version > stable

With stable-with-version, we invoke the RA with a `--version` arg and attempt to extract a date. Given the same date as a nightly, the nightly RA will win.

Fixes #17663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-vscode vscode plugin issues C-feature Category: feature request
Projects
None yet
2 participants