Skip to content

Commit 384ad08

Browse files
authored
shell env - actually revert to how it used to be (microsoft#252755)
1 parent 9bf5ea5 commit 384ad08

File tree

4 files changed

+84
-175
lines changed

4 files changed

+84
-175
lines changed

src/vs/base/node/processes.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ async function fileExistsDefault(path: string): Promise<boolean> {
8383
return false;
8484
}
8585

86-
export function getWindowPathExtensions(env = processCommon.env) {
87-
return (getCaseInsensitive(env, 'PATHEXT') as string || '.COM;.EXE;.BAT;.CMD').split(';');
88-
}
89-
9086
export async function findExecutable(command: string, cwd?: string, paths?: string[], env: Platform.IProcessEnvironment = processCommon.env as Platform.IProcessEnvironment, fileExists: (path: string) => Promise<boolean> = fileExistsDefault): Promise<string | undefined> {
9187
// If we have an absolute path then we take it.
9288
if (path.isAbsolute(command)) {
@@ -123,7 +119,8 @@ export async function findExecutable(command: string, cwd?: string, paths?: stri
123119
fullPath = path.join(cwd, pathEntry, command);
124120
}
125121
if (Platform.isWindows) {
126-
const pathExtsFound = getWindowPathExtensions(env).map(async ext => {
122+
const pathExt = getCaseInsensitive(env, 'PATHEXT') as string || '.COM;.EXE;.BAT;.CMD';
123+
const pathExtsFound = pathExt.split(';').map(async ext => {
127124
const withExtension = fullPath + ext;
128125
return await fileExists(withExtension) ? withExtension : undefined;
129126
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export interface ISandboxNodeProcess extends INodeProcess {
7777
* - `process.env`: this is the actual environment of the process before this method
7878
* - `shellEnv` : if the program was not started from a terminal, we resolve all shell
7979
* variables to get the same experience as if the program was started from
80-
* a terminal
80+
* a terminal (Linux, macOS)
8181
* - `userEnv` : this is instance specific environment, e.g. if the user started the program
8282
* from a terminal and changed certain variables
8383
*

src/vs/platform/shell/node/shellEnv.ts

Lines changed: 80 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,22 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { spawn } from 'child_process';
7-
import { homedir } from 'os';
8-
import { first, Promises } from '../../../base/common/async.js';
7+
import { basename } from '../../../base/common/path.js';
8+
import { localize } from '../../../nls.js';
99
import { CancellationToken, CancellationTokenSource } from '../../../base/common/cancellation.js';
1010
import { toErrorMessage } from '../../../base/common/errorMessage.js';
1111
import { CancellationError, isCancellationError } from '../../../base/common/errors.js';
12-
import { clamp } from '../../../base/common/numbers.js';
13-
import { basename, join } from '../../../base/common/path.js';
14-
import { IProcessEnvironment, isMacintosh, isWindows, OS } from '../../../base/common/platform.js';
12+
import { IProcessEnvironment, isWindows, OS } from '../../../base/common/platform.js';
1513
import { generateUuid } from '../../../base/common/uuid.js';
16-
import { StreamSplitter } from '../../../base/node/nodeStreams.js';
17-
import { Promises as FSPromises } from '../../../base/node/pfs.js';
1814
import { getSystemShell } from '../../../base/node/shell.js';
19-
import { localize } from '../../../nls.js';
20-
import { IConfigurationService } from '../../configuration/common/configuration.js';
2115
import { NativeParsedArgs } from '../../environment/common/argv.js';
2216
import { isLaunchedFromCli } from '../../environment/node/argvHelper.js';
2317
import { ILogService } from '../../log/common/log.js';
18+
import { Promises } from '../../../base/common/async.js';
19+
import { IConfigurationService } from '../../configuration/common/configuration.js';
20+
import { clamp } from '../../../base/common/numbers.js';
2421

25-
let shellEnvPromise: Promise<typeof process.env> | undefined = undefined;
22+
let unixShellEnvPromise: Promise<typeof process.env> | undefined = undefined;
2623

2724
/**
2825
* Resolves the shell environment by spawning a shell. This call will cache
@@ -34,16 +31,16 @@ let shellEnvPromise: Promise<typeof process.env> | undefined = undefined;
3431
*/
3532
export async function getResolvedShellEnv(configurationService: IConfigurationService, logService: ILogService, args: NativeParsedArgs, env: IProcessEnvironment): Promise<typeof process.env> {
3633

37-
// Skip on windows
38-
if (isWindows) {
39-
logService.trace('resolveShellEnv(): skipped (Windows)');
34+
// Skip if --force-disable-user-env
35+
if (args['force-disable-user-env']) {
36+
logService.trace('resolveShellEnv(): skipped (--force-disable-user-env)');
4037

4138
return {};
4239
}
4340

44-
// Skip if --force-disable-user-env
45-
if (args['force-disable-user-env']) {
46-
logService.trace('resolveShellEnv(): skipped (--force-disable-user-env)');
41+
// Skip on windows
42+
else if (isWindows) {
43+
logService.trace('resolveShellEnv(): skipped (Windows)');
4744

4845
return {};
4946
}
@@ -55,19 +52,19 @@ export async function getResolvedShellEnv(configurationService: IConfigurationSe
5552
return {};
5653
}
5754

58-
// Otherwise resolve
55+
// Otherwise resolve (macOS, Linux)
5956
else {
6057
if (isLaunchedFromCli(env)) {
6158
logService.trace('resolveShellEnv(): running (--force-user-env)');
6259
} else {
63-
logService.trace('resolveShellEnv(): running');
60+
logService.trace('resolveShellEnv(): running (macOS/Linux)');
6461
}
6562

6663
// Call this only once and cache the promise for
6764
// subsequent calls since this operation can be
6865
// expensive (spawns a process).
69-
if (!shellEnvPromise) {
70-
shellEnvPromise = Promises.withAsyncBody<NodeJS.ProcessEnv>(async (resolve, reject) => {
66+
if (!unixShellEnvPromise) {
67+
unixShellEnvPromise = Promises.withAsyncBody<NodeJS.ProcessEnv>(async (resolve, reject) => {
7168
const cts = new CancellationTokenSource();
7269

7370
let timeoutValue = 10000; // default to 10 seconds
@@ -84,7 +81,7 @@ export async function getResolvedShellEnv(configurationService: IConfigurationSe
8481

8582
// Resolve shell env and handle errors
8683
try {
87-
resolve(await doResolveShellEnv(logService, cts.token));
84+
resolve(await doResolveUnixShellEnv(logService, cts.token));
8885
} catch (error) {
8986
if (!isCancellationError(error) && !cts.token.isCancellationRequested) {
9087
reject(new Error(localize('resolveShellEnvError', "Unable to resolve your shell environment: {0}", toErrorMessage(error))));
@@ -98,94 +95,104 @@ export async function getResolvedShellEnv(configurationService: IConfigurationSe
9895
});
9996
}
10097

101-
return shellEnvPromise;
98+
return unixShellEnvPromise;
10299
}
103100
}
104101

105-
async function doResolveShellEnv(logService: ILogService, token: CancellationToken): Promise<typeof process.env> {
102+
async function doResolveUnixShellEnv(logService: ILogService, token: CancellationToken): Promise<typeof process.env> {
106103
const runAsNode = process.env['ELECTRON_RUN_AS_NODE'];
107-
logService.trace('doResolveShellEnv#runAsNode', runAsNode);
104+
logService.trace('getUnixShellEnvironment#runAsNode', runAsNode);
108105

109106
const noAttach = process.env['ELECTRON_NO_ATTACH_CONSOLE'];
110-
logService.trace('doResolveShellEnv#noAttach', noAttach);
107+
logService.trace('getUnixShellEnvironment#noAttach', noAttach);
111108

112109
const mark = generateUuid().replace(/-/g, '').substr(0, 12);
110+
const regex = new RegExp(mark + '({.*})' + mark);
111+
113112
const env = {
114113
...process.env,
115114
ELECTRON_RUN_AS_NODE: '1',
116115
ELECTRON_NO_ATTACH_CONSOLE: '1',
117116
VSCODE_RESOLVING_ENVIRONMENT: '1'
118117
};
119118

120-
logService.trace('doResolveShellEnv#env', env);
121-
const systemShell = await getSystemShell(OS, env);
122-
logService.trace('doResolveShellEnv#shell', systemShell);
123-
124-
const name = basename(systemShell);
125-
let command: string, shellArgs: Array<string>;
126-
if (/^(?:pwsh|powershell)(?:-preview)?$/.test(name)) {
127-
const profilePaths = getPowershellProfilePaths();
128-
const profilePathThatExists = await first(profilePaths.map(profilePath => async () => (await FSPromises.exists(profilePath)) ? profilePath : undefined));
129-
if (!profilePathThatExists) {
130-
logService.trace('doResolveShellEnv#noPowershellProfile after testing paths', profilePaths);
119+
logService.trace('getUnixShellEnvironment#env', env);
120+
const systemShellUnix = await getSystemShell(OS, env);
121+
logService.trace('getUnixShellEnvironment#shell', systemShellUnix);
131122

132-
return {};
123+
return new Promise<typeof process.env>((resolve, reject) => {
124+
if (token.isCancellationRequested) {
125+
return reject(new CancellationError());
133126
}
134127

135-
logService.trace('doResolveShellEnv#powershellProfile found in', profilePathThatExists);
136-
137-
// Older versions of PowerShell removes double quotes sometimes
138-
// so we use "double single quotes" which is how you escape single
139-
// quotes inside of a single quoted string.
140-
command = `Write-Output '${mark}'; [System.Environment]::GetEnvironmentVariables() | ConvertTo-Json -Compress; Write-Output '${mark}'`;
141-
shellArgs = ['-Login', '-Command'];
142-
} else if (name === 'nu') { // nushell requires ^ before quoted path to treat it as a command
143-
command = `^'${process.execPath}' -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
144-
shellArgs = ['-i', '-l', '-c'];
145-
} else if (name === 'xonsh') { // #200374: native implementation is shorter
146-
command = `import os, json; print("${mark}", json.dumps(dict(os.environ)), "${mark}")`;
147-
shellArgs = ['-i', '-l', '-c'];
148-
} else {
149-
command = `'${process.execPath}' -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
150-
151-
if (name === 'tcsh' || name === 'csh') {
152-
shellArgs = ['-ic'];
153-
} else {
128+
// handle popular non-POSIX shells
129+
const name = basename(systemShellUnix);
130+
let command: string, shellArgs: Array<string>;
131+
const extraArgs = '';
132+
if (/^(?:pwsh|powershell)(?:-preview)?$/.test(name)) {
133+
// Older versions of PowerShell removes double quotes sometimes so we use "double single quotes" which is how
134+
// you escape single quotes inside of a single quoted string.
135+
command = `& '${process.execPath}' ${extraArgs} -p '''${mark}'' + JSON.stringify(process.env) + ''${mark}'''`;
136+
shellArgs = ['-Login', '-Command'];
137+
} else if (name === 'nu') { // nushell requires ^ before quoted path to treat it as a command
138+
command = `^'${process.execPath}' ${extraArgs} -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
154139
shellArgs = ['-i', '-l', '-c'];
155-
}
156-
}
140+
} else if (name === 'xonsh') { // #200374: native implementation is shorter
141+
command = `import os, json; print("${mark}", json.dumps(dict(os.environ)), "${mark}")`;
142+
shellArgs = ['-i', '-l', '-c'];
143+
} else {
144+
command = `'${process.execPath}' ${extraArgs} -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
157145

158-
return new Promise<typeof process.env>((resolve, reject) => {
159-
if (token.isCancellationRequested) {
160-
return reject(new CancellationError());
146+
if (name === 'tcsh' || name === 'csh') {
147+
shellArgs = ['-ic'];
148+
} else {
149+
shellArgs = ['-i', '-l', '-c'];
150+
}
161151
}
162152

163-
logService.trace('doResolveShellEnv#spawn', JSON.stringify(shellArgs), command);
153+
logService.trace('getUnixShellEnvironment#spawn', JSON.stringify(shellArgs), command);
164154

165-
const child = spawn(systemShell, [...shellArgs, command], {
166-
detached: !isWindows,
155+
const child = spawn(systemShellUnix, [...shellArgs, command], {
156+
detached: true,
167157
stdio: ['ignore', 'pipe', 'pipe'],
168158
env
169159
});
170160

171161
token.onCancellationRequested(() => {
172-
logService.error('doResolveShellEnv#timeout', 'Shell environment resolution timed out, buffers so far:');
173-
logService.error('doResolveShellEnv#stdout', Buffer.concat(buffers).toString('utf8') || '<empty>');
174-
logService.error('doResolveShellEnv#stderr', Buffer.concat(stderr).toString('utf8') || '<empty>');
175162
child.kill();
176163

177164
return reject(new CancellationError());
178165
});
179166

180167
child.on('error', err => {
181-
logService.error('doResolveShellEnv#errorChildProcess', toErrorMessage(err));
168+
logService.error('getUnixShellEnvironment#errorChildProcess', toErrorMessage(err));
182169
reject(err);
183170
});
184171

185-
let didResolve = false;
186-
function tryParseEnvironment(data: string) {
172+
const buffers: Buffer[] = [];
173+
child.stdout.on('data', b => buffers.push(b));
174+
175+
const stderr: Buffer[] = [];
176+
child.stderr.on('data', b => stderr.push(b));
177+
178+
child.on('close', (code, signal) => {
179+
const raw = Buffer.concat(buffers).toString('utf8');
180+
logService.trace('getUnixShellEnvironment#raw', raw);
181+
182+
const stderrStr = Buffer.concat(stderr).toString('utf8');
183+
if (stderrStr.trim()) {
184+
logService.trace('getUnixShellEnvironment#stderr', stderrStr);
185+
}
186+
187+
if (code || signal) {
188+
return reject(new Error(localize('resolveShellEnvExitError', "Unexpected exit code from spawned shell (code {0}, signal {1})", code, signal)));
189+
}
190+
191+
const match = regex.exec(raw);
192+
const rawStripped = match ? match[1] : '{}';
193+
187194
try {
188-
const env = JSON.parse(data);
195+
const env = JSON.parse(rawStripped);
189196

190197
if (runAsNode) {
191198
env['ELECTRON_RUN_AS_NODE'] = runAsNode;
@@ -204,108 +211,12 @@ async function doResolveShellEnv(logService: ILogService, token: CancellationTok
204211
// https://github.com/microsoft/vscode/issues/22593#issuecomment-336050758
205212
delete env['XDG_RUNTIME_DIR'];
206213

207-
logService.trace('doResolveShellEnv#result', env);
208-
didResolve = true;
214+
logService.trace('getUnixShellEnvironment#result', env);
209215
resolve(env);
210216
} catch (err) {
211-
logService.error('doResolveShellEnv#errorCaught', toErrorMessage(err));
217+
logService.error('getUnixShellEnvironment#errorCaught', toErrorMessage(err));
212218
reject(err);
213219
}
214-
}
215-
216-
const buffers: Buffer[] = [];
217-
let accumulator: string | undefined;
218-
219-
child.stdout
220-
.on('data', d => buffers.push(d))
221-
.pipe(new StreamSplitter(mark))
222-
.on('data', (data: Buffer) => {
223-
if (accumulator === undefined || didResolve) {
224-
// The first chunk will be the data leading up to the opening mark.
225-
// Ignore that by only setting the accumulator once we see it, and
226-
// also ignore any further data if we already resolved.
227-
accumulator = '';
228-
return;
229-
}
230-
231-
accumulator += data.toString('utf8').trim();
232-
// Wait to start accumulating until we see the start of the JSON
233-
// object to avoid issues with `ps` in profile scripts (#251650)
234-
if (!accumulator.startsWith('{')) {
235-
accumulator = '';
236-
return;
237-
}
238-
239-
logService.trace('doResolveShellEnv#tryEagerParse', accumulator);
240-
tryParseEnvironment(accumulator.slice(0, -mark.length));
241-
if (didResolve) {
242-
child.kill();
243-
}
244-
});
245-
246-
child.stdout.on('data', b => buffers.push(b));
247-
248-
const stderr: Buffer[] = [];
249-
child.stderr.on('data', b => stderr.push(b));
250-
251-
child.on('close', (code, signal) => {
252-
if (didResolve) {
253-
return;
254-
}
255-
256-
// Although we try to parse the environment eagerly, we still check one
257-
// more time when the process closes in case the data was oddly written
258-
// to stderr instead of stdout, and so we can do final debug logging as needed.
259-
const raw = Buffer.concat(buffers).toString('utf8');
260-
logService.trace('doResolveShellEnv#raw', raw);
261-
262-
const stderrStr = Buffer.concat(stderr).toString('utf8');
263-
if (stderrStr.trim()) {
264-
logService.trace('doResolveShellEnv#stderr', stderrStr);
265-
}
266-
267-
if (code || signal) {
268-
return reject(new Error(localize('resolveShellEnvExitError', "Unexpected exit code from spawned shell (code {0}, signal {1})", code, signal)));
269-
}
270-
271-
const startIndex = raw.indexOf(mark);
272-
const endIndex = raw.lastIndexOf(mark);
273-
const rawStripped = startIndex !== -1 && endIndex !== -1 && startIndex < endIndex ? raw.substring(startIndex + mark.length, endIndex).trim() : '{}';
274-
tryParseEnvironment(rawStripped);
275220
});
276221
});
277222
}
278-
279-
/**
280-
* Returns powershell profile paths that are used to source its environment.
281-
* This is used to determine whether we should resolve a powershell environment,
282-
* potentially saving us from spawning a powershell process.
283-
*
284-
* @see https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_profiles?view=powershell-7.5
285-
*/
286-
function getPowershellProfilePaths() {
287-
const paths: string[] = [];
288-
const userHome = homedir();
289-
290-
if (isMacintosh) {
291-
292-
// note: powershell 7 is the first (and yet only) powershell version on posix,
293-
// so no need to look for any extra paths yet.
294-
295-
paths.push(
296-
'/usr/local/microsoft/powershell/7/profile.ps1', // All Users, All Hosts
297-
'/usr/local/microsoft/powershell/7/Microsoft.PowerShell_profile.ps1', // All Users, Current Host
298-
join(userHome, '.config', 'powershell', 'profile.ps1'), // Current User, All Hosts
299-
join(userHome, '.config', 'powershell', 'Microsoft.PowerShell_profile.ps1'), // Current User, Current Host
300-
);
301-
} else {
302-
paths.push(
303-
'/opt/microsoft/powershell/7/profile.ps1', // All Users, All Hosts
304-
'/opt/microsoft/powershell/7/Microsoft.PowerShell_profile.ps1', // All Users, Current Host
305-
join(userHome, '.config', 'powershell', 'profile.ps1'), // Current User, All Hosts
306-
join(userHome, '.config', 'powershell', 'Microsoft.PowerShell_profile.ps1'), // Current User, Current Host
307-
);
308-
}
309-
310-
return paths;
311-
}

src/vs/workbench/electron-browser/desktop.contribution.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ import { registerWorkbenchContribution2, WorkbenchPhase } from '../common/contri
145145
'default': 10,
146146
'minimum': 1,
147147
'maximum': 120,
148+
'included': !isWindows,
148149
'scope': ConfigurationScope.APPLICATION,
149150
'markdownDescription': localize('application.shellEnvironmentResolutionTimeout', "Controls the timeout in seconds before giving up resolving the shell environment when the application is not already launched from a terminal. See our [documentation](https://go.microsoft.com/fwlink/?linkid=2149667) for more information.")
150151
}

0 commit comments

Comments
 (0)