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

Try to find a valid dotnet version from the path before falling back to runtime extension #6074

Merged
merged 4 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
123 changes: 122 additions & 1 deletion src/lsptoolshost/dotnetRuntimeExtensionResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@

import * as path from 'path';
import * as vscode from 'vscode';
import * as semver from 'semver';
import { HostExecutableInformation } from '../shared/constants/hostExecutableInformation';
import { IHostExecutableResolver } from '../shared/constants/IHostExecutableResolver';
import { PlatformInformation } from '../shared/platform';
import { Options } from '../shared/options';
import { existsSync } from 'fs';
import { CSharpExtensionId } from '../constants/csharpExtensionId';
import { promisify } from 'util';
import { exec } from 'child_process';
import { getDotnetInfo } from '../shared/utils/getDotnetInfo';
import { readFile } from 'fs/promises';

export const DotNetRuntimeVersion = '7.0';

Expand All @@ -22,19 +27,36 @@ interface IDotnetAcquireResult {
* Resolves the dotnet runtime for a server executable from given options and the dotnet runtime VSCode extension.
*/
export class DotnetRuntimeExtensionResolver implements IHostExecutableResolver {
private readonly minimumDotnetVersion = '7.0.100';
constructor(
private platformInfo: PlatformInformation,
/**
* This is a function instead of a string because the server path can change while the extension is active (when the option changes).
*/
private getServerPath: (options: Options, platform: PlatformInformation) => string
private getServerPath: (options: Options, platform: PlatformInformation) => string,
private channel: vscode.OutputChannel,
private extensionPath: string
) {}

private hostInfo: HostExecutableInformation | undefined;

async getHostExecutableInfo(options: Options): Promise<HostExecutableInformation> {
let dotnetRuntimePath = options.commonOptions.dotnetPath;
const serverPath = this.getServerPath(options, this.platformInfo);

// Check if we can find a valid dotnet from dotnet --version on the PATH.
if (!dotnetRuntimePath) {
const dotnetPath = await this.findDotnetFromPath();
if (dotnetPath) {
return {
version: '' /* We don't need to know the version - we've already verified its high enough */,
path: dotnetPath,
env: process.env,
};
}
}

// We didn't find it on the path, see if we can install the correct runtime using the runtime extension.
if (!dotnetRuntimePath) {
const dotnetInfo = await this.acquireDotNetProcessDependencies(serverPath);
dotnetRuntimePath = path.dirname(dotnetInfo.path);
Expand Down Expand Up @@ -101,4 +123,103 @@ export class DotnetRuntimeExtensionResolver implements IHostExecutableResolver {

return dotnetInfo;
}

/**
* Checks dotnet --version to see if the value on the path is greater than the minimum required version.
* This is adapated from similar O# server logic and should be removed when we have a stable acquisition extension.
* @returns true if the dotnet version is greater than the minimum required version, false otherwise.
*/
private async findDotnetFromPath(): Promise<string | undefined> {
try {
const dotnetInfo = await getDotnetInfo([]);
const dotnetVersionStr = dotnetInfo.Version;

const extensionArchitecture = await this.getArchitectureFromTargetPlatform();
const dotnetArchitecture = dotnetInfo.Architecture;

// If the extension arhcitecture is defined, we check that it matches the dotnet architecture.
// If its undefined we likely have a platform neutral server and assume it can run on any architecture.
if (extensionArchitecture && extensionArchitecture !== dotnetArchitecture) {
throw new Error(
`The architecture of the .NET runtime (${dotnetArchitecture}) does not match the architecture of the extension (${extensionArchitecture}).`
);
}

const dotnetVersion = semver.parse(dotnetVersionStr);
if (!dotnetVersion) {
throw new Error(`Unknown result output from 'dotnet --version'. Received ${dotnetVersionStr}`);
}

if (semver.lt(dotnetVersion, this.minimumDotnetVersion)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

everything before here is the exact code O# uses which has been working

throw new Error(
gregg-miskelly marked this conversation as resolved.
Show resolved Hide resolved
`Found dotnet version ${dotnetVersion}. Minimum required version is ${this.minimumDotnetVersion}.`
);
}

// Find the location of the dotnet on path.
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 where.`);
}

const path = whereOutput.stdout.trim();
if (!existsSync(path)) {
throw new Error(`dotnet path does not exist: ${path}`);
}

this.channel.appendLine(`Using dotnet configured on PATH`);
return path;
} catch (e) {
this.channel.appendLine(
'Failed to find dotnet info from path, falling back to acquire runtime via ms-dotnettools.vscode-dotnet-runtime'
);
if (e instanceof Error) {
this.channel.appendLine(e.message);
}
}

return undefined;
}

private async getArchitectureFromTargetPlatform(): Promise<string | undefined> {
const vsixManifestFile = path.join(this.extensionPath, '.vsixmanifest');
if (!existsSync(vsixManifestFile)) {
// This is not an error as normal development F5 builds do not generate a .vsixmanifest file.
this.channel.appendLine(
`Unable to find extension target platform - no vsix manifest file exists at ${vsixManifestFile}`
);
return undefined;
}

const contents = await readFile(vsixManifestFile, 'utf-8');
const targetPlatformMatch = /TargetPlatform="(.*)"/.exec(contents);
if (!targetPlatformMatch) {
throw new Error(`Could not find extension target platform in ${vsixManifestFile}`);
}

const targetPlatform = targetPlatformMatch[1];

// The currently known extension platforms are taken from here:
// https://code.visualstudio.com/api/working-with-extensions/publishing-extension#platformspecific-extensions
switch (targetPlatform) {
case 'win32-x64':
case 'linux-x64':
case 'alpine-x64':
case 'darwin-x64':
return 'x64';
case 'win32-ia32':
return 'x86';
case 'win32-arm64':
case 'linux-arm64':
case 'alpine-arm64':
case 'darwin-arm64':
return 'arm64';
case 'linux-armhf':
case 'web':
return undefined;
default:
throw new Error(`Unknown extension target platform: ${targetPlatform}`);
}
}
}
7 changes: 6 additions & 1 deletion src/lsptoolshost/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,12 @@ export async function activateRoslynLanguageServer(
// Create a separate channel for outputting trace logs - these are incredibly verbose and make other logs very difficult to see.
_traceChannel = vscode.window.createOutputChannel('C# LSP Trace Logs');

const hostExecutableResolver = new DotnetRuntimeExtensionResolver(platformInfo, getServerPath);
const hostExecutableResolver = new DotnetRuntimeExtensionResolver(
platformInfo,
getServerPath,
outputChannel,
context.extensionPath
);
const additionalExtensionPaths = scanExtensionPlugins();
_languageServer = new RoslynLanguageServer(
platformInfo,
Expand Down
5 changes: 5 additions & 0 deletions src/shared/utils/getDotnetInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export async function getDotnetInfo(dotNetCliPaths: string[]): Promise<DotnetInf

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@dibarbet dibarbet Aug 9, 2023

Choose a reason for hiding this comment

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

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

let version: string | undefined;
let runtimeId: string | undefined;
let architecture: string | undefined;

const lines = data.replace(/\r/gm, '').split('\n');
for (const line of lines) {
Expand All @@ -33,6 +34,8 @@ export async function getDotnetInfo(dotNetCliPaths: string[]): Promise<DotnetInf
version = match[1];
} else if ((match = /^ RID:\s*([\w\-.]+)$/.exec(line))) {
runtimeId = match[1];
} else if ((match = /^\s*Architecture:\s*(.*)/.exec(line))) {
architecture = match[1];
}
}

Expand All @@ -42,6 +45,7 @@ export async function getDotnetInfo(dotNetCliPaths: string[]): Promise<DotnetInf
FullInfo: fullInfo,
Version: version,
RuntimeId: runtimeId,
Architecture: architecture,
};
return _dotnetInfo;
}
Expand Down Expand Up @@ -73,4 +77,5 @@ export interface DotnetInfo {
Version: string;
/* a runtime-only install of dotnet will not output a runtimeId in dotnet --info. */
RuntimeId?: string;
Architecture?: string;
}