Try to find a valid dotnet version from the path before falling back to runtime extension #6074
Conversation
…to runtime extension
| throw new Error(`Unknown result output from 'dotnet --version'. Received ${result.stdout}`); | ||
| } | ||
|
|
||
| if (semver.lt(dotnetVersion, this.minimumDotnetVersion)) { |
There was a problem hiding this comment.
everything before here is the exact code O# uses which has been working
dibarbet
force-pushed
the
dotnet_from_path
branch
from
a4b84e0
to
8c51451
Compare
| @@ -25,6 +25,7 @@ export async function getDotnetInfo(dotNetCliPaths: string[]): Promise<DotnetInf | |||
|
|
|||
There was a problem hiding this comment.
This file uses --info. --info is not meant for automation and does not have a guaranteed format. The recommendation is to use --list-runtimes and --list-sdks. If there is data you need that is not avaialble in those existing commands, please let us know and we can look into additional stable, machine readable options that are not --info.
CC @elinor-fung @vitek-karas since the work would come out of the host to add new information/commands potentially.
There was a problem hiding this comment.
I can probably replace the version lookup using the --list-sdks command with some work. However we also read the runtimeid (for the debugger) and the architecture (for this PR) which don't appear to be in either of those commands.
Generally I would like to avoid needing to parse dotnet info as well - and once the new version of the sdk acquisition extension is available I will likely be able to remove the part of the code I'm adding here. I'm not so sure about the other use cases like the debugger (cc @gregg-miskelly).
There was a problem hiding this comment.
For the debugger, we are trying to check the version and architecture of the globally installed .NET, as we need the one the application is going to run on. If we want to switch the target app to run on the version of .NET that the acquisition extension downloads, we could probably get rid of the check. But dotnet --list-runtimes and dotnet --list-sdks don't have architecture information, so those commands aren't useful enough.
There was a problem hiding this comment.
Thinking about it more: as long as we provide a way to specify a .NET SDK in the extension, we probably are going to need a way to check the architecture.
| const command = this.platformInfo.isWindows() ? 'where' : 'which'; | ||
| const whereOutput = await promisify(exec)(`${command} dotnet`); | ||
| if (!whereOutput.stdout) { | ||
| throw new Error(`Unable to find dotnet from ${command}.`); | ||
| } | ||
|
|
||
| const path = whereOutput.stdout.trim(); | ||
| if (!existsSync(path)) { | ||
| throw new Error(`dotnet path does not exist: ${path}`); | ||
| } |
There was a problem hiding this comment.
I reused this code for C# Dev Kit, but it failed in AzP because where returns one line for each dotnet found on the PATH, and this code assumes exactly one line will be produced. I'll fix my copy. But I suspect your code here will fail on any customer machine that has dotnet.exe installed in two places.
There was a problem hiding this comment.
Ah - thanks for the info. I'll take a look.
We've gotten lots of reports of issues with the .net runtime downloader extension, e.g. #6004
While we wait for those to be fixed, we can workaround the issue by seeing if there is a valid dotnet version on the path that we can use to start the server instead. This will not fix issues in devkit.
TODO - verify on linux/mac.