Skip to content

Commit

Permalink
env - move shell env into bootstrap-window and let user env win over …
Browse files Browse the repository at this point in the history
…shell env
  • Loading branch information
bpasero committed Nov 20, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 69de646 commit 6f3fcd2
Showing 6 changed files with 85 additions and 89 deletions.
71 changes: 47 additions & 24 deletions src/bootstrap-window.js
Original file line number Diff line number Diff line change
@@ -26,25 +26,18 @@
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
* @param {(result, configuration: object) => any} resultCallback
* @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
};
}));
26 changes: 17 additions & 9 deletions src/vs/base/parts/sandbox/electron-browser/preload.js
Original file line number Diff line number Diff line change
@@ -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();
});
14 changes: 11 additions & 3 deletions src/vs/base/parts/sandbox/electron-sandbox/globals.ts
Original file line number Diff line number Diff line change
@@ -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.
29 changes: 4 additions & 25 deletions src/vs/code/electron-browser/workbench/workbench.js
Original file line number Diff line number Diff line change
@@ -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() {
7 changes: 2 additions & 5 deletions src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
@@ -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,15 +278,15 @@ 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);

try {
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);
27 changes: 4 additions & 23 deletions src/vs/code/electron-sandbox/workbench/workbench.js
Original file line number Diff line number Diff line change
@@ -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() {

0 comments on commit 6f3fcd2

Please sign in to comment.