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

Add option to run with a custom mono installation #2425

Merged
merged 5 commits into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 14 additions & 7 deletions src/omnisharp/launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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");

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add some logging here, like printing the mono launch path, just like we print the version?

We create the log event here from the launch result: https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/server.ts#L324. We might also return a monoLaunchPath in the launch result and display it here: https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/observers/OmnisharpLoggerObserver.ts#L58. We will have to add the appropriate property to OmnisharpLaunch event here: https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/omnisharp/loggingEvents.ts#L30.
Basically to do logging in omnisharp-vscode, we post the events into an event stream to which some observers subscribe. Whenever we post an event to the event stream, all the observers are invoked and take action appropriately. Does that make sense ?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I hacked up some manual logging directly to a separate window just so I could see what was going on while I was putting this together, but adding some more information to the event stream probably makes sense, I can give that a try.

Copy link
Author

Choose a reason for hiding this comment

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

@akshita31 I added some logging and in the process tweaked the test method a bit to avoid growing the if block, let me know if that's what you had in mind.

let process = spawn('mono', argsCopy, {
detached: false,
cwd: cwd
cwd: cwd,
env: environment
});

return {
Expand All @@ -323,13 +330,13 @@ function launchNixMono(launchPath: string, monoVersion: string, cwd: string, arg
};
}

async function getMonoVersion(): Promise<string> {
async function getMonoVersion(environment: NodeJS.ProcessEnv): Promise<string> {
const versionRegexp = /(\d+\.\d+\.\d+)/;

return new Promise<string>((resolve, reject) => {
let childprocess: ChildProcess;
try {
childprocess = spawn('mono', ['--version']);
childprocess = spawn('mono', ['--version'], { env: environment });
}
catch (e) {
return resolve(undefined);
Expand Down
10 changes: 7 additions & 3 deletions src/omnisharp/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -36,6 +37,7 @@ export class Options {

const path = Options.readPathOption(csharpConfig, omnisharpConfig);
const useGlobalMono = Options.readUseGlobalMonoOption(omnisharpConfig, csharpConfig);
const monoPath = omnisharpConfig.get<string>('monoPath', undefined);

const waitForDebugger = omnisharpConfig.get<boolean>('waitForDebugger', false);

Expand Down Expand Up @@ -75,7 +77,9 @@ export class Options {
showTestsCodeLens,
disableCodeActions,
disableMSBuildDiagnosticWarning,
defaultLaunchSolution);
defaultLaunchSolution,
monoPath,
);
}

private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | null {
Expand Down
1 change: 1 addition & 0 deletions test/unitTests/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down