Use the .NET runtime extension to find an appropriate .NET install #7684
Conversation
| env: this.getEnvironmentVariables(dotnetPath), | ||
| }; | ||
| let dotnetExecutablePath: string; | ||
| if (commonOptions.dotnetPath) { |
There was a problem hiding this comment.
this should be deprecated in favor of the options on the .net install tool side. However I plan to tackle that in a separate PR as we'll need to separate out the O# usage of this option.
There was a problem hiding this comment.
I thought about whether we should keep the existing logic as a backup, but if this has about a mo in pre-release I'm not so concerned.
| }, | ||
| versionSpecRequirement: 'greater_than_or_equal', | ||
| }; | ||
| const result = await vscode.commands.executeCommand<string>('dotnet.findPath', findPathRequest); |
There was a problem hiding this comment.
fyi @nagilson - https://github.com/dotnet/vscode-dotnet-runtime/blob/a2e6a5c385190fe8e48e51dca17897b3a209d4f1/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts#L235 appears to indicate it returns a IDotnetAcquireResult, but it appears to only ever return a string
There was a problem hiding this comment.
It can return an IDotnetAcquireResult if the existingDotnetPath is set. It should always return an IDotnetAcquireResult. This was a mistake on my part, thank you for bringing this to my attention. I will make a fix.
src/main.ts
Outdated
| @@ -83,6 +84,35 @@ export async function activate( | |||
| requiredPackageIds.push('OmniSharp'); | |||
| } | |||
|
|
|||
| const dotnetRuntimeExtensionId = 'ms-dotnettools.vscode-dotnet-runtime'; | |||
| const requiredDotnetRuntimeExtensionVersion = '2.2.1'; | |||
There was a problem hiding this comment.
Too bad this can't be expressed in the package.json
There was a problem hiding this comment.
Yeah, unfortunately no versions in extension dependencies :(
|
converting to draft - there is one slight modification to the API coming in version |
dibarbet
force-pushed
the
find_dotnet_runtime
branch
from
12f338e
to
a9bf31f
Compare
dibarbet
force-pushed
the
find_dotnet_runtime
branch
from
7e9340b
to
8081622
Compare
|
2.2.3 is now available which also includes support for requiring at least a patch version. this is ready for review |
src/main.ts
Outdated
| if (lt(dotnetRuntimeExtensionVersion, requiredDotnetRuntimeExtensionVersion)) { | ||
| const button = vscode.l10n.t('Update and reload'); | ||
| const prompt = vscode.l10n.t( | ||
| 'The {0} extension requires version {1} or greater of the .NET Install Tool ({2}) extension. Please update to continue', |
There was a problem hiding this comment.
Some people who install C# have no idea what .NET is, so we might expect some amount of confusion from users who don't understand why they would need a '.NET Install Tool' -- it may be helpful to include more context, but people don't tend to read long error messages, so this is likely better. Might be worth PM consultant.
There was a problem hiding this comment.
Yeah - I think this is already fairly long and am hesitant to increase it. cc @webreidi for thoughts.
| @@ -39,38 +36,47 @@ export class DotnetRuntimeExtensionResolver implements IHostExecutableResolver { | |||
| private hostInfo: HostExecutableInformation | undefined; | |||
|
|
|||
| async getHostExecutableInfo(): Promise<HostExecutableInformation> { | |||
| let dotnetRuntimePath = commonOptions.dotnetPath; | |||
There was a problem hiding this comment.
A breaking change document for dotnetPath should be created, IMO - though for later of course. I would also make sure this is remarked in the release notes, and that our existingPath setting is mentioned.
https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.vscode-dotnet-runtime&ssr=false#overview:~:text=I%20already%20have%20a%20.NET%20Runtime%20or%20SDK%20installed%2C%20and%20I%20want%20to%20use%20it
There was a problem hiding this comment.
I haven't removed this in this PR - plan to in a followup. What might be an even better option is to migrate existing users of dotnet.dotnetPath to the new setting. We already do that for migrating O# options
|
merging - validation looked fine (encountered only known issues that behave the same way in the current version) |
C# extension side of microsoft/vscode-dotnettools#1243
Switches from our custom dotnet resolution to call the .net install tool API. Also ensures that a high enough version of the install tool exists.