env - move shell env into bootstrap-window and let user env win over shell env

src/bootstrap-window.js

@@ -26,6 +26,11 @@
 	const sandbox = preloadGlobals.context.sandbox;
 	const webFrame = preloadGlobals.webFrame;
 	const safeProcess = sandbox ? preloadGlobals.process : process;
+	const configuration = parseWindowConfiguration();
+
+	// Start to resolve process.env before anything gets load
+	// so that we can run loading and resolving in parallel
+	const whenEnvResolved = preloadGlobals.process.resolveEnv(configuration.userEnv);
 
 	/**
 	 * @param {string[]} modulePaths
@@ -33,18 +38,6 @@
 	 * @param {{ forceEnableDeveloperKeybindings?: boolean, disallowReloadKeybinding?: boolean, removeDeveloperKeybindingsAfterLoad?: boolean, canModifyDOM?: (config: object) => void, beforeLoaderConfig?: (config: object, loaderConfig: object) => void, beforeRequire?: () => void }=} options
 	 */
 	function load(modulePaths, resultCallback, options) {
-		const args = parseURLQueryArgs();
-		/**
-		 * // configuration: INativeWindowConfiguration
-		 * @type {{
-		 * zoomLevel?: number,
-		 * extensionDevelopmentPath?: string[],
-		 * extensionTestsPath?: string,
-		 * userEnv?: { [key: string]: string | undefined },
-		 * appRoot: string,
-		 * nodeCachedDataDir?: string
-		 * }} */
-		const configuration = JSON.parse(args['config'] || '{}') || {};
 
 		// Apply zoom level early to avoid glitches
 		const zoomLevel = configuration.zoomLevel;
@@ -64,11 +57,6 @@
 			developerToolsUnbind = registerDeveloperKeybindings(options && options.disallowReloadKeybinding);
 		}
 
-		// Correctly inherit the parent's environment (TODO@sandbox non-sandboxed only)
-		if (!sandbox) {
-			Object.assign(safeProcess.env, configuration.userEnv);
-		}
-
 		// Enable ASAR support (TODO@sandbox non-sandboxed only)
 		if (!sandbox) {
 			globalThis.MonacoBootstrap.enableASARSupport(configuration.appRoot);
@@ -150,8 +138,16 @@
 			options.beforeRequire();
 		}
 
-		require(modulePaths, result => {
+		require(modulePaths, async result => {
 			try {
+
+				// Wait for process environment being fully resolved
+				const perf = perfLib();
+				perf.mark('willWaitForShellEnv');
+				await whenEnvResolved;
+				perf.mark('didWaitForShellEnv');
+
+				// Callback only after process environment is resolved
 				const callbackResult = resultCallback(result, configuration);
 				if (callbackResult && typeof callbackResult.then === 'function') {
 					callbackResult.then(() => {
@@ -169,16 +165,26 @@
 	}
 
 	/**
-	 * @returns {{[param: string]: string }}
+	 * Parses the contents of the `INativeWindowConfiguration` that
+	 * is passed into the URL from the `electron-main` side.
+	 *
+	 * @returns {{
+	 * zoomLevel?: number,
+	 * extensionDevelopmentPath?: string[],
+	 * extensionTestsPath?: string,
+	 * userEnv?: { [key: string]: string | undefined },
+	 * appRoot: string,
+	 * nodeCachedDataDir?: string
+	 * }}
 	 */
-	function parseURLQueryArgs() {
-		const search = window.location.search || '';
-
-		return search.split(/[?&]/)
+	function parseWindowConfiguration() {
+		const rawConfiguration = (window.location.search || '').split(/[?&]/)
 			.filter(function (param) { return !!param; })
 			.map(function (param) { return param.split('='); })
 			.filter(function (param) { return param.length === 2; })
 			.reduce(function (r, param) { r[param[0]] = decodeURIComponent(param[1]); return r; }, {});
+
+		return JSON.parse(rawConfiguration['config'] || '{}') || {};
 	}
 
 	/**
@@ -256,8 +262,25 @@
 		return window.vscode;
 	}
 
+	/**
+	 * @return {{ mark: (name: string) => void }}
+	 */
+	function perfLib() {
+		globalThis.MonacoPerformanceMarks = globalThis.MonacoPerformanceMarks || [];
+
+		return {
+			/**
+			 * @param {string} name
+			 */
+			mark(name) {
+				globalThis.MonacoPerformanceMarks.push(name, Date.now());
+			}
+		};
+	}
+
 	return {
 		load,
-		globals
+		globals,
+		perfLib
 	};
 }));

src/vs/base/parts/sandbox/electron-browser/preload.js

@@ -108,17 +108,18 @@
 			get type() { return 'renderer'; },
 			get execPath() { return process.execPath; },
 
-			_whenEnvResolved: undefined,
-			whenEnvResolved:
+			_resolveEnv: undefined,
+			resolveEnv:
 				/**
-				 * @returns when the shell environment has been resolved.
+				 * @param userEnv {{[key: string]: string}}
+				 * @returns {Promise<void>}
 				 */
-				function () {
-					if (!this._whenEnvResolved) {
-						this._whenEnvResolved = resolveEnv();
+				function (userEnv) {
+					if (!this._resolveEnv) {
+						this._resolveEnv = resolveEnv(userEnv);
 					}
 
-					return this._whenEnvResolved;
+					return this._resolveEnv;
 				},
 
 			getProcessMemoryInfo:
@@ -199,14 +200,21 @@
 	 * all development related environment variables. We do this from the
 	 * main process because it may involve spawning a shell.
 	 *
+	 * @param userEnv {{[key: string]: string}}
 	 * @returns {Promise<void>}
 	 */
-	function resolveEnv() {
+	function resolveEnv(userEnv) {
+
+		// Apply `userEnv` directly
+		Object.assign(process.env, userEnv);
+
+		// Resolve `shellEnv` from the main side
 		return new Promise(function (resolve) {
 			ipcRenderer.once('vscode:acceptShellEnv', function (event, shellEnv) {
 
 				// Assign all keys of the shell environment to our process environment
-				Object.assign(process.env, shellEnv);
+				// But make sure that the user environment wins in the end
+				Object.assign(process.env, shellEnv, userEnv);
 
 				resolve();
 			});

src/vs/base/parts/sandbox/electron-sandbox/globals.ts

@@ -35,10 +35,18 @@ export interface ISandboxNodeProcess extends INodeProcess {
 	readonly execPath: string;
 
 	/**
-	 * Allows to await resolving the full process environment by checking for the shell environment
-	 * of the OS in certain cases (e.g. when the app is started from the Dock on macOS).
+	 * Resolve the true process environment to use. There are different layers of environment
+	 * that will apply:
+	 * - `process.env`: this is the actual environment of the process
+	 * - `shellEnv`   : if the program was not started from a terminal, we resolve all shell
+	 *                  variables to get the same experience as if the program was started from
+	 *                  a terminal (Linux, macOS)
+	 * - `userEnv`    : this is instance specific environment, e.g. if the user started the program
+	 *                  from a terminal and changed certain variables
+	 *
+	 * The order of overwrites is `process.env` < `shellEnv` < `userEnv`.
 	 */
-	whenEnvResolved(): Promise<void>;
+	resolveEnv(userEnv: IProcessEnvironment): Promise<void>;
 
 	/**
 	 * A listener on the process. Only a small subset of listener types are allowed.

src/vs/code/electron-browser/workbench/workbench.js

@@ -9,15 +9,12 @@
 'use strict';
 
 (function () {
+	const bootstrapWindow = bootstrapWindowLib();
 
 	// Add a perf entry right from the top
-	const perf = perfLib();
+	const perf = bootstrapWindow.perfLib();
 	perf.mark('renderer/started');
 
-	// Load environment in parallel to workbench loading to avoid waterfall
-	const bootstrapWindow = bootstrapWindowLib();
-	const whenEnvResolved = bootstrapWindow.globals().process.whenEnvResolved();
-
 	// Load workbench main JS, CSS and NLS all in parallel. This is an
 	// optimization to prevent a waterfall of loading to happen, because
 	// we know for a fact that workbench.desktop.main will depend on
@@ -32,12 +29,6 @@
 			// Mark start of workbench
 			perf.mark('didLoadWorkbenchMain');
 			performance.mark('workbench-start');
-
-			// Wait for process environment being fully resolved
-			perf.mark('willWaitForShellEnv');
-			await whenEnvResolved;
-			perf.mark('didWaitForShellEnv');
-
 			perf.mark('main/startup');
 
 			// @ts-ignore
@@ -60,23 +51,11 @@
 
 	//region Helpers
 
-	function perfLib() {
-		globalThis.MonacoPerformanceMarks = globalThis.MonacoPerformanceMarks || [];
-
-		return {
-			/**
-			 * @param {string} name
-			 */
-			mark(name) {
-				globalThis.MonacoPerformanceMarks.push(name, Date.now());
-			}
-		};
-	}
-
 	/**
 	 * @returns {{
 	 *   load: (modules: string[], resultCallback: (result, configuration: object) => any, options: object) => unknown,
-	 *   globals: () => typeof import('../../../base/parts/sandbox/electron-sandbox/globals')
+	 *   globals: () => typeof import('../../../base/parts/sandbox/electron-sandbox/globals'),
+	 *   perfLib: () => { mark: (name: string) => void }
 	 * }}
 	 */
 	function bootstrapWindowLib() {

src/vs/code/electron-main/app.ts

@@ -265,9 +265,6 @@ export class CodeApplication extends Disposable {
 		ipc.on('vscode:fetchShellEnv', async (event: IpcMainEvent) => {
 			const webContents = event.sender;
 			const window = this.windowsMainService?.getWindowByWebContents(event.sender);
-			if (!window) {
-				return;
-			}
 
 			let replied = false;
 
@@ -281,7 +278,7 @@ export class CodeApplication extends Disposable {
 			}
 
 			const shellEnvTimeoutWarningHandle = setTimeout(function () {
-				window.sendWhenReady('vscode:showShellEnvTimeoutWarning');
+				window?.sendWhenReady('vscode:showShellEnvTimeoutWarning'); // notify inside window if we have one
 				acceptShellEnv({});
 			}, 10000);
 
@@ -289,7 +286,7 @@ export class CodeApplication extends Disposable {
 				const shellEnv = await getShellEnvironment(this.logService, this.environmentService);
 				acceptShellEnv(shellEnv);
 			} catch (error) {
-				window.sendWhenReady('vscode:showShellEnvError', toErrorMessage(error));
+				window?.sendWhenReady('vscode:showShellEnvError', toErrorMessage(error)); // notify inside window if we have one
 				acceptShellEnv({});
 
 				this.logService.error('Error fetching shell env', error);

src/vs/code/electron-sandbox/workbench/workbench.js

@@ -9,15 +9,12 @@
 'use strict';
 
 (function () {
+	const bootstrapWindow = bootstrapWindowLib();
 
 	// Add a perf entry right from the top
-	const perf = perfLib();
+	const perf = bootstrapWindow.perfLib();
 	perf.mark('renderer/started');
 
-	// Load environment in parallel to workbench loading to avoid waterfall
-	const bootstrapWindow = bootstrapWindowLib();
-	const whenEnvResolved = bootstrapWindow.globals().process.whenEnvResolved();
-
 	// Load workbench main JS, CSS and NLS all in parallel. This is an
 	// optimization to prevent a waterfall of loading to happen, because
 	// we know for a fact that workbench.desktop.sandbox.main will depend on
@@ -32,10 +29,6 @@
 			// Mark start of workbench
 			perf.mark('didLoadWorkbenchMain');
 			performance.mark('workbench-start');
-
-			// Wait for process environment being fully resolved
-			await whenEnvResolved;
-
 			perf.mark('main/startup');
 
 			// @ts-ignore
@@ -58,23 +51,11 @@
 
 	//region Helpers
 
-	function perfLib() {
-		globalThis.MonacoPerformanceMarks = globalThis.MonacoPerformanceMarks || [];
-
-		return {
-			/**
-			 * @param {string} name
-			 */
-			mark(name) {
-				globalThis.MonacoPerformanceMarks.push(name, Date.now());
-			}
-		};
-	}
-
 	/**
 	 * @returns {{
 	 *   load: (modules: string[], resultCallback: (result, configuration: object) => any, options: object) => unknown,
-	 *   globals: () => typeof import('../../../base/parts/sandbox/electron-sandbox/globals')
+	 *   globals: () => typeof import('../../../base/parts/sandbox/electron-sandbox/globals'),
+	 *   perfLib: () => { mark: (name: string) => void }
 	 * }}
 	 */
 	function bootstrapWindowLib() {