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

@@ -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.
 

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<IConfigValues> = {
@@ -162,6 +172,10 @@ export class Config extends EventEmitter {
     private LinuxConfig: Partial<IConfigValues> = {
         "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

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<any> {
         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"
         }

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[]) {

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)
+    }
+}

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)

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()

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({