Skip to content

Commit cdb900a

Browse files
committed
Make preserveEnv return a new object
Modifying the object didn't feel quite right, plus this makes the code a bit more compact.
1 parent 1622fd4 commit cdb900a

File tree

5 files changed

+20
-39
lines changed

5 files changed

+20
-39
lines changed

packages/protocol/src/common/util.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,11 @@ export const isPromise = (value: any): value is Promise<any> => {
236236
* When spawning VS Code tries to preserve the environment but since it's in
237237
* the browser, it doesn't work.
238238
*/
239-
export const preserveEnv = (options?: { env?: NodeJS.ProcessEnv } | null): void => {
240-
if (options && options.env) {
241-
options.env = { ...process.env, ...options.env };
242-
}
239+
export const withEnv = <T extends { env?: NodeJS.ProcessEnv }>(options?: T): T | undefined => {
240+
return options && options.env ? {
241+
...options,
242+
env: {
243+
...process.env, ...options.env,
244+
},
245+
} : options;
243246
};

packages/protocol/src/node/modules/child_process.ts

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as cp from "child_process";
22
import { ServerProxy } from "../../common/proxy";
3-
import { preserveEnv } from "../../common/util";
3+
import { withEnv } from "../../common/util";
44
import { WritableProxy, ReadableProxy } from "./stream";
55

66
// tslint:disable completed-docs
@@ -71,21 +71,15 @@ export class ChildProcessModuleProxy {
7171
options?: { encoding?: string | null } & cp.ExecOptions | null,
7272
callback?: ((error: cp.ExecException | null, stdin: string | Buffer, stdout: string | Buffer) => void),
7373
): Promise<ChildProcessProxies> {
74-
preserveEnv(options);
75-
76-
return this.returnProxies(cp.exec(command, options, callback));
74+
return this.returnProxies(cp.exec(command, options && withEnv(options), callback));
7775
}
7876

7977
public async fork(modulePath: string, args?: string[], options?: cp.ForkOptions): Promise<ChildProcessProxies> {
80-
preserveEnv(options);
81-
82-
return this.returnProxies((this.forkProvider || cp.fork)(modulePath, args, options));
78+
return this.returnProxies((this.forkProvider || cp.fork)(modulePath, args, withEnv(options)));
8379
}
8480

8581
public async spawn(command: string, args?: string[], options?: cp.SpawnOptions): Promise<ChildProcessProxies> {
86-
preserveEnv(options);
87-
88-
return this.returnProxies(cp.spawn(command, args, options));
82+
return this.returnProxies(cp.spawn(command, args, withEnv(options)));
8983
}
9084

9185
private returnProxies(process: cp.ChildProcess): ChildProcessProxies {

packages/protocol/src/node/modules/node-pty.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { EventEmitter } from "events";
33
import * as pty from "node-pty";
44
import { ServerProxy } from "../../common/proxy";
5-
import { preserveEnv } from "../../common/util";
5+
import { withEnv } from "../../common/util";
66

77
// tslint:disable completed-docs
88

@@ -66,8 +66,6 @@ export class NodePtyProcessProxy extends ServerProxy {
6666
*/
6767
export class NodePtyModuleProxy {
6868
public async spawn(file: string, args: string[] | string, options: pty.IPtyForkOptions): Promise<NodePtyProcessProxy> {
69-
preserveEnv(options);
70-
71-
return new NodePtyProcessProxy(require("node-pty").spawn(file, args, options));
69+
return new NodePtyProcessProxy(require("node-pty").spawn(file, args, withEnv(options)));
7270
}
7371
}

packages/server/src/cli.ts

+2-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { field, logger } from "@coder/logger";
22
import { ServerMessage, SharedProcessActive } from "@coder/protocol/src/proto";
3-
import { preserveEnv } from "@coder/protocol";
3+
import { withEnv } from "@coder/protocol";
44
import { ChildProcess, fork, ForkOptions } from "child_process";
55
import { randomFillSync } from "crypto";
66
import * as fs from "fs";
@@ -175,21 +175,12 @@ const bold = (text: string | number): string | number => {
175175
}
176176

177177
if (options.installExtension) {
178-
179-
let forkOptions = {
180-
env: {
181-
VSCODE_ALLOW_IO: "true"
182-
}
183-
}
184-
185-
preserveEnv(forkOptions);
186-
187178
const fork = forkModule("vs/code/node/cli", [
188179
"--user-data-dir", dataDir,
189180
"--builtin-extensions-dir", builtInExtensionsDir,
190181
"--extensions-dir", extensionsDir,
191182
"--install-extension", options.installExtension,
192-
], forkOptions, dataDir);
183+
], withEnv({ env: { VSCODE_ALLOW_IO: "true" } }), dataDir);
193184

194185
fork.stdout.on("data", (d: Buffer) => d.toString().split("\n").forEach((l) => logger.info(l)));
195186
fork.stderr.on("data", (d: Buffer) => d.toString().split("\n").forEach((l) => logger.error(l)));

packages/server/src/vscode/sharedProcess.ts

+5-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { ParsedArgs } from "vs/platform/environment/common/environment";
77
import { Emitter } from "@coder/events/src";
88
import { retry } from "@coder/ide/src/retry";
99
import { logger, field, Level } from "@coder/logger";
10-
import { preserveEnv } from "@coder/protocol";
10+
import { withEnv } from "@coder/protocol";
1111

1212
export enum SharedProcessState {
1313
Stopped,
@@ -89,15 +89,10 @@ export class SharedProcess {
8989
this.activeProcess.kill();
9090
}
9191

92-
let forkOptions = {
93-
env: {
94-
VSCODE_ALLOW_IO: "true"
95-
}
96-
}
97-
98-
preserveEnv(forkOptions);
99-
100-
const activeProcess = forkModule("vs/code/electron-browser/sharedProcess/sharedProcessMain", [], forkOptions, this.userDataDir);
92+
const activeProcess = forkModule(
93+
"vs/code/electron-browser/sharedProcess/sharedProcessMain", [],
94+
withEnv({ env: { VSCODE_ALLOW_IO: "true" } }), this.userDataDir,
95+
);
10196
this.activeProcess = activeProcess;
10297

10398
await new Promise((resolve, reject): void => {

0 commit comments

Comments
 (0)