Skip to content

Commit

Permalink
Delete dotnet.dotnetPath option and migrate to replacement (#7825)
Browse files Browse the repository at this point in the history
  • Loading branch information
dibarbet authored Nov 26, 2024
2 parents b70cadb + 98e9ba1 commit ab7e380
Show file tree
Hide file tree
Showing 14 changed files with 386 additions and 80 deletions.
8 changes: 6 additions & 2 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
// Set the context to the workspace folder to allow us to copy files from it.
"context": ".."
},
"mounts": [
"type=volume,target=${containerWorkspaceFolder}/node_modules"
],
"customizations": {
"vscode": {
"settings": {
Expand Down Expand Up @@ -37,7 +40,8 @@
"powershell.integratedConsole.showOnStartup": false,
"powershell.startAutomatically": false,
// ms-azure-devops.azure-pipelines settings
"azure-pipelines.customSchemaFile": ".vscode/dnceng-schema.json"
"azure-pipelines.customSchemaFile": ".vscode/dnceng-schema.json",
"jest.jestCommandLine": "./node_modules/.bin/jest"
},
"extensions": [
"ms-dotnettools.csharp",
Expand All @@ -52,5 +56,5 @@
]
}
},
"postCreateCommand": "npm ci && npx gulp installDependencies"
"postCreateCommand": "npm ci && npx gulp installDependencies && npm i -g gulp"
}
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
"editor.formatOnSave": false,
"eslint.lintTask.enable": true,
"dotnet.defaultSolution": "disable",
"jest.autoRun": "off"
"jest.autoRun": "off",
}
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Bump xamltools to 17.13.35521.31 (PR: [#7814](https://github.com/dotnet/vscode-csharp/pull/7814))

# 2.59.x
* Delete `dotnet.dotnetPath` setting and support automatic migration to replacements (PR: [#7825](https://github.com/dotnet/vscode-csharp/pull/7825))
* Existing `dotnet.dotnetPath` values will be migrated to the .NET Install Tool extension's `dotnetAcquisitionExtension.existingDotnetPath` setting. See [this page](https://github.com/dotnet/vscode-dotnet-runtime/blob/main/vscode-dotnet-runtime-extension/README.md#i-already-have-a-net-runtime-or-sdk-installed-and-i-want-to-use-it) for more details on configuring the .NET Install Tool.
* The OmniSharp version of `dotnet.dotnetPath` has been migrated to `omnisharp.dotnetPath`

# 2.58.x
* Update Razor to 9.0.0-preview.24569.4 (PR: [#7805](https://github.com/dotnet/vscode-csharp/pull/7805))
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1405,11 +1405,6 @@
"default": false,
"description": "%configuration.dotnet.preferCSharpExtension%"
},
"dotnet.dotnetPath": {
"type": "string",
"scope": "machine-overridable",
"description": "%configuration.dotnet.dotnetPath%"
},
"dotnet.server.path": {
"type": "string",
"scope": "machine-overridable",
Expand Down Expand Up @@ -1536,6 +1531,11 @@
"description": "%configuration.omnisharp.dotnet.server.useOmnisharp%",
"order": 0
},
"omnisharp.dotnetPath": {
"type": "string",
"scope": "machine-overridable",
"description": "%configuration.omnisharp.dotnetPath%"
},
"csharp.format.enable": {
"type": "boolean",
"default": true,
Expand Down
2 changes: 1 addition & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
"command.dotnet.test.debugTestsInContext": "Debug Tests in Context",
"command.dotnet.restartServer": "Restart Language Server",
"configuration.dotnet.defaultSolution.description": "The path of the default solution to be opened in the workspace, or set to 'disable' to skip it. (Previously `omnisharp.defaultLaunchSolution`)",
"configuration.dotnet.dotnetPath": "Specifies the path to a dotnet installation directory to use instead of the default system one. This only influences the dotnet installation to use for hosting the language server itself. Example: \"/home/username/mycustomdotnetdirectory\".",
"configuration.dotnet.server.path": "Specifies the absolute path to the server (LSP or O#) executable. When left empty the version pinned to the C# Extension is used. (Previously `omnisharp.path`)",
"configuration.dotnet.server.componentPaths": "Allows overriding the folder path for built in components of the language server (for example, override the .roslynDevKit path in the extension directory to use locally built components)",
"configuration.dotnet.server.componentPaths.roslynDevKit": "Overrides the folder path for the .roslynDevKit component of the language server",
Expand Down Expand Up @@ -86,6 +85,7 @@
]
},
"configuration.omnisharp.dotnet.server.useOmnisharp": "Switches to use the Omnisharp server for language features when enabled (requires restart). This option will not be honored with C# Dev Kit installed.",
"configuration.omnisharp.dotnetPath": "Specifies the path to a dotnet installation directory to use instead of the default system one. This only influences the dotnet installation to use for hosting the OmniSharp server itself. Example: \"/home/username/mycustomdotnetdirectory\".",
"configuration.omnisharp.csharp.format.enable": "Enable/disable default C# formatter (requires restart).",
"configuration.omnisharp.csharp.suppressDotnetInstallWarning": "Suppress the warning that the .NET Core SDK is not on the path.",
"configuration.omnisharp.csharp.suppressDotnetRestoreNotification": "Suppress the notification window to perform a 'dotnet restore' when dependencies can't be resolved.",
Expand Down
60 changes: 25 additions & 35 deletions src/lsptoolshost/dotnetRuntimeExtensionResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as vscode from 'vscode';
import { HostExecutableInformation } from '../shared/constants/hostExecutableInformation';
import { IHostExecutableResolver } from '../shared/constants/IHostExecutableResolver';
import { PlatformInformation } from '../shared/platform';
import { commonOptions, languageServerOptions } from '../shared/options';
import { languageServerOptions } from '../shared/options';
import { existsSync } from 'fs';
import { CSharpExtensionId } from '../constants/csharpExtensionId';
import { readFile } from 'fs/promises';
Expand Down Expand Up @@ -36,40 +36,34 @@ export class DotnetRuntimeExtensionResolver implements IHostExecutableResolver {
private hostInfo: HostExecutableInformation | undefined;

async getHostExecutableInfo(): Promise<HostExecutableInformation> {
let dotnetExecutablePath: string;
if (commonOptions.dotnetPath) {
const dotnetExecutableName = this.getDotnetExecutableName();
dotnetExecutablePath = path.join(commonOptions.dotnetPath, dotnetExecutableName);
} else {
if (this.hostInfo) {
return this.hostInfo;
}
if (this.hostInfo) {
return this.hostInfo;
}

this.channel.appendLine(`Locating .NET runtime version ${DotNetRuntimeVersion}`);
const extensionArchitecture = (await this.getArchitectureFromTargetPlatform()) ?? process.arch;
const findPathRequest: IDotnetFindPathContext = {
acquireContext: {
version: DotNetRuntimeVersion,
requestingExtensionId: CSharpExtensionId,
architecture: extensionArchitecture,
mode: 'runtime',
},
versionSpecRequirement: 'greater_than_or_equal',
};
let acquireResult = await vscode.commands.executeCommand<IDotnetAcquireResult | undefined>(
'dotnet.findPath',
findPathRequest
this.channel.appendLine(`Locating .NET runtime version ${DotNetRuntimeVersion}`);
const extensionArchitecture = (await this.getArchitectureFromTargetPlatform()) ?? process.arch;
const findPathRequest: IDotnetFindPathContext = {
acquireContext: {
version: DotNetRuntimeVersion,
requestingExtensionId: CSharpExtensionId,
architecture: extensionArchitecture,
mode: 'runtime',
},
versionSpecRequirement: 'greater_than_or_equal',
};
let acquireResult = await vscode.commands.executeCommand<IDotnetAcquireResult | undefined>(
'dotnet.findPath',
findPathRequest
);
if (acquireResult === undefined) {
this.channel.appendLine(
`Did not find .NET ${DotNetRuntimeVersion} on path, falling back to acquire runtime via ms-dotnettools.vscode-dotnet-runtime`
);
if (acquireResult === undefined) {
this.channel.appendLine(
`Did not find .NET ${DotNetRuntimeVersion} on path, falling back to acquire runtime via ms-dotnettools.vscode-dotnet-runtime`
);
acquireResult = await this.acquireDotNetProcessDependencies();
}

dotnetExecutablePath = acquireResult.dotnetPath;
acquireResult = await this.acquireDotNetProcessDependencies();
}

const dotnetExecutablePath = acquireResult.dotnetPath;

const hostInfo = {
version: '' /* We don't need to know the version - we've already downloaded the correct one */,
path: dotnetExecutablePath,
Expand Down Expand Up @@ -186,8 +180,4 @@ export class DotnetRuntimeExtensionResolver implements IHostExecutableResolver {
throw new Error(`Unknown extension target platform: ${targetPlatform}`);
}
}

private getDotnetExecutableName(): string {
return this.platformInfo.isWindows() ? 'dotnet.exe' : 'dotnet';
}
}
3 changes: 2 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ import { TelemetryEventNames } from './shared/telemetryEventNames';
export async function activate(
context: vscode.ExtensionContext
): Promise<CSharpExtensionExports | OmnisharpExtensionExports | null> {
const csharpChannel = vscode.window.createOutputChannel('C#', { log: true });

await MigrateOptions(vscode);
const optionStream = createOptionStream(vscode);

Expand All @@ -67,7 +69,6 @@ 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#', { log: true });
const csharpchannelObserver = new CsharpChannelObserver(csharpChannel);
const csharpLogObserver = new CsharpLoggerObserver(csharpChannel);
eventStream.subscribe(csharpchannelObserver.post);
Expand Down
4 changes: 2 additions & 2 deletions src/omnisharp/dotnetResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { promisify } from 'util';
import { HostExecutableInformation } from '../shared/constants/hostExecutableInformation';
import { IHostExecutableResolver } from '../shared/constants/IHostExecutableResolver';
import { PlatformInformation } from '../shared/platform';
import { commonOptions } from '../shared/options';
import { omnisharpOptions } from '../shared/options';

export class DotnetResolver implements IHostExecutableResolver {
private readonly minimumDotnetVersion = '6.0.100';
Expand All @@ -21,7 +21,7 @@ export class DotnetResolver implements IHostExecutableResolver {
const dotnet = this.platformInfo.isWindows() ? 'dotnet.exe' : 'dotnet';
const env = { ...process.env };

const dotnetPathOption = commonOptions.dotnetPath;
const dotnetPathOption = omnisharpOptions.dotnetPath;
if (dotnetPathOption.length > 0) {
env['PATH'] = dotnetPathOption + path.delimiter + env['PATH'];
}
Expand Down
9 changes: 4 additions & 5 deletions src/shared/configurationProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
} from '../shared/processPicker';
import { PlatformInformation } from './platform';
import { getCSharpDevKit } from '../utils/getCSharpDevKit';
import { commonOptions } from './options';
import {
ActionOption,
showErrorMessageWithOptions,
Expand Down Expand Up @@ -150,7 +149,7 @@ export class BaseVsDbgConfigurationProvider implements vscode.DebugConfiguration
}

if (debugConfiguration.checkForDevCert) {
if (!(await this.checkForDevCerts(commonOptions.dotnetPath))) {
if (!(await this.checkForDevCerts())) {
return undefined;
}
}
Expand Down Expand Up @@ -235,11 +234,11 @@ export class BaseVsDbgConfigurationProvider implements vscode.DebugConfiguration
}
}

private async checkForDevCerts(dotnetPath: string): Promise<boolean> {
private async checkForDevCerts(): Promise<boolean> {
let result: boolean | undefined = undefined;

while (result === undefined) {
const returnData = await hasDotnetDevCertsHttps(dotnetPath);
const returnData = await hasDotnetDevCertsHttps();
const errorCode = returnData.error?.code;
if (
errorCode === CertToolStatusCodes.CertificateNotTrusted ||
Expand All @@ -261,7 +260,7 @@ export class BaseVsDbgConfigurationProvider implements vscode.DebugConfiguration
);

if (dialogResult === labelYes) {
const returnData = await createSelfSignedCert(dotnetPath);
const returnData = await createSelfSignedCert();
if (returnData.error === null) {
// if the process returns 0, returnData.error is null, otherwise the return code can be accessed in returnData.error.code
const message = errorCode === CertToolStatusCodes.CertificateNotTrusted ? 'trusted' : 'created';
Expand Down
102 changes: 102 additions & 0 deletions src/shared/migrateOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,20 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as fs from 'fs';
import * as path from 'path';
import { types } from 'util';
import { ConfigurationTarget, vscode, WorkspaceConfiguration } from '../vscodeAdapter';
import { CSharpExtensionId } from '../constants/csharpExtensionId';
import { commonOptions } from './options';

export interface IDotnetAcquisitionExistingPaths {
extensionId: string;
path: string;
}

export const dotnetPathOption = 'dotnet.dotnetPath';
export const dotnetAcquisitionExtensionOption = 'dotnetAcquisitionExtension.existingDotnetPath';

// Option in the array should be identical to each other, except the name.
export const migrateOptions = [
Expand Down Expand Up @@ -111,6 +123,96 @@ export async function MigrateOptions(vscode: vscode): Promise<void> {
await MoveOptionsValue(oldName, newName, configuration);
}
}

await migrateDotnetPathOption(vscode);
}

async function migrateDotnetPathOption(vscode: vscode): Promise<void> {
const configuration = vscode.workspace.getConfiguration();

if (commonOptions.useOmnisharpServer) {
// Migrate to O# specific option.
await MoveOptionsValue(dotnetPathOption, 'omnisharp.dotnetPath', configuration);
} else {
const oldOptionInspect = configuration.inspect<string>(dotnetPathOption);
if (
!oldOptionInspect ||
(!oldOptionInspect.globalValue &&
!oldOptionInspect.workspaceValue &&
!oldOptionInspect.workspaceFolderValue)
) {
// No value is set, nothing to migrate.
return;
}

const newOptionInspect = configuration.inspect<IDotnetAcquisitionExistingPaths[]>(
dotnetAcquisitionExtensionOption
);

if (oldOptionInspect.globalValue) {
await migrateSingleDotnetPathValue(
dotnetPathOption,
oldOptionInspect.globalValue,
dotnetAcquisitionExtensionOption,
newOptionInspect?.globalValue,
configuration,
ConfigurationTarget.Global
);
}

if (oldOptionInspect.workspaceValue) {
await migrateSingleDotnetPathValue(
dotnetPathOption,
oldOptionInspect.workspaceValue,
dotnetAcquisitionExtensionOption,
newOptionInspect?.workspaceValue,
configuration,
ConfigurationTarget.Workspace
);
}

if (oldOptionInspect.workspaceFolderValue) {
await migrateSingleDotnetPathValue(
dotnetPathOption,
oldOptionInspect.workspaceFolderValue,
dotnetAcquisitionExtensionOption,
newOptionInspect?.workspaceFolderValue,
configuration,
ConfigurationTarget.WorkspaceFolder
);
}
}
}

async function migrateSingleDotnetPathValue(
oldOptionName: string,
oldValue: string,
newOptionName: string,
currentNewValue: IDotnetAcquisitionExistingPaths[] | undefined,
configuration: WorkspaceConfiguration,
configurationTarget: ConfigurationTarget
): Promise<void> {
// Migrate to .NET install tool specific option.
// This requires some adjustments as the .NET install tool expects the full path to the exe and can already have a value set (e.g. a diff extension).
const extension = process.platform === 'win32' ? '.exe' : '';
const newValue = path.join(oldValue, `dotnet${extension}`);

if (!fs.existsSync(newValue)) {
// If the existing option points to a location that doesn't exist, we'll just remove the old value.
configuration.update(oldOptionName, undefined, configurationTarget);
return;
}

currentNewValue = currentNewValue ?? [];
if (currentNewValue && currentNewValue.filter((p) => p.extensionId === CSharpExtensionId).length !== 0) {
// There's already a dotnet path set for this extension, we don't want to overwrite it. Just delete the old one.
await configuration.update(oldOptionName, undefined, configurationTarget);
return;
}

currentNewValue.push({ extensionId: CSharpExtensionId, path: newValue });
await configuration.update(newOptionName, currentNewValue, configurationTarget);
await configuration.update(oldOptionName, undefined, configurationTarget);
}

function shouldMove(newOptionValue: unknown, defaultInspectValueOfNewOption: unknown): boolean {
Expand Down
Loading

0 comments on commit ab7e380

Please sign in to comment.