From 70ceaccecc96d3c8c18acc311aecf3574de569a2 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 19 Jul 2018 11:53:55 +0200 Subject: [PATCH 1/4] Add option to run with a custom mono installation When `useGlobalMono` is set to `always` or `auto`, the launcher uses whatever mono it finds in the path, which may or may not be a usable mono version for omnisharp. The user may have a suitable mono installation for omnisharp somewhere else, but not set as the default system one, or they may want to have omnisharp launched with a specific mono for omnisharp for debugging purposes. This adds a `monoPath` configuration option, and changes the launcher so that the environment variables PATH and MONO_GAC_PREFIX are changed to include the `monoPath` value, if set, when launching mono. --- package.json | 8 ++++++++ src/omnisharp/launcher.ts | 21 ++++++++++++++------- src/omnisharp/options.ts | 10 +++++++--- test/unitTests/options.test.ts | 1 + 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 92a60818a..5d46de20c 100644 --- a/package.json +++ b/package.json @@ -535,6 +535,14 @@ ], "description": "Launch OmniSharp with the globally-installed Mono. If set to \"always\", \"mono\" version 5.8.1 or greater must be available on the PATH. If set to \"auto\", OmniSharp will be launched with \"mono\" if version 5.8.1 or greater is available on the PATH." }, + "omnisharp.monoPath": { + "type": [ + "string", + "null" + ], + "default": null, + "description": "Specifies the path to a mono installation to use when \"useGlobalMono\" is set to \"always\" or \"auto\", instead of the default system one." + }, "omnisharp.waitForDebugger": { "type": "boolean", "default": false, diff --git a/src/omnisharp/launcher.ts b/src/omnisharp/launcher.ts index 8a5233564..d9149158d 100644 --- a/src/omnisharp/launcher.ts +++ b/src/omnisharp/launcher.ts @@ -233,7 +233,13 @@ async function launch(cwd: string, args: string[], launchInfo: LaunchInfo, platf return launchWindows(launchInfo.LaunchPath, cwd, args); } - let monoVersion = await getMonoVersion(); + let childEnv = { ...process.env }; + if (options.useGlobalMono !== "never" && options.monoPath !== undefined) { + childEnv['PATH'] = path.join(options.monoPath, 'bin') + path.delimiter + childEnv['PATH']; + childEnv['MONO_GAC_PREFIX'] = options.monoPath; + } + + let monoVersion = await getMonoVersion(childEnv); let isValidMonoAvailable = await satisfies(monoVersion, '>=5.8.1'); // If the user specifically said that they wanted to launch on Mono, respect their wishes. @@ -244,12 +250,12 @@ async function launch(cwd: string, args: string[], launchInfo: LaunchInfo, platf const launchPath = launchInfo.MonoLaunchPath || launchInfo.LaunchPath; - return launchNixMono(launchPath, monoVersion, cwd, args); + return launchNixMono(launchPath, monoVersion, cwd, args, childEnv); } // If we can launch on the global Mono, do so; otherwise, launch directly; if (options.useGlobalMono === "auto" && isValidMonoAvailable && launchInfo.MonoLaunchPath) { - return launchNixMono(launchInfo.MonoLaunchPath, monoVersion, cwd, args); + return launchNixMono(launchInfo.MonoLaunchPath, monoVersion, cwd, args, childEnv); } else { return launchNix(launchInfo.LaunchPath, cwd, args); @@ -306,14 +312,15 @@ function launchNix(launchPath: string, cwd: string, args: string[]): LaunchResul }; } -function launchNixMono(launchPath: string, monoVersion: string, cwd: string, args: string[]): LaunchResult { +function launchNixMono(launchPath: string, monoVersion: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv): LaunchResult { let argsCopy = args.slice(0); // create copy of details args argsCopy.unshift(launchPath); argsCopy.unshift("--assembly-loader=strict"); let process = spawn('mono', argsCopy, { detached: false, - cwd: cwd + cwd: cwd, + env: environment }); return { @@ -323,13 +330,13 @@ function launchNixMono(launchPath: string, monoVersion: string, cwd: string, arg }; } -async function getMonoVersion(): Promise { +async function getMonoVersion(environment: NodeJS.ProcessEnv): Promise { const versionRegexp = /(\d+\.\d+\.\d+)/; return new Promise((resolve, reject) => { let childprocess: ChildProcess; try { - childprocess = spawn('mono', ['--version']); + childprocess = spawn('mono', ['--version'], { env: environment }); } catch (e) { return resolve(undefined); diff --git a/src/omnisharp/options.ts b/src/omnisharp/options.ts index 3c7e7d947..fb489d40a 100644 --- a/src/omnisharp/options.ts +++ b/src/omnisharp/options.ts @@ -20,8 +20,9 @@ export class Options { public showTestsCodeLens: boolean, public disableCodeActions: boolean, public disableMSBuildDiagnosticWarning: boolean, - public defaultLaunchSolution?: string) { } - + public defaultLaunchSolution?: string, + public monoPath?: string) { } + public static Read(vscode: vscode): Options { // Extra effort is taken below to ensure that legacy versions of options @@ -36,6 +37,7 @@ export class Options { const path = Options.readPathOption(csharpConfig, omnisharpConfig); const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig); + const monoPath = omnisharpConfig.get('monoPath', undefined); const waitForDebugger = omnisharpConfig.get('waitForDebugger', false); @@ -75,7 +77,9 @@ export class Options { showTestsCodeLens, disableCodeActions, disableMSBuildDiagnosticWarning, - defaultLaunchSolution); + defaultLaunchSolution, + monoPath, + ); } private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | null { diff --git a/test/unitTests/options.test.ts b/test/unitTests/options.test.ts index 09ab064ff..dd2166dca 100644 --- a/test/unitTests/options.test.ts +++ b/test/unitTests/options.test.ts @@ -16,6 +16,7 @@ suite("Options tests", () => { const options = Options.Read(vscode); expect(options.path).to.be.null; options.useGlobalMono.should.equal("auto"); + expect(options.monoPath).to.be.undefined; options.waitForDebugger.should.equal(false); options.loggingLevel.should.equal("information"); options.autoStart.should.equal(true); From c181c741e6b7ea64bbd4b88646a327c932513a2f Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Fri, 20 Jul 2018 12:31:50 +0200 Subject: [PATCH 2/4] VSCode returns null instead of undefined when there's no value, which made other comparisons fail :/ --- src/omnisharp/options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/omnisharp/options.ts b/src/omnisharp/options.ts index fb489d40a..f38816d56 100644 --- a/src/omnisharp/options.ts +++ b/src/omnisharp/options.ts @@ -37,7 +37,7 @@ export class Options { const path = Options.readPathOption(csharpConfig, omnisharpConfig); const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig); - const monoPath = omnisharpConfig.get('monoPath', undefined); + const monoPath = omnisharpConfig.get('monoPath', undefined) || undefined; const waitForDebugger = omnisharpConfig.get('waitForDebugger', false); From a7226d6e6ab645a93f26fae533c35c2e5937be78 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Mon, 23 Jul 2018 17:08:12 +0200 Subject: [PATCH 3/4] Log the mono path when it's set, and add tests for logging output --- src/observers/OmnisharpLoggerObserver.ts | 10 ++++++---- src/omnisharp/loggingEvents.ts | 2 +- src/omnisharp/server.ts | 2 +- .../logging/OmnisharpLoggerObserver.test.ts | 18 ++++++++---------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/observers/OmnisharpLoggerObserver.ts b/src/observers/OmnisharpLoggerObserver.ts index 5a53f1268..9d7ebc403 100644 --- a/src/observers/OmnisharpLoggerObserver.ts +++ b/src/observers/OmnisharpLoggerObserver.ts @@ -54,12 +54,14 @@ export class OmnisharpLoggerObserver extends BaseLoggerObserver { } private handleOmnisharpLaunch(event: OmnisharpLaunch) { + this.logger.append(`OmniSharp server started`); if (event.monoVersion) { - this.logger.appendLine(`OmniSharp server started with Mono ${event.monoVersion}`); - } - else { - this.logger.appendLine(`OmniSharp server started`); + this.logger.append(` with Mono ${event.monoVersion}`); + if (event.monoPath !== undefined) { + this.logger.append(` (${event.monoPath})`); + } } + this.logger.appendLine('.'); this.logger.increaseIndent(); this.logger.appendLine(`Path: ${event.command}`); diff --git a/src/omnisharp/loggingEvents.ts b/src/omnisharp/loggingEvents.ts index da9c58d2c..f0679ecd6 100644 --- a/src/omnisharp/loggingEvents.ts +++ b/src/omnisharp/loggingEvents.ts @@ -27,7 +27,7 @@ export class OmnisharpInitialisation implements BaseEvent { } export class OmnisharpLaunch implements BaseEvent { - constructor(public monoVersion: string, public command: string, public pid: number) { } + constructor(public monoVersion: string, public command: string, public pid: number, public monoPath?: string) { } } export class PackageInstallation implements BaseEvent { diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 0b75a47de..22b1bfe00 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -321,7 +321,7 @@ export class OmniSharpServer { try { let launchResult = await launchOmniSharp(cwd, args, launchInfo, this.platformInfo, options); - this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.monoVersion, launchResult.command, launchResult.process.pid)); + this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.monoVersion, launchResult.command, launchResult.process.pid, options.monoPath)); this._serverProcess = launchResult.process; this._delayTrackers = {}; diff --git a/test/unitTests/logging/OmnisharpLoggerObserver.test.ts b/test/unitTests/logging/OmnisharpLoggerObserver.test.ts index 25a601268..cb8fd0538 100644 --- a/test/unitTests/logging/OmnisharpLoggerObserver.test.ts +++ b/test/unitTests/logging/OmnisharpLoggerObserver.test.ts @@ -135,9 +135,12 @@ suite("OmnisharpLoggerObserver", () => { suite('OmnisharpLaunch', () => { [ - new OmnisharpLaunch("5.8.0", "someCommand", 4), - new OmnisharpLaunch(undefined, "someCommand", 4) - ].forEach((event: OmnisharpLaunch) => { + { 'event': new OmnisharpLaunch("5.12.0", "someCommand", 4), 'expected': "OmniSharp server started with Mono 5.12.0." }, + { 'event': new OmnisharpLaunch(undefined, "someCommand", 4), 'expected': "OmniSharp server started." }, + { 'event': new OmnisharpLaunch("5.12.0", "someCommand", 4, 'path to mono'), 'expected': "OmniSharp server started with Mono 5.12.0 (path to mono)." }, + { 'event': new OmnisharpLaunch(undefined, "someCommand", 4, 'path to mono'), 'expected': "OmniSharp server started." }, + ].forEach((data: { event: OmnisharpLaunch, expected: string }) => { + const event = data.event; test(`Command and Pid are displayed`, () => { observer.post(event); @@ -145,14 +148,9 @@ suite("OmnisharpLoggerObserver", () => { expect(logOutput).to.contain(event.pid); }); - test(`Message is displayed depending on usingMono value`, () => { + test(`Message is displayed depending on monoVersion and monoPath value`, () => { observer.post(event); - if (event.monoVersion) { - expect(logOutput).to.contain("OmniSharp server started with Mono 5.8.0"); - } - else { - expect(logOutput).to.contain("OmniSharp server started"); - } + expect(logOutput).to.contain(data.expected); }); }); }); From 69ba549021ff52c31cfe13bbd746c6a91f0c02a3 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Tue, 24 Jul 2018 09:45:15 +0200 Subject: [PATCH 4/4] Record the mono path in the launch result for the log Also no reason why the logging object should have optional arguments, fix that. --- src/omnisharp/launcher.ts | 8 +++++--- src/omnisharp/loggingEvents.ts | 2 +- src/omnisharp/server.ts | 2 +- test/unitTests/logging/OmnisharpLoggerObserver.test.ts | 8 ++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/omnisharp/launcher.ts b/src/omnisharp/launcher.ts index d9149158d..5aada6e00 100644 --- a/src/omnisharp/launcher.ts +++ b/src/omnisharp/launcher.ts @@ -199,6 +199,7 @@ export interface LaunchResult { process: ChildProcess; command: string; monoVersion?: string; + monoPath?: string; } export async function launchOmniSharp(cwd: string, args: string[], launchInfo: LaunchInfo, platformInfo: PlatformInformation, options: Options): Promise { @@ -250,12 +251,12 @@ async function launch(cwd: string, args: string[], launchInfo: LaunchInfo, platf const launchPath = launchInfo.MonoLaunchPath || launchInfo.LaunchPath; - return launchNixMono(launchPath, monoVersion, cwd, args, childEnv); + return launchNixMono(launchPath, monoVersion, options.monoPath, cwd, args, childEnv); } // If we can launch on the global Mono, do so; otherwise, launch directly; if (options.useGlobalMono === "auto" && isValidMonoAvailable && launchInfo.MonoLaunchPath) { - return launchNixMono(launchInfo.MonoLaunchPath, monoVersion, cwd, args, childEnv); + return launchNixMono(launchInfo.MonoLaunchPath, monoVersion, options.monoPath, cwd, args, childEnv); } else { return launchNix(launchInfo.LaunchPath, cwd, args); @@ -312,7 +313,7 @@ function launchNix(launchPath: string, cwd: string, args: string[]): LaunchResul }; } -function launchNixMono(launchPath: string, monoVersion: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv): LaunchResult { +function launchNixMono(launchPath: string, monoVersion: string, monoPath: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv): LaunchResult { let argsCopy = args.slice(0); // create copy of details args argsCopy.unshift(launchPath); argsCopy.unshift("--assembly-loader=strict"); @@ -327,6 +328,7 @@ function launchNixMono(launchPath: string, monoVersion: string, cwd: string, arg process, command: launchPath, monoVersion, + monoPath, }; } diff --git a/src/omnisharp/loggingEvents.ts b/src/omnisharp/loggingEvents.ts index f0679ecd6..13eb6b52c 100644 --- a/src/omnisharp/loggingEvents.ts +++ b/src/omnisharp/loggingEvents.ts @@ -27,7 +27,7 @@ export class OmnisharpInitialisation implements BaseEvent { } export class OmnisharpLaunch implements BaseEvent { - constructor(public monoVersion: string, public command: string, public pid: number, public monoPath?: string) { } + constructor(public monoVersion: string, public monoPath: string, public command: string, public pid: number) { } } export class PackageInstallation implements BaseEvent { diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 22b1bfe00..72dca5417 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -321,7 +321,7 @@ export class OmniSharpServer { try { let launchResult = await launchOmniSharp(cwd, args, launchInfo, this.platformInfo, options); - this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.monoVersion, launchResult.command, launchResult.process.pid, options.monoPath)); + this.eventStream.post(new ObservableEvents.OmnisharpLaunch(launchResult.monoVersion, launchResult.monoPath, launchResult.command, launchResult.process.pid)); this._serverProcess = launchResult.process; this._delayTrackers = {}; diff --git a/test/unitTests/logging/OmnisharpLoggerObserver.test.ts b/test/unitTests/logging/OmnisharpLoggerObserver.test.ts index cb8fd0538..8a63370b5 100644 --- a/test/unitTests/logging/OmnisharpLoggerObserver.test.ts +++ b/test/unitTests/logging/OmnisharpLoggerObserver.test.ts @@ -135,10 +135,10 @@ suite("OmnisharpLoggerObserver", () => { suite('OmnisharpLaunch', () => { [ - { 'event': new OmnisharpLaunch("5.12.0", "someCommand", 4), 'expected': "OmniSharp server started with Mono 5.12.0." }, - { 'event': new OmnisharpLaunch(undefined, "someCommand", 4), 'expected': "OmniSharp server started." }, - { 'event': new OmnisharpLaunch("5.12.0", "someCommand", 4, 'path to mono'), 'expected': "OmniSharp server started with Mono 5.12.0 (path to mono)." }, - { 'event': new OmnisharpLaunch(undefined, "someCommand", 4, 'path to mono'), 'expected': "OmniSharp server started." }, + { 'event': new OmnisharpLaunch("5.8.0", undefined, "someCommand", 4), 'expected': "OmniSharp server started with Mono 5.8.0." }, + { 'event': new OmnisharpLaunch(undefined, undefined, "someCommand", 4), 'expected': "OmniSharp server started." }, + { 'event': new OmnisharpLaunch("5.8.0", "path to mono", "someCommand", 4), 'expected': "OmniSharp server started with Mono 5.8.0 (path to mono)." }, + { 'event': new OmnisharpLaunch(undefined, "path to mono", "someCommand", 4), 'expected': "OmniSharp server started." }, ].forEach((data: { event: OmnisharpLaunch, expected: string }) => { const event = data.event;