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.

Sign up for GitHub

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

Jump to bottom

Enable automatic nuget restore on the client #6676

Merged
merged 6 commits into from
Dec 5, 2023
Merged

Enable automatic nuget restore on the client #6676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b8a3c26
Enable automatic nuget restore on the client
dibarbet Nov 16, 2023
4f20608
Review feedback
dibarbet Nov 28, 2023
19828db
More localization
dibarbet Dec 4, 2023
2325fae
Rename
dibarbet Dec 4, 2023
bd05428
Update Roslyn version
dibarbet Dec 5, 2023
407106e
Merge branch 'main' into automatic_nuget_restore
dibarbet Dec 5, 2023
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
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
- Debug from .csproj and .sln [#5876](https://github.com/dotnet/vscode-csharp/issues/5876)

## Latest
* Update Roslyn to 4.9.0-3.23604.10 (PR: [#6676](https://github.com/dotnet/vscode-csharp/pull/6676))
* Pass through folders for additional files (PR: [#71061](https://github.com/dotnet/roslyn/pull/71061))
* Automatically detect missing NuGet packages and restore (PR: [#70851](https://github.com/dotnet/roslyn/pull/70851))
* Enable route embedded language features in vscode (PR: [#70927](https://github.com/dotnet/roslyn/pull/70927))
* Add automatic nuget restore support to C# standalone (PR: [#6676](https://github.com/dotnet/vscode-csharp/pull/6676))
* Update required VSCode version to 1.75.0 (PR: [#6711](https://github.com/dotnet/vscode-csharp/pull/6711))
* Update debugger docs to point to official documentation (PR: [#6674](https://github.com/dotnet/vscode-csharp/pull/6674))

## 2.12.19
* Update Roslyn to 4.9.0-2.23571.2 (PR: [#6681](https://github.com/dotnet/vscode-csharp/pull/6681))
* Workaround vscode bug with returning defaultBehavior from prepareRename (PR: [#70840](https://github.com/dotnet/roslyn/pull/70840))
* Implement textDocument/prepareRename to show error in invalid rename locations (PR: [#70724](https://github.com/dotnet/roslyn/pull/70724))
Expand Down
7 changes: 6 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}
},
"defaults": {
"roslyn": "4.9.0-2.23571.2",
"roslyn": "4.9.0-3.23604.10",
"omniSharp": "1.39.10",
"razor": "7.0.0-preview.23528.1",
"razorOmnisharp": "7.0.0-preview.23363.1",
Expand Down Expand Up @@ -1731,6 +1731,11 @@
"default": null,
"description": "Sets a path where MSBuild binary logs are written to when loading projects, to help diagnose loading errors."
},
"dotnet.projects.enableAutomaticRestore": {
"type": "boolean",
"default": true,
"description": "%configuration.dotnet.projects.enableAutomaticRestore%"
},
"razor.languageServer.directory": {
"type": "string",
"scope": "machine-overridable",
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"configuration.dotnet.server.trace": "Sets the logging level for the language server",
"configuration.dotnet.server.extensionPaths": "Override for path to language server --extension arguments",
"configuration.dotnet.server.crashDumpPath": "Sets a folder path where crash dumps are written to if the language server crashes. Must be writeable by the user.",
"configuration.dotnet.projects.enableAutomaticRestore": "Enables automatic NuGet restore if the extension detects assets are missing.",
"configuration.dotnet.preferCSharpExtension": "Forces projects to load with the C# extension only. This can be useful when using legacy project types that are not supported by C# Dev Kit. (Requires window reload)",
"configuration.dotnet.implementType.insertionBehavior": "The insertion location of properties, events, and methods When implement interface or abstract class.",
"configuration.dotnet.implementType.insertionBehavior.withOtherMembersOfTheSameKind": "Place them with other members of the same kind.",
Expand Down
28 changes: 21 additions & 7 deletions src/lsptoolshost/restore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@

import * as vscode from 'vscode';
import { RoslynLanguageServer } from './roslynLanguageServer';
import { RestorableProjects, RestoreParams, RestorePartialResult, RestoreRequest } from './roslynProtocol';
import {
RestorableProjects,
RestoreParams,
RestorePartialResult,
RestoreRequest,
ProjectHasUnresolvedDependenciesRequest,
} from './roslynProtocol';
import path = require('path');

let _restoreInProgress = false;
Expand All @@ -22,10 +28,15 @@ export function registerRestoreCommands(
);
context.subscriptions.push(
vscode.commands.registerCommand('dotnet.restore.all', async (): Promise<void> => {
return restore(languageServer, restoreChannel);
return restore(languageServer, restoreChannel, [], true);
})
);

languageServer.registerOnRequest(ProjectHasUnresolvedDependenciesRequest.type, async (params) => {
await restore(languageServer, restoreChannel, params.projectFilePaths, false);
});
}

async function chooseProjectAndRestore(
languageServer: RoslynLanguageServer,
restoreChannel: vscode.OutputChannel
Expand All @@ -49,22 +60,25 @@ async function chooseProjectAndRestore(
return;
}

await restore(languageServer, restoreChannel, pickedItem.description);
await restore(languageServer, restoreChannel, [pickedItem.description!], true);
}

async function restore(
export async function restore(
languageServer: RoslynLanguageServer,
restoreChannel: vscode.OutputChannel,
projectFile?: string
projectFiles: string[],
showOutput: boolean
): Promise<void> {
if (_restoreInProgress) {
vscode.window.showErrorMessage(vscode.l10n.t('Restore already in progress'));
Copy link
Member

Choose a reason for hiding this comment

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

Is there a possibility we can now hit this if automatic restore is on? I'm thinking something like the user invokes restore, it starts to restore projects, and tries (but fails) to fully restore one project, call it A. It's still going and marching to restore other projects in the solution, but we saw the restore of A, and then trigger a reload which triggers a check of A's dependencies. We see it's missing, and so we trigger a restore of A again, but the restore of the rest of the solution is still going on...

Should we be queueing these instead in some way? That might also make sense if somebody is invoking two project restores one after the other in rapid succession.

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be safe on the reload side because the restores on the client will block the project load queue until they're done. So the reload won't be processed until the rest of the projects finish restoring (I actually hit this issue with the first iteration with notifications)

This should only get hit if someone triggers a manual restore while an automatic one is in progress

Copy link
Member

Choose a reason for hiding this comment

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

Yeah manual and automatic restore happening at once was what worried me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it is possible to hit this with a manual restore. I think for now though we don't need a queue - it should be a relatively uncommon case. If its not and ends up being a bad experience for people we can come back and add it.

Copy link
Member

Choose a reason for hiding this comment

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

@dibarbet Maybe this is better to defer anyways until we get all the restores being done by the server rather thant he back-and-forth happening here anyways.

return;
}
_restoreInProgress = true;
restoreChannel.show(true);
if (showOutput) {
restoreChannel.show(true);
}

const request: RestoreParams = { projectFilePath: projectFile };
const request: RestoreParams = { projectFilePaths: projectFiles };
await vscode.window
.withProgress(
{
Expand Down
8 changes: 8 additions & 0 deletions src/lsptoolshost/roslynLanguageServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
MessageTransports,
RAL,
CancellationToken,
RequestHandler,
} from 'vscode-languageclient/node';
import { PlatformInformation } from '../shared/platform';
import { readConfigurations } from './configurationMiddleware';
Expand Down Expand Up @@ -319,6 +320,13 @@ export class RoslynLanguageServer {
return response;
}

public registerOnRequest<Params, Result, Error>(
type: RequestType<Params, Result, Error>,
handler: RequestHandler<Params, Result, Error>
) {
this._languageClient.addDisposable(this._languageClient.onRequest(type, handler));
}

public async registerSolutionSnapshot(token: vscode.CancellationToken): Promise<SolutionSnapshotId> {
const response = await this.sendRequest0(RoslynProtocol.RegisterSolutionSnapshotRequest.type, token);
if (response) {
Expand Down
19 changes: 16 additions & 3 deletions src/lsptoolshost/roslynProtocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,24 @@ export interface NamedPipeInformation {

export interface RestoreParams extends lsp.WorkDoneProgressParams, lsp.PartialResultParams {
/**
* An optional file path to restore.
* If none is specified, the solution (or all loaded projects) are restored.
* The set of projects to restore.
* If none are specified, the solution (or all loaded projects) are restored.
*/
projectFilePath?: string;
projectFilePaths: string[];
}

export interface RestorePartialResult {
stage: string;
message: string;
}

export interface UnresolvedProjectDependenciesParams {
/**
* The set of projects that have unresolved dependencies and require a restore.
*/
Comment on lines +167 to +169
Copy link
Member

Choose a reason for hiding this comment

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

Might still need a rename or update too since we're also handling other changes.

projectFilePaths: string[];
}

export namespace WorkspaceDebugConfigurationRequest {
export const method = 'workspace/debugConfiguration';
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.clientToServer;
Expand Down Expand Up @@ -260,3 +267,9 @@ export namespace RestorableProjects {
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.clientToServer;
export const type = new lsp.RequestType0<string[], void>(method);
}

export namespace ProjectHasUnresolvedDependenciesRequest {
export const method = 'workspace/_roslyn_projectHasUnresolvedDependencies';
export const messageDirection: lsp.MessageDirection = lsp.MessageDirection.serverToClient;
export const type = new lsp.RequestType<UnresolvedProjectDependenciesParams, void, void>(method);
}
2 changes: 1 addition & 1 deletion src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ export async function activate(
// ensure it gets properly disposed. Upon disposal the events will be flushed.
context.subscriptions.push(reporter);

const csharpChannel = vscode.window.createOutputChannel('C#');
const dotnetTestChannel = vscode.window.createOutputChannel('.NET Test Log');
const dotnetChannel = vscode.window.createOutputChannel('.NET NuGet Restore');
const csharpChannel = vscode.window.createOutputChannel('C#');
const csharpchannelObserver = new CsharpChannelObserver(csharpChannel);
const csharpLogObserver = new CsharpLoggerObserver(csharpChannel);
eventStream.subscribe(csharpchannelObserver.post);
Expand Down
Loading