From 6f3fcd2ce0002ffd876b7b43bff42a912edddaae Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 20 Nov 2020 11:57:19 +0100 Subject: [PATCH] env - move shell env into bootstrap-window and let user env win over shell env --- src/bootstrap-window.js | 71 ++++++++++++------- .../parts/sandbox/electron-browser/preload.js | 26 ++++--- .../parts/sandbox/electron-sandbox/globals.ts | 14 +++- .../electron-browser/workbench/workbench.js | 29 ++------ src/vs/code/electron-main/app.ts | 7 +- .../electron-sandbox/workbench/workbench.js | 27 ++----- 6 files changed, 85 insertions(+), 89 deletions(-) diff --git a/src/bootstrap-window.js b/src/bootstrap-window.js index 38867c15c4948..0981fe0baaf55 100644 --- a/src/bootstrap-window.js +++ b/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 }; })); diff --git a/src/vs/base/parts/sandbox/electron-browser/preload.js b/src/vs/base/parts/sandbox/electron-browser/preload.js index dfad98f286d54..702a28ea6528f 100644 --- a/src/vs/base/parts/sandbox/electron-browser/preload.js +++ b/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} */ - 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} */ - 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(); }); diff --git a/src/vs/base/parts/sandbox/electron-sandbox/globals.ts b/src/vs/base/parts/sandbox/electron-sandbox/globals.ts index 3bd0c203e5bd9..1116cf2135f0f 100644 --- a/src/vs/base/parts/sandbox/electron-sandbox/globals.ts +++ b/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; + resolveEnv(userEnv: IProcessEnvironment): Promise; /** * A listener on the process. Only a small subset of listener types are allowed. diff --git a/src/vs/code/electron-browser/workbench/workbench.js b/src/vs/code/electron-browser/workbench/workbench.js index e71e4a031b1cb..dc0b08b093aba 100644 --- a/src/vs/code/electron-browser/workbench/workbench.js +++ b/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() { diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index b357b99a73031..390317f43910e 100644 --- a/src/vs/code/electron-main/app.ts +++ b/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); diff --git a/src/vs/code/electron-sandbox/workbench/workbench.js b/src/vs/code/electron-sandbox/workbench/workbench.js index 3966a9d0eac02..162e40bf593e3 100644 --- a/src/vs/code/electron-sandbox/workbench/workbench.js +++ b/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() {