From cdb900aca81006b88a92e4f933349a5496b3fd06 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 29 Apr 2019 11:49:59 -0500 Subject: [PATCH] Make preserveEnv return a new object Modifying the object didn't feel quite right, plus this makes the code a bit more compact. --- packages/protocol/src/common/util.ts | 11 +++++++---- .../protocol/src/node/modules/child_process.ts | 14 ++++---------- packages/protocol/src/node/modules/node-pty.ts | 6 ++---- packages/server/src/cli.ts | 13 ++----------- packages/server/src/vscode/sharedProcess.ts | 15 +++++---------- 5 files changed, 20 insertions(+), 39 deletions(-) diff --git a/packages/protocol/src/common/util.ts b/packages/protocol/src/common/util.ts index 789bc3cdd..b21941c34 100644 --- a/packages/protocol/src/common/util.ts +++ b/packages/protocol/src/common/util.ts @@ -236,8 +236,11 @@ export const isPromise = (value: any): value is Promise => { * When spawning VS Code tries to preserve the environment but since it's in * the browser, it doesn't work. */ -export const preserveEnv = (options?: { env?: NodeJS.ProcessEnv } | null): void => { - if (options && options.env) { - options.env = { ...process.env, ...options.env }; - } +export const withEnv = (options?: T): T | undefined => { + return options && options.env ? { + ...options, + env: { + ...process.env, ...options.env, + }, + } : options; }; diff --git a/packages/protocol/src/node/modules/child_process.ts b/packages/protocol/src/node/modules/child_process.ts index 0b0c9eb03..5df9a5274 100644 --- a/packages/protocol/src/node/modules/child_process.ts +++ b/packages/protocol/src/node/modules/child_process.ts @@ -1,6 +1,6 @@ import * as cp from "child_process"; import { ServerProxy } from "../../common/proxy"; -import { preserveEnv } from "../../common/util"; +import { withEnv } from "../../common/util"; import { WritableProxy, ReadableProxy } from "./stream"; // tslint:disable completed-docs @@ -71,21 +71,15 @@ export class ChildProcessModuleProxy { options?: { encoding?: string | null } & cp.ExecOptions | null, callback?: ((error: cp.ExecException | null, stdin: string | Buffer, stdout: string | Buffer) => void), ): Promise { - preserveEnv(options); - - return this.returnProxies(cp.exec(command, options, callback)); + return this.returnProxies(cp.exec(command, options && withEnv(options), callback)); } public async fork(modulePath: string, args?: string[], options?: cp.ForkOptions): Promise { - preserveEnv(options); - - return this.returnProxies((this.forkProvider || cp.fork)(modulePath, args, options)); + return this.returnProxies((this.forkProvider || cp.fork)(modulePath, args, withEnv(options))); } public async spawn(command: string, args?: string[], options?: cp.SpawnOptions): Promise { - preserveEnv(options); - - return this.returnProxies(cp.spawn(command, args, options)); + return this.returnProxies(cp.spawn(command, args, withEnv(options))); } private returnProxies(process: cp.ChildProcess): ChildProcessProxies { diff --git a/packages/protocol/src/node/modules/node-pty.ts b/packages/protocol/src/node/modules/node-pty.ts index c5f2581ce..c6786d416 100644 --- a/packages/protocol/src/node/modules/node-pty.ts +++ b/packages/protocol/src/node/modules/node-pty.ts @@ -2,7 +2,7 @@ import { EventEmitter } from "events"; import * as pty from "node-pty"; import { ServerProxy } from "../../common/proxy"; -import { preserveEnv } from "../../common/util"; +import { withEnv } from "../../common/util"; // tslint:disable completed-docs @@ -66,8 +66,6 @@ export class NodePtyProcessProxy extends ServerProxy { */ export class NodePtyModuleProxy { public async spawn(file: string, args: string[] | string, options: pty.IPtyForkOptions): Promise { - preserveEnv(options); - - return new NodePtyProcessProxy(require("node-pty").spawn(file, args, options)); + return new NodePtyProcessProxy(require("node-pty").spawn(file, args, withEnv(options))); } } diff --git a/packages/server/src/cli.ts b/packages/server/src/cli.ts index 13bbd7bc1..54a98b962 100644 --- a/packages/server/src/cli.ts +++ b/packages/server/src/cli.ts @@ -1,6 +1,6 @@ import { field, logger } from "@coder/logger"; import { ServerMessage, SharedProcessActive } from "@coder/protocol/src/proto"; -import { preserveEnv } from "@coder/protocol"; +import { withEnv } from "@coder/protocol"; import { ChildProcess, fork, ForkOptions } from "child_process"; import { randomFillSync } from "crypto"; import * as fs from "fs"; @@ -175,21 +175,12 @@ const bold = (text: string | number): string | number => { } if (options.installExtension) { - - let forkOptions = { - env: { - VSCODE_ALLOW_IO: "true" - } - } - - preserveEnv(forkOptions); - const fork = forkModule("vs/code/node/cli", [ "--user-data-dir", dataDir, "--builtin-extensions-dir", builtInExtensionsDir, "--extensions-dir", extensionsDir, "--install-extension", options.installExtension, - ], forkOptions, dataDir); + ], withEnv({ env: { VSCODE_ALLOW_IO: "true" } }), dataDir); fork.stdout.on("data", (d: Buffer) => d.toString().split("\n").forEach((l) => logger.info(l))); fork.stderr.on("data", (d: Buffer) => d.toString().split("\n").forEach((l) => logger.error(l))); diff --git a/packages/server/src/vscode/sharedProcess.ts b/packages/server/src/vscode/sharedProcess.ts index 5c10318cb..1348f1d85 100644 --- a/packages/server/src/vscode/sharedProcess.ts +++ b/packages/server/src/vscode/sharedProcess.ts @@ -7,7 +7,7 @@ import { ParsedArgs } from "vs/platform/environment/common/environment"; import { Emitter } from "@coder/events/src"; import { retry } from "@coder/ide/src/retry"; import { logger, field, Level } from "@coder/logger"; -import { preserveEnv } from "@coder/protocol"; +import { withEnv } from "@coder/protocol"; export enum SharedProcessState { Stopped, @@ -89,15 +89,10 @@ export class SharedProcess { this.activeProcess.kill(); } - let forkOptions = { - env: { - VSCODE_ALLOW_IO: "true" - } - } - - preserveEnv(forkOptions); - - const activeProcess = forkModule("vs/code/electron-browser/sharedProcess/sharedProcessMain", [], forkOptions, this.userDataDir); + const activeProcess = forkModule( + "vs/code/electron-browser/sharedProcess/sharedProcessMain", [], + withEnv({ env: { VSCODE_ALLOW_IO: "true" } }), this.userDataDir, + ); this.activeProcess = activeProcess; await new Promise((resolve, reject): void => {