Skip to content

Commit 90aced5

Browse files
TyriarTyriar
authored
Merge pull request #171066 from microsoft/tyriar/99878
Apply env var collection path prefixes in shell integration scripts when mac+login shell
2 parents b8e6961 + 04799e1 commit 90aced5

22 files changed

+143
-92
lines changed

‎src/vs/platform/terminal/common/environmentVariable.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { IProcessEnvironment } from 'vs/base/common/platform';
7+
68
export enum EnvironmentVariableMutatorType {
79
Replace = 1,
810
Append = 2,
@@ -12,8 +14,47 @@ export interface IEnvironmentVariableMutator {
1214
readonly value: string;
1315
readonly type: EnvironmentVariableMutatorType;
1416
}
17+
18+
export interface IEnvironmentVariableCollection {
19+
readonly map: ReadonlyMap<string, IEnvironmentVariableMutator>;
20+
}
21+
1522
/** [variable, mutator] */
1623
export type ISerializableEnvironmentVariableCollection = [string, IEnvironmentVariableMutator][];
1724

1825
/** [extension, collection] */
1926
export type ISerializableEnvironmentVariableCollections = [string, ISerializableEnvironmentVariableCollection][];
27+
28+
export interface IExtensionOwnedEnvironmentVariableMutator extends IEnvironmentVariableMutator {
29+
readonly extensionIdentifier: string;
30+
}
31+
32+
export interface IMergedEnvironmentVariableCollectionDiff {
33+
added: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
34+
changed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
35+
removed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
36+
}
37+
38+
type VariableResolver = (str: string) => Promise<string>;
39+
40+
/**
41+
* Represents an environment variable collection that results from merging several collections
42+
* together.
43+
*/
44+
export interface IMergedEnvironmentVariableCollection {
45+
readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>;
46+
readonly map: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
47+
48+
/**
49+
* Applies this collection to a process environment.
50+
* @param variableResolver An optional function to use to resolve variables within the
51+
* environment values.
52+
*/
53+
applyToProcessEnvironment(env: IProcessEnvironment, variableResolver?: VariableResolver): Promise<void>;
54+
55+
/**
56+
* Generates a diff of this collection against another. Returns undefined if the collections are
57+
* the same.
58+
*/
59+
diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined;
60+
}

‎src/vs/workbench/contrib/terminal/common/environmentVariableCollection.ts‎ renamed to ‎src/vs/platform/terminal/common/environmentVariableCollection.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
7-
import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/workbench/contrib/terminal/common/environmentVariable';
8-
import { VariableResolver } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
7+
import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';
8+
9+
type VariableResolver = (str: string) => Promise<string>;
910

1011
export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
1112
readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

‎src/vs/workbench/contrib/terminal/common/environmentVariableShared.ts‎ renamed to ‎src/vs/platform/terminal/common/environmentVariableShared.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { IEnvironmentVariableCollection, IEnvironmentVariableMutator, ISerializableEnvironmentVariableCollection, ISerializableEnvironmentVariableCollections } from 'vs/workbench/contrib/terminal/common/environmentVariable';
6+
import { IEnvironmentVariableCollection, IEnvironmentVariableMutator, ISerializableEnvironmentVariableCollection, ISerializableEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariable';
77

88
// This file is shared between the renderer and extension host
99

‎src/vs/platform/terminal/node/terminalEnvironment.ts‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ import * as os from 'os';
77
import { FileAccess } from 'vs/base/common/network';
88
import { getCaseInsensitive } from 'vs/base/common/objects';
99
import * as path from 'vs/base/common/path';
10-
import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
10+
import { IProcessEnvironment, isMacintosh, isWindows } from 'vs/base/common/platform';
1111
import * as process from 'vs/base/common/process';
1212
import { format } from 'vs/base/common/strings';
1313
import { isString } from 'vs/base/common/types';
1414
import * as pfs from 'vs/base/node/pfs';
1515
import { ILogService } from 'vs/platform/log/common/log';
1616
import { IProductService } from 'vs/platform/product/common/productService';
1717
import { IShellLaunchConfig, ITerminalEnvironment, ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
18+
import { EnvironmentVariableMutatorType } from 'vs/platform/terminal/common/environmentVariable';
19+
import { deserializeEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariableShared';
20+
import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';
1821

1922
export function getWindowsBuildNumber(): number {
2023
const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release());
@@ -104,7 +107,7 @@ export interface IShellIntegrationConfigInjection {
104107
*/
105108
export function getShellIntegrationInjection(
106109
shellLaunchConfig: IShellLaunchConfig,
107-
options: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'>,
110+
options: ITerminalProcessOptions,
108111
env: ITerminalEnvironment | undefined,
109112
logService: ILogService,
110113
productService: IProductService
@@ -151,6 +154,7 @@ export function getShellIntegrationInjection(
151154
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
152155
} else if (areZshBashLoginArgs(originalArgs)) {
153156
envMixin['VSCODE_SHELL_LOGIN'] = '1';
157+
addEnvMixinPathPrefix(options, envMixin);
154158
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
155159
}
156160
if (!newArgs) {
@@ -166,6 +170,7 @@ export function getShellIntegrationInjection(
166170
const oldDataDirs = env?.XDG_DATA_DIRS ?? '/usr/local/share:/usr/share';
167171
const newDataDir = path.join(appRoot, 'out/vs/workbench/contrib/xdg_data');
168172
envMixin['XDG_DATA_DIRS'] = `${oldDataDirs}:${newDataDir}`;
173+
addEnvMixinPathPrefix(options, envMixin);
169174
return { newArgs: undefined, envMixin };
170175
}
171176
case 'pwsh': {
@@ -186,6 +191,7 @@ export function getShellIntegrationInjection(
186191
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh);
187192
} else if (areZshBashLoginArgs(originalArgs)) {
188193
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin);
194+
addEnvMixinPathPrefix(options, envMixin);
189195
} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin)) {
190196
newArgs = originalArgs;
191197
}
@@ -230,6 +236,39 @@ export function getShellIntegrationInjection(
230236
return undefined;
231237
}
232238

239+
/**
240+
* On macOS the profile calls path_helper which adds a bunch of standard bin directories to the
241+
* beginning of the PATH. This causes significant problems for the environment variable
242+
* collection API as the custom paths added to the end will now be somewhere in the middle of
243+
* the PATH. To combat this, VSCODE_PATH_PREFIX is used to re-apply any prefix after the profile
244+
* has run. This will cause duplication in the PATH but should fix the issue.
245+
*
246+
* See #99878 for more information.
247+
*/
248+
function addEnvMixinPathPrefix(options: ITerminalProcessOptions, envMixin: IProcessEnvironment): void {
249+
if (isMacintosh && options.environmentVariableCollections) {
250+
// Deserialize and merge
251+
const deserialized = deserializeEnvironmentVariableCollections(options.environmentVariableCollections);
252+
const merged = new MergedEnvironmentVariableCollection(deserialized);
253+
254+
// Get all prepend PATH entries
255+
const pathEntry = merged.map.get('PATH');
256+
const prependToPath: string[] = [];
257+
if (pathEntry) {
258+
for (const mutator of pathEntry) {
259+
if (mutator.type === EnvironmentVariableMutatorType.Prepend) {
260+
prependToPath.push(mutator.value);
261+
}
262+
}
263+
}
264+
265+
// Add to the environment mixin to be applied in the shell integration script
266+
if (prependToPath.length > 0) {
267+
envMixin['VSCODE_PATH_PREFIX'] = prependToPath.join('');
268+
}
269+
}
270+
}
271+
233272
enum ShellIntegrationExecutable {
234273
WindowsPwsh = 'windows-pwsh',
235274
WindowsPwshLogin = 'windows-pwsh-login',

‎src/vs/platform/terminal/node/terminalProcess.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess
202202

203203
let injection: IShellIntegrationConfigInjection | undefined;
204204
if (this._options.shellIntegration.enabled) {
205-
injection = getShellIntegrationInjection(this.shellLaunchConfig, { shellIntegration: this._options.shellIntegration, windowsEnableConpty: this._options.windowsEnableConpty }, this._ptyOptions.env, this._logService, this._productService);
205+
injection = getShellIntegrationInjection(this.shellLaunchConfig, this._options, this._ptyOptions.env, this._logService, this._productService);
206206
if (injection) {
207207
this._onDidChangeProperty.fire({ type: ProcessPropertyType.UsedShellIntegrationInjection, value: true });
208208
if (injection.envMixin) {

‎src/vs/platform/terminal/test/node/terminalEnvironment.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import { IProductService } from 'vs/platform/product/common/productService';
1111
import { ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
1212
import { getShellIntegrationInjection, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';
1313

14-
const enabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: true };
15-
const disabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: false }, windowsEnableConpty: true };
16-
const winptyProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: false };
14+
const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: true, environmentVariableCollections: undefined };
15+
const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined };
16+
const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: false, environmentVariableCollections: undefined };
1717
const pwshExe = process.platform === 'win32' ? 'pwsh.exe' : 'pwsh';
1818
const repoRoot = process.platform === 'win32' ? process.cwd()[0].toLowerCase() + process.cwd().substring(1) : process.cwd();
1919
const logService = new NullLogService();

‎src/vs/server/node/remoteTerminalChannel.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import { IGetTerminalLayoutInfoArgs, ISetTerminalLayoutInfoArgs } from 'vs/platf
2020
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
2121
import { createURITransformer } from 'vs/workbench/api/node/uriTransformer';
2222
import { CLIServerBase, ICommandsExecuter } from 'vs/workbench/api/node/extHostCLIServer';
23-
import { IEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
24-
import { MergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableCollection';
25-
import { deserializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
23+
import { IEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
24+
import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';
25+
import { deserializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
2626
import { ICreateTerminalProcessArguments, ICreateTerminalProcessResult, IWorkspaceFolderData } from 'vs/workbench/contrib/terminal/common/remoteTerminalChannel';
2727
import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
2828
import { AbstractVariableResolverService } from 'vs/workbench/services/configurationResolver/common/variableResolver';

‎src/vs/workbench/api/browser/mainThreadTerminalService.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { IProcessProperty, IShellLaunchConfig, IShellLaunchConfigDto, ProcessPro
1414
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
1515
import { ITerminalEditorService, ITerminalExternalLinkProvider, ITerminalGroupService, ITerminalInstance, ITerminalLink, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
1616
import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy';
17-
import { IEnvironmentVariableService, ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
18-
import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
17+
import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
18+
import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
1919
import { IStartExtensionTerminalRequest, ITerminalProcessExtHostProxy, ITerminalProfileResolverService, ITerminalProfileService, ITerminalQuickFixService } from 'vs/workbench/contrib/terminal/common/terminal';
2020
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
2121
import { withNullAsUndefined } from 'vs/base/common/types';
@@ -26,6 +26,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
2626
import { ITerminalCommand } from 'vs/platform/terminal/common/capabilities/capabilities';
2727
import { ITerminalOutputMatch, ITerminalOutputMatcher, ITerminalQuickFix, ITerminalQuickFixOptions } from 'vs/platform/terminal/common/xterm/terminalQuickFix';
2828
import { TerminalQuickFixType } from 'vs/workbench/api/common/extHostTypes';
29+
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
2930

3031

3132
@extHostNamedCustomer(MainContext.MainThreadTerminalService)

‎src/vs/workbench/api/common/extHost.protocol.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
5555
import { OutputChannelUpdateMode } from 'vs/workbench/services/output/common/output';
5656
import { InputValidationType } from 'vs/workbench/contrib/scm/common/scm';
5757
import { IWorkspaceSymbol } from 'vs/workbench/contrib/search/common/search';
58-
import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
58+
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
5959
import { CoverageDetails, ExtensionRunTestsRequest, ICallProfileRunHandler, IFileCoverage, ISerializedTestResults, ITestItem, ITestMessage, ITestRunProfile, ITestRunTask, ResolvedTestRunRequest, IStartControllerTests, TestResultState, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testTypes';
6060
import { Timeline, TimelineChangeEvent, TimelineOptions, TimelineProviderDescriptor } from 'vs/workbench/contrib/timeline/common/timeline';
6161
import { TypeHierarchyItem } from 'vs/workbench/contrib/typeHierarchy/common/typeHierarchy';

‎src/vs/workbench/api/common/extHostTerminalService.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import { Disposable as VSCodeDisposable, EnvironmentVariableMutatorType, Termina
1414
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
1515
import { localize } from 'vs/nls';
1616
import { NotSupportedError } from 'vs/base/common/errors';
17-
import { serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
17+
import { serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
1818
import { CancellationTokenSource } from 'vs/base/common/cancellation';
1919
import { generateUuid } from 'vs/base/common/uuid';
20-
import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
20+
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
2121
import { ICreateContributedTerminalProfileOptions, IProcessReadyEvent, IShellLaunchConfigDto, ITerminalChildProcess, ITerminalLaunchError, ITerminalProfile, TerminalIcon, TerminalLocation, IProcessProperty, ProcessPropertyType, IProcessPropertyMap } from 'vs/platform/terminal/common/terminal';
2222
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
2323
import { ThemeColor } from 'vs/platform/theme/common/themeService';

0 commit comments

Comments
 (0)