From eed7659f961d228f60dd3163e06a00b75fc0ef3e Mon Sep 17 00:00:00 2001 From: extr0py Date: Tue, 25 Jul 2017 09:11:40 -0700 Subject: [PATCH] Fix #553: Add additional, optional paths to spawned processes (#556) * Start refactoring process spawning to common code path * Refactor process management to separate namespace * Add information to README.md * Factor out mergeSpawnOptions, and use current working directory for language server process * Pass full set of path variables * Fix path variable setting in Windows * Add additional paths default to Linux as well * Fix lint issue --- README.md | 1 + browser/src/Config.ts | 14 ++++ .../Api/LanguageClient/LanguageClient.ts | 14 ++-- browser/src/Plugins/Api/Oni.ts | 39 +++------- browser/src/Plugins/Api/Process.ts | 76 +++++++++++++++++++ definitions/Oni.d.ts | 8 ++ vim/core/oni-plugin-tslint/lib/index.js | 2 +- .../src/TypeScriptServerHost.ts | 2 +- 8 files changed, 121 insertions(+), 35 deletions(-) create mode 100644 browser/src/Plugins/Api/Process.ts diff --git a/README.md b/README.md index cc2ee5244a..dc602fca30 100644 --- a/README.md +++ b/README.md @@ -354,6 +354,7 @@ A few interesting configuration options to set: - `editor.backgroundImageUrl` - specific a custom background image - `editor.backgroundImageSize` - specific a custom background size (cover, contain) - `editor.scrollBar.visible` - (default: `true`) sets whether the buffer scrollbar is visible +- `environment.additionalPaths` - (default: `[] on Windows, ['/usr/bin', '/usr/local/bin'] on OSX and Linux`). Sets additional paths for binaries. This may be necessary to configure, if using plugins or a Language Server that is not in the default set of runtime paths. Note that depending on where you launch Oni, there may be a different set of runtime paths sent to it - you can always check by opening the developer tools and running `process.env.PATH` in the console. See the `Config.ts` file for other interesting values to set. diff --git a/browser/src/Config.ts b/browser/src/Config.ts index 8b357c781f..9110e50e34 100644 --- a/browser/src/Config.ts +++ b/browser/src/Config.ts @@ -73,6 +73,10 @@ export interface IConfigValues { // If true (default), the buffer scroll bar will be visible "editor.scrollBar.visible": boolean + // Additional paths to include when launching sub-process from Oni + // (and available in terminal integration, later) + "environment.additionalPaths": string[] + // Command to list files for 'quick open' // For example, to use 'ag': ag --nocolor -l . // @@ -144,6 +148,8 @@ export class Config extends EventEmitter { "editor.cursorColumn": false, "editor.cursorColumnOpacity": 0.1, + "environment.additionalPaths": [], + "statusbar.enabled": true, "statusbar.fontSize": "12px", } @@ -152,6 +158,10 @@ export class Config extends EventEmitter { "editor.fontFamily": "Menlo", "editor.fontSize": "12px", "statusbar.fontSize": "10px", + "environment.additionalPaths": [ + "/usr/bin", + "/usr/local/bin", + ], } private WindowsConfig: Partial = { @@ -162,6 +172,10 @@ export class Config extends EventEmitter { private LinuxConfig: Partial = { "editor.fontFamily": "DejaVu Sans Mono", "statusbar.fontSize": "11px", + "environment.additionalPaths": [ + "/usr/bin", + "/usr/local/bin", + ], } private DefaultPlatformConfig = Platform.isWindows() ? this.WindowsConfig : Platform.isLinux() ? this.LinuxConfig : this.MacConfig diff --git a/browser/src/Plugins/Api/LanguageClient/LanguageClient.ts b/browser/src/Plugins/Api/LanguageClient/LanguageClient.ts index b76bdf1415..a176aee3ad 100644 --- a/browser/src/Plugins/Api/LanguageClient/LanguageClient.ts +++ b/browser/src/Plugins/Api/LanguageClient/LanguageClient.ts @@ -11,7 +11,7 @@ import * as _ from "lodash" import * as rpc from "vscode-jsonrpc" import * as types from "vscode-languageserver-types" -import { ChildProcess, spawn } from "child_process" +import { ChildProcess } from "child_process" import { getCompletionMeet } from "./../../../Services/AutoCompletionUtility" import { Oni } from "./../Oni" @@ -155,12 +155,16 @@ export class LanguageClient { public start(initializationParams: LanguageClientInitializationParams): Thenable { const startArgs = this._startOptions.args || [] + const options = { + cwd: process.cwd(), + } + if (this._startOptions.command) { - console.log(`[LANGUAGE CLIENT]: Starting process via '${this._startOptions.command}`) // tslint:disable-line no-console - this._process = spawn(this._startOptions.command, startArgs) + console.log(`[LANGUAGE CLIENT]: Starting process via '${this._startOptions.command}'`) // tslint:disable-line no-console + this._process = this._oni.process.spawnProcess(this._startOptions.command, startArgs, options) } else if (this._startOptions.module) { - console.log(`[LANGUAGE CLIENT]: Starting process via node script '${this._startOptions.module}`) // tslint:disable-line no-console - this._process = this._oni.spawnNodeScript(this._startOptions.module, startArgs) + console.log(`[LANGUAGE CLIENT]: Starting process via node script '${this._startOptions.module}'`) // tslint:disable-line no-console + this._process = this._oni.process.spawnNodeScript(this._startOptions.module, startArgs, options) } else { throw "A command or module must be specified to start the server" } diff --git a/browser/src/Plugins/Api/Oni.ts b/browser/src/Plugins/Api/Oni.ts index 07273d9994..35002afa9b 100644 --- a/browser/src/Plugins/Api/Oni.ts +++ b/browser/src/Plugins/Api/Oni.ts @@ -12,6 +12,7 @@ import { StatusBar } from "./StatusBar" import { DebouncedLanguageService } from "./DebouncedLanguageService" import { InitializationParamsCreator, LanguageClient } from "./LanguageClient/LanguageClient" +import { Process } from "./Process" import { Services } from "./Services" import { Ui } from "./Ui" @@ -37,6 +38,7 @@ export class Oni extends EventEmitter implements Oni.Plugin.Api { private _diagnostics: Oni.Plugin.Diagnostics.Api private _ui: Ui private _services: Services + private _process: Process public get commands(): Oni.Commands { return this._commands @@ -58,6 +60,10 @@ export class Oni extends EventEmitter implements Oni.Plugin.Api { return this._editor } + public get process(): Oni.Process { + return this._process + } + public get statusBar(): StatusBar { return this._statusBar } @@ -81,6 +87,7 @@ export class Oni extends EventEmitter implements Oni.Plugin.Api { this._statusBar = new StatusBar(this._channel) this._ui = new Ui(react) this._services = new Services() + this._process = new Process() this._channel.onRequest((arg: any) => { this._handleNotification(arg) @@ -96,43 +103,19 @@ export class Oni extends EventEmitter implements Oni.Plugin.Api { } public execNodeScript(scriptPath: string, args: string[] = [], options: ChildProcess.ExecOptions = {}, callback: (err: any, stdout: string, stderr: string) => void): ChildProcess.ChildProcess { - const requiredOptions = { - env: { - ...process.env, - ELECTRON_RUN_AS_NODE: 1, - }, - } - - const opts = { - ...options, - ...requiredOptions, - } + console.warn("WARNING: `Oni.execNodeScript` is deprecated. Please use `Oni.process.execNodeScript` instead") // tslint:disable-line no-console-log - const execOptions = [process.execPath, scriptPath].concat(args) - const execString = execOptions.map((s) => `"${s}"`).join(" ") - - return ChildProcess.exec(execString, opts, callback) + return this._process.execNodeScript(scriptPath, args, options, callback) } /** * Wrapper around `child_process.exec` to run using electron as opposed to node */ public spawnNodeScript(scriptPath: string, args: string[] = [], options: ChildProcess.SpawnOptions = {}): ChildProcess.ChildProcess { - const requiredOptions = { - env: { - ...process.env, - ELECTRON_RUN_AS_NODE: 1, - }, - } - - const opts = { - ...options, - ...requiredOptions, - } - const allArgs = [scriptPath].concat(args) + console.warn("WARNING: `Oni.spawnNodeScript` is deprecated. Please use `Oni.process.spawnNodeScript` instead") // tslint:disable-line no-console-log - return ChildProcess.spawn(process.execPath, allArgs, opts) + return this._process.spawnNodeScript(scriptPath, args, options) } public setHighlights(file: string, key: string, highlights: Oni.Plugin.SyntaxHighlight[]) { diff --git a/browser/src/Plugins/Api/Process.ts b/browser/src/Plugins/Api/Process.ts new file mode 100644 index 0000000000..127bf6df36 --- /dev/null +++ b/browser/src/Plugins/Api/Process.ts @@ -0,0 +1,76 @@ +import * as ChildProcess from "child_process" + +import * as Config from "./../../Config" +import * as Platform from "./../../Platform" + +const getPathSeparator = () => { + return Platform.isWindows() ? ";" : ":" +} + +const mergePathEnvironmentVariable = (currentPath: string, pathsToAdd: string[]): string => { + if (!pathsToAdd || !pathsToAdd.length) { + return currentPath + } + + const separator = getPathSeparator() + + const joinedPathsToAdd = pathsToAdd.join(separator) + + return currentPath + separator + joinedPathsToAdd + separator +} + +const mergeSpawnOptions = (originalSpawnOptions: ChildProcess.ExecOptions | ChildProcess.SpawnOptions): any => { + const requiredOptions = { + env: { + ...process.env, + ...originalSpawnOptions.env, + }, + } + + const existingPath = process.env.Path || process.env.PATH + + requiredOptions.env.PATH = mergePathEnvironmentVariable(existingPath, Config.instance().getValue("environment.additionalPaths")) + + return { + ...originalSpawnOptions, + ...requiredOptions, + } +} + +/** + * API surface area responsible for handling process-related tasks + * (spawning processes, managing running process, etc) + */ +export class Process { + + public execNodeScript(scriptPath: string, args: string[] = [], options: ChildProcess.ExecOptions = {}, callback: (err: any, stdout: string, stderr: string) => void): ChildProcess.ChildProcess { + const spawnOptions = mergeSpawnOptions(options) + spawnOptions.env.ELECTRON_RUN_AS_NODE = 1 + + const execOptions = [process.execPath, scriptPath].concat(args) + const execString = execOptions.map((s) => `"${s}"`).join(" ") + + return ChildProcess.exec(execString, spawnOptions, callback) + } + + /** + * Wrapper around `child_process.exec` to run using electron as opposed to node + */ + public spawnNodeScript(scriptPath: string, args: string[] = [], options: ChildProcess.SpawnOptions = {}): ChildProcess.ChildProcess { + const spawnOptions = mergeSpawnOptions(options) + spawnOptions.env.ELECTRON_RUN_AS_NODE = 1 + + const allArgs = [scriptPath].concat(args) + + return ChildProcess.spawn(process.execPath, allArgs, spawnOptions) + } + + /** + * Spawn process - wrapper around `child_process.spawn` + */ + public spawnProcess(startCommand: string, args: string[] = [], options: ChildProcess.SpawnOptions = {}): ChildProcess.ChildProcess { + const spawnOptions = mergeSpawnOptions(options) + + return ChildProcess.spawn(startCommand, args, spawnOptions) + } +} diff --git a/definitions/Oni.d.ts b/definitions/Oni.d.ts index c4fd5fee4b..0cd5b73633 100644 --- a/definitions/Oni.d.ts +++ b/definitions/Oni.d.ts @@ -1,3 +1,4 @@ +import * as ChildProcess from "child_process" import { EventEmitter } from "events" import * as types from "vscode-languageserver-types" @@ -30,6 +31,12 @@ declare namespace Oni { createItem(alignment: number, priority: number, globalId?: string): StatusBarItem } + export interface Process { + execNodeScript(scriptPath: string, args?: string[], options?: ChildProcess.ExecOptions, callback?: (err: any, stdout: string, stderr: string) => void): ChildProcess.ChildProcess + spawnNodeScript(scriptPath: string, args?: string[], options?: ChildProcess.SpawnOptions): ChildProcess.ChildProcess + spawnProcess(startCommand: string, args?: string[], options?: ChildProcess.SpawnOptions): ChildProcess.ChildProcess + } + export interface StatusBarItem { show(): void hide(): void @@ -150,6 +157,7 @@ declare namespace Oni { configuration: Configuration diagnostics: Diagnostics.Api editor: Editor + process: Process statusBar: StatusBar registerLanguageService(languageType: string, languageService: LanguageService) diff --git a/vim/core/oni-plugin-tslint/lib/index.js b/vim/core/oni-plugin-tslint/lib/index.js index 7ccc6294f0..e7638ff74b 100644 --- a/vim/core/oni-plugin-tslint/lib/index.js +++ b/vim/core/oni-plugin-tslint/lib/index.js @@ -112,7 +112,7 @@ const activate = (Oni) => { processArgs = processArgs.concat(["--config", configPath]) processArgs = processArgs.concat(args) - return Q.nfcall(Oni.execNodeScript, tslintPath, processArgs, { cwd: workingDirectory }) + return Q.nfcall(Oni.process.execNodeScript, tslintPath, processArgs, { cwd: workingDirectory }) .then((stdout, stderr) => { const errorOutput = stdout.join(os.EOL).trim() diff --git a/vim/core/oni-plugin-typescript/src/TypeScriptServerHost.ts b/vim/core/oni-plugin-typescript/src/TypeScriptServerHost.ts index 73d256c045..0e598b7c1a 100644 --- a/vim/core/oni-plugin-typescript/src/TypeScriptServerHost.ts +++ b/vim/core/oni-plugin-typescript/src/TypeScriptServerHost.ts @@ -39,7 +39,7 @@ export class TypeScriptServerHost extends events.EventEmitter { // This has some info on using eventPort: https://github.com/Microsoft/TypeScript/blob/master/src/server/server.ts // which might be more reliable // Can create the port using this here: https://github.com/Microsoft/TypeScript/blob/master/src/server/server.ts - this._tssProcess = Oni.spawnNodeScript(tssPath) + this._tssProcess = Oni.process.spawnNodeScript(tssPath) console.log("Process ID: " + this._tssProcess.pid) // tslint:disable-line no-console this._rl = readline.createInterface({