Skip to content

Commit

Permalink
Make SDK build of OmniSharp the default
Browse files Browse the repository at this point in the history
  • Loading branch information
JoeRobich committed Apr 19, 2022
1 parent c7239d9 commit cd81b5c
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 280 deletions.
25 changes: 5 additions & 20 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -925,25 +925,10 @@
},
"omnisharp.useModernNet": {
"type": "boolean",
"default": false,
"default": true,
"scope": "window",
"title": "Use .NET 6 build of OmniSharp (experimental)",
"description": "Use OmniSharp build for .NET 6. This version _does not_ support non-SDK-style .NET Framework projects, including Unity. SDK-style Framework, .NET Core, and .NET 5+ projects should see significant performance improvements, but there may still be bugs. Please open issues if you find any bugs."
},
"omnisharp.useGlobalMono": {
"type": "string",
"default": "auto",
"enum": [
"auto",
"always",
"never"
],
"enumDescriptions": [
"Automatically launch OmniSharp with internal \"mono\", since \"mono\" 6.12.0 does not support .NET Core 3.1.40x or .NET 5 SDKs.",
"Always launch OmniSharp with \"mono\". If version 6.4.0 or greater is not available on the PATH, an error will be printed.",
"Never launch OmniSharp on a globally-installed Mono."
],
"description": "Launch OmniSharp with the globally-installed Mono. If set to \"always\", \"mono\" version 6.4.0 or greater must be available on the PATH. If set to \"auto\", OmniSharp will be launched with \"mono\" if version 6.4.0 or greater is available on the PATH."
"title": "Use .NET 6 build of OmniSharp",
"description": "Use OmniSharp build for .NET 6. This version _does not_ support non-SDK-style .NET Framework projects, including Unity. SDK-style Framework, .NET Core, and .NET 5+ projects should see significant performance improvements."
},
"omnisharp.monoPath": {
"type": [
Expand All @@ -952,7 +937,7 @@
],
"default": null,
"scope": "machine",
"description": "Specifies the path to a mono installation to use when \"useGlobalMono\" is set to \"always\", instead of the default system one. Example: \"/Library/Frameworks/Mono.framework/Versions/Current\""
"description": "Specifies the path to a mono installation to use when \"useModenNet\" is set to false, instead of the default system one. Example: \"/Library/Frameworks/Mono.framework/Versions/Current\""
},
"omnisharp.dotnetPath": {
"type": [
Expand Down Expand Up @@ -4073,4 +4058,4 @@
}
]
}
}
}
2 changes: 1 addition & 1 deletion src/features/diagnosticsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class DiagnosticsProvider extends AbstractSupport {

this._subscriptions.push(this._validateCurrentDocumentPipe
.pipe(debounceTime(750))
.subscribe(x => this._validateDocument(x)));
.subscribe(async x => this._validateDocument(x)));

this._subscriptions.push(this._validateAllPipe
.pipe(debounceTime(3000))
Expand Down
5 changes: 5 additions & 0 deletions src/features/fileOpenCloseProvider.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IDisposable } from "../Disposable";
import { OmniSharpServer } from "../omnisharp/server";
import * as vscode from 'vscode';
Expand Down
9 changes: 3 additions & 6 deletions src/features/reportIssue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,9 @@ async function getMonoIfPlatformValid(isValidPlatformForMono: boolean, options:
if (isValidPlatformForMono) {
let monoVersion: string;
try {
let globalMonoInfo = await monoResolver.getHostExecutableInfo(options);
if (globalMonoInfo) {
monoVersion = `OmniSharp using global mono :${globalMonoInfo.version}`;
}
else {
monoVersion = `OmniSharp using built-in mono`;
let monoInfo = await monoResolver.getHostExecutableInfo(options);
if (monoInfo) {
monoVersion = `OmniSharp using mono :${monoInfo.version}`;
}
}
catch (error) {
Expand Down
1 change: 0 additions & 1 deletion src/observers/OptionChangeObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ type OptionsKey = keyof Options;

const omniSharpOptions: ReadonlyArray<OptionsKey> = [
"path",
"useGlobalMono",
"enableMsBuildLoadProjectsOnDemand",
"waitForDebugger",
"loggingLevel",
Expand Down
42 changes: 17 additions & 25 deletions src/omnisharp/OmniSharpMonoResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@ export class OmniSharpMonoResolver implements IHostExecutableResolver {
}

private async configureEnvironmentAndGetInfo(options: Options): Promise<HostExecutableInformation> {
let env = { ...process.env };
const env = { ...process.env };
let monoPath: string;
if (options.useGlobalMono !== "never" && options.monoPath !== undefined) {

if (options.monoPath !== undefined) {
env['PATH'] = path.join(options.monoPath, 'bin') + path.delimiter + env['PATH'];
env['MONO_GAC_PREFIX'] = options.monoPath;
monoPath = options.monoPath;
}

let version = await this.getMonoVersion(env);
const version = await this.getMonoVersion(env);

return {
version,
Expand All @@ -35,30 +36,21 @@ export class OmniSharpMonoResolver implements IHostExecutableResolver {
}

public async getHostExecutableInfo(options: Options): Promise<HostExecutableInformation> {
let monoInfo = await this.configureEnvironmentAndGetInfo(options);
let isValid = monoInfo.version && satisfies(monoInfo.version, `>=${this.minimumMonoVersion}`);
if (options.useGlobalMono === "always") {
let isMissing = monoInfo.version === undefined;
if (isMissing) {
const suggestedAction = options.monoPath
? "Update the \"omnisharp.monoPath\" setting to point to the folder containing Mono's '/bin' folder."
: "Ensure that Mono's '/bin' folder is added to your environment's PATH variable.";
throw new Error(`Unable to find Mono. ${suggestedAction}`);
}

if (!isValid) {
throw new Error(`Found Mono version ${monoInfo.version}. Cannot start OmniSharp because Mono version >=${this.minimumMonoVersion} is required.`);
}

return monoInfo;
const monoInfo = await this.configureEnvironmentAndGetInfo(options);
const isValid = monoInfo.version && satisfies(monoInfo.version, `>=${this.minimumMonoVersion}`);

const isMissing = monoInfo.version === undefined;
if (isMissing) {
const suggestedAction = options.monoPath
? "Update the \"omnisharp.monoPath\" setting to point to the folder containing Mono's '/bin' folder."
: "Ensure that Mono's '/bin' folder is added to your environment's PATH variable.";
throw new Error(`Unable to find Mono. ${suggestedAction}`);
}
else if (options.useGlobalMono === "auto" && isValid) {
// While waiting for Mono to ship with a MSBuild version 16.8 or higher, we will treat "auto"
// as "Use included Mono".
// return monoInfo;

if (!isValid) {
throw new Error(`Found Mono version ${monoInfo.version}. Cannot start OmniSharp because Mono version >=${this.minimumMonoVersion} is required.`);
}

return undefined;
return monoInfo;
}
}

38 changes: 10 additions & 28 deletions src/omnisharp/launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,21 +312,7 @@ async function launch(cwd: string, args: string[], launchInfo: LaunchInfo, platf
return launchWindows(launchInfo.LaunchPath, cwd, args);
}

let monoInfo = await monoResolver.getHostExecutableInfo(options);

if (monoInfo) {
const launchPath = launchInfo.MonoLaunchPath || launchInfo.LaunchPath;
let childEnv = monoInfo.env;
return {
...launchNixMono(launchPath, cwd, args, childEnv, options.waitForDebugger),
hostIsMono: true,
hostVersion: monoInfo.version,
hostPath: monoInfo.path
};
}
else {
return launchNix(launchInfo.LaunchPath, cwd, args);
}
return await launchNix(launchInfo, cwd, args, options, monoResolver);
}

function getConfigurationValue(globalConfig: vscode.WorkspaceConfiguration, csharpConfig: vscode.WorkspaceConfiguration,
Expand Down Expand Up @@ -393,20 +379,20 @@ function launchWindows(launchPath: string, cwd: string, args: string[]): LaunchR
};
}

function launchNix(launchPath: string, cwd: string, args: string[]): LaunchResult {
let process = spawn(launchPath, args, {
detached: false,
cwd: cwd
});
async function launchNix(launchInfo: LaunchInfo, cwd: string, args: string[], options: Options, monoResolver: IHostExecutableResolver): Promise<LaunchResult> {
const monoInfo = await monoResolver.getHostExecutableInfo(options);
const launchPath = launchInfo.MonoLaunchPath || launchInfo.LaunchPath;

return {
process,
process: launchNixMono(launchPath, cwd, args, monoInfo.env, options.waitForDebugger),
command: launchPath,
hostIsMono: false
hostIsMono: true,
hostVersion: monoInfo.version,
hostPath: monoInfo.path
};
}

function launchNixMono(launchPath: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv, useDebugger: boolean): LaunchResult {
function launchNixMono(launchPath: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv, useDebugger: boolean): ChildProcess {
let argsCopy = args.slice(0); // create copy of details args
argsCopy.unshift(launchPath);
argsCopy.unshift("--assembly-loader=strict");
Expand All @@ -422,9 +408,5 @@ function launchNixMono(launchPath: string, cwd: string, args: string[], environm
env: environment
});

return {
process,
command: launchPath,
hostIsMono: true
};
return process;
}
29 changes: 0 additions & 29 deletions src/omnisharp/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export class Options {
constructor(
public path: string,
public useModernNet: boolean,
public useGlobalMono: string,
public waitForDebugger: boolean,
public loggingLevel: string,
public autoStart: boolean,
Expand Down Expand Up @@ -62,16 +61,13 @@ export class Options {
// are supported below. In particular, these are:
//
// - "csharp.omnisharp" -> "omnisharp.path"
// - "csharp.omnisharpUsesMono" -> "omnisharp.useMono"
// - "omnisharp.useMono" -> "omnisharp.useGlobalMono"

const omnisharpConfig = vscode.workspace.getConfiguration('omnisharp');
const csharpConfig = vscode.workspace.getConfiguration('csharp');
const razorConfig = vscode.workspace.getConfiguration('razor');

const path = Options.readPathOption(csharpConfig, omnisharpConfig);
const useModernNet = omnisharpConfig.get<boolean>("useModernNet", false);
const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig);
const monoPath = omnisharpConfig.get<string>('monoPath', undefined) || undefined;
const dotnetPath = omnisharpConfig.get<string>('dotnetPath', undefined) || undefined;

Expand Down Expand Up @@ -143,7 +139,6 @@ export class Options {
return new Options(
path,
useModernNet,
useGlobalMono,
waitForDebugger,
loggingLevel,
autoStart,
Expand Down Expand Up @@ -232,28 +227,4 @@ export class Options {
return null;
}
}

private static readUseGlobalMonoOption(omnisharpConfig: WorkspaceConfiguration, csharpConfig: WorkspaceConfiguration): string {
function toUseGlobalMonoValue(value: boolean): string {
// True means 'always' and false means 'auto'.
return value ? "always" : "auto";
}

if (omnisharpConfig.has('useGlobalMono')) {
// If 'omnisharp.useGlobalMono' setting was found, just use it.
return omnisharpConfig.get<string>('useGlobalMono', "auto");
}
else if (omnisharpConfig.has('useMono')) {
// BACKCOMPAT: If 'omnisharp.useMono' setting was found, true maps to "always" and false maps to "auto"
return toUseGlobalMonoValue(omnisharpConfig.get<boolean>('useMono'));
}
else if (csharpConfig.has('omnisharpUsesMono')) {
// BACKCOMPAT: If 'csharp.omnisharpUsesMono' setting was found, true maps to "always" and false maps to "auto"
return toUseGlobalMonoValue(csharpConfig.get<boolean>('omnisharpUsesMono'));
}
else {
// Otherwise, the default value is "auto".
return "auto";
}
}
}
16 changes: 9 additions & 7 deletions tasks/offlinePackagingTasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ import { getRuntimeDependenciesPackages } from '../src/tools/RuntimeDependencyPa
import { getAbsolutePathPackagesToInstall } from '../src/packageManager/getAbsolutePathPackagesToInstall';
import { isValidDownload } from '../src/packageManager/isValidDownload';

const includeFrameworkOmniSharp = false;

export const offlinePackages = [
{ platformInfo: new PlatformInformation('win32', 'x86_64'), id: "win32-x64", isFramework: true },
{ platformInfo: new PlatformInformation('win32', 'x86'), id: "win32-ia32", isFramework: true },
{ platformInfo: new PlatformInformation('win32', 'arm64'), id: "win32-arm64", isFramework: true },
{ platformInfo: new PlatformInformation('linux', 'x86_64'), id: "linux-x64", isFramework: true },
{ platformInfo: new PlatformInformation('linux', 'arm64'), id: "linux-arm64", isFramework: true },
{ platformInfo: new PlatformInformation('darwin', 'x86_64'), id: "darwin-x64", isFramework: true },
{ platformInfo: new PlatformInformation('darwin', 'arm64'), id: "darwin-arm64", isFramework: true },
{ platformInfo: new PlatformInformation('win32', 'x86_64'), id: "win32-x64", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('win32', 'x86'), id: "win32-ia32", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('win32', 'arm64'), id: "win32-arm64", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('linux', 'x86_64'), id: "linux-x64", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('linux', 'arm64'), id: "linux-arm64", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('darwin', 'x86_64'), id: "darwin-x64", isFramework: includeFrameworkOmniSharp },
{ platformInfo: new PlatformInformation('darwin', 'arm64'), id: "darwin-arm64", isFramework: includeFrameworkOmniSharp },
];

export function getPackageName(packageJSON: any, vscodePlatformId: string) {
Expand Down
11 changes: 0 additions & 11 deletions test-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -475,17 +475,6 @@ Changing this option should result in a notification message at the bottom right
* if the option is not set, the OmniSharp log should indicate that the registered MSBuild instance is either the Standalone MSBuild, a Visual Studio MSBuild instance, or a Mono MSBuild isntance.
* if the option is set, the OmniSharp log should inlcude text like the following "OmniSharp server started with .NET 6.0.100" and "Registered MSBuild instance: .NET Core SDK 6.0.100 17.0.0 - "/usr/local/share/dotnet/sdk/6.0.100/". All language services should continue to work as expected when an SDK-style project is open.

#### omnisharp.useGlobalMono (for Linux/Mac)
This option can be set to any of the following values:
* "auto" - Will launch OmniSharp using mono if version>=5.2.0 is installed but will launch using the run script if that is not so.
* "always" - Will launch OmniSharp using mono if version>=5.2.0 is installed and will throw an error otherwise.
* "never" - Launches OmniSharp without using the global mono

The value of OmniSharp path displayed in the OmniSharp log can be used to know if OmniSharp has launched using mono or not. If it is running using global mono, the path will end with "OmniSharp.exe" else the path will end with "run".
For using this option, mono version greater than or equal to 5.2.0 must be installed. If that is not so, setting this option to true, should give an error.
* If the option is not set, the OmniSharp path displayed in the "OmniSharp Log" should end with "run"
* If the option is set, the OmniSharp path as mentioned above should end with "OmniSharp.exe"

#### omnisharp.path
Setting this path to any of the values as listed below, should start the OmniSharp server and display the correct OmniSharp path in the `OmniSharp Log`(View --> Output--> OmniSharp Log).
* undefined - OmniSharp server must start using the copy of omnisharp shipped with the extension, that is, the OmniSharp path must be the extension path, followed by .omnisharp followed by the default omnisharp version as present in the package.json and the platform-specific executable.
Expand Down
6 changes: 3 additions & 3 deletions test/unitTests/Fakes/FakeMonoResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ export const fakeMonoInfo: HostExecutableInformation = {
};

export class FakeMonoResolver implements IHostExecutableResolver {
public getGlobalMonoCalled: boolean;
public getMonoCalled: boolean;

constructor(public willReturnMonoInfo = true) {
this.getGlobalMonoCalled = false;
this.getMonoCalled = false;
}

async getHostExecutableInfo(): Promise<HostExecutableInformation> {
this.getGlobalMonoCalled = true;
this.getMonoCalled = true;
if (this.willReturnMonoInfo) {
return Promise.resolve(fakeMonoInfo);
}
Expand Down
1 change: 0 additions & 1 deletion test/unitTests/Fakes/FakeOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export function getEmptyOptions(): Options {
return new Options(
/* path */"",
/* useModernNet */false,
/* useGlobalMono */"",
/* waitForDebugger */false,
/* loggingLevel */"",
/* autoStart */false,
Expand Down
2 changes: 1 addition & 1 deletion test/unitTests/OptionObserver/OptionChangeObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ suite("OmniSharpConfigChangeObserver", () => {
{ config: "omnisharp", section: "path", value: "somePath" },
{ config: "omnisharp", section: "waitForDebugger", value: true },
{ config: "omnisharp", section: "enableMsBuildLoadProjectsOnDemand", value: true },
{ config: "omnisharp", section: "useGlobalMono", value: "always" },
{ config: "omnisharp", section: "useModernNet", value: true },
{ config: "omnisharp", section: 'loggingLevel', value: 'verbose' }
].forEach(elem => {
suite(`When the ${elem.config} ${elem.section} changes`, () => {
Expand Down
15 changes: 4 additions & 11 deletions test/unitTests/features/reportIssue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,25 +100,18 @@ suite(`${reportIssue.name}`, () => {

test("mono information is obtained when it is a valid mono platform", async () => {
await reportIssue(vscode, eventStream, getDotnetInfo, isValidForMono, options, fakeMonoResolver);
expect(fakeMonoResolver.getGlobalMonoCalled).to.be.equal(true);
expect(fakeMonoResolver.getMonoCalled).to.be.equal(true);
});

test("mono version is put in the body when shouldUseGlobalMono returns a monoInfo", async () => {
test("mono version is put in the body when it is a valid mono platform", async () => {
await reportIssue(vscode, eventStream, getDotnetInfo, isValidForMono, options, fakeMonoResolver);
expect(fakeMonoResolver.getGlobalMonoCalled).to.be.equal(true);
expect(fakeMonoResolver.getMonoCalled).to.be.equal(true);
expect(issueBody).to.contain(fakeMonoInfo.version);
});

test("built-in mono usage message is put in the body when shouldUseGlobalMono returns a null", async () => {
fakeMonoResolver = new FakeMonoResolver(false);
await reportIssue(vscode, eventStream, getDotnetInfo, isValidForMono, options, fakeMonoResolver);
expect(fakeMonoResolver.getGlobalMonoCalled).to.be.equal(true);
expect(issueBody).to.contain(`OmniSharp using built-in mono`);
});

test("mono information is not obtained when it is not a valid mono platform", async () => {
await reportIssue(vscode, eventStream, getDotnetInfo, false, options, fakeMonoResolver);
expect(fakeMonoResolver.getGlobalMonoCalled).to.be.equal(false);
expect(fakeMonoResolver.getMonoCalled).to.be.equal(false);
});

test("The url contains the name, publisher and version for all the extensions that are not builtin", async () => {
Expand Down
Loading

0 comments on commit cd81b5c

Please sign in to comment.