Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply env var collection path prefixes in shell integration scripts when mac+login shell #171066

Merged
merged 12 commits into from
Jan 11, 2023
Merged
41 changes: 41 additions & 0 deletions src/vs/platform/terminal/common/environmentVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IProcessEnvironment } from 'vs/base/common/platform';

export enum EnvironmentVariableMutatorType {
Replace = 1,
Append = 2,
Expand All @@ -12,8 +14,47 @@ export interface IEnvironmentVariableMutator {
readonly value: string;
readonly type: EnvironmentVariableMutatorType;
}

export interface IEnvironmentVariableCollection {
readonly map: ReadonlyMap<string, IEnvironmentVariableMutator>;
}

/** [variable, mutator] */
export type ISerializableEnvironmentVariableCollection = [string, IEnvironmentVariableMutator][];

/** [extension, collection] */
export type ISerializableEnvironmentVariableCollections = [string, ISerializableEnvironmentVariableCollection][];

export interface IExtensionOwnedEnvironmentVariableMutator extends IEnvironmentVariableMutator {
readonly extensionIdentifier: string;
}

export interface IMergedEnvironmentVariableCollectionDiff {
added: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
changed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
removed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
}

type VariableResolver = (str: string) => Promise<string>;

/**
* Represents an environment variable collection that results from merging several collections
* together.
*/
export interface IMergedEnvironmentVariableCollection {
readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>;
readonly map: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;

/**
* Applies this collection to a process environment.
* @param variableResolver An optional function to use to resolve variables within the
* environment values.
*/
applyToProcessEnvironment(env: IProcessEnvironment, variableResolver?: VariableResolver): Promise<void>;

/**
* Generates a diff of this connection against another. Returns undefined if the collections are
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
* the same.
*/
diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
*--------------------------------------------------------------------------------------------*/

import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { VariableResolver } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';

type VariableResolver = (str: string) => Promise<string>;

export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

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

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

Expand Down
43 changes: 41 additions & 2 deletions src/vs/platform/terminal/node/terminalEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ import * as os from 'os';
import { FileAccess } from 'vs/base/common/network';
import { getCaseInsensitive } from 'vs/base/common/objects';
import * as path from 'vs/base/common/path';
import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
import { IProcessEnvironment, isMacintosh, isWindows } from 'vs/base/common/platform';
import * as process from 'vs/base/common/process';
import { format } from 'vs/base/common/strings';
import { isString } from 'vs/base/common/types';
import * as pfs from 'vs/base/node/pfs';
import { ILogService } from 'vs/platform/log/common/log';
import { IProductService } from 'vs/platform/product/common/productService';
import { IShellLaunchConfig, ITerminalEnvironment, ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
import { EnvironmentVariableMutatorType } from 'vs/platform/terminal/common/environmentVariable';
import { deserializeEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariableShared';
import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';

export function getWindowsBuildNumber(): number {
const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release());
Expand Down Expand Up @@ -104,7 +107,7 @@ export interface IShellIntegrationConfigInjection {
*/
export function getShellIntegrationInjection(
shellLaunchConfig: IShellLaunchConfig,
options: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'>,
options: ITerminalProcessOptions,
env: ITerminalEnvironment | undefined,
logService: ILogService,
productService: IProductService
Expand Down Expand Up @@ -151,6 +154,7 @@ export function getShellIntegrationInjection(
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
} else if (areZshBashLoginArgs(originalArgs)) {
envMixin['VSCODE_SHELL_LOGIN'] = '1';
addEnvMixinPathPrefix(options, envMixin);
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
}
if (!newArgs) {
Expand All @@ -166,6 +170,7 @@ export function getShellIntegrationInjection(
const oldDataDirs = env?.XDG_DATA_DIRS ?? '/usr/local/share:/usr/share';
const newDataDir = path.join(appRoot, 'out/vs/workbench/contrib/xdg_data');
envMixin['XDG_DATA_DIRS'] = `${oldDataDirs}:${newDataDir}`;
addEnvMixinPathPrefix(options, envMixin);
return { newArgs: undefined, envMixin };
}
case 'pwsh': {
Expand All @@ -186,6 +191,7 @@ export function getShellIntegrationInjection(
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh);
} else if (areZshBashLoginArgs(originalArgs)) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin);
addEnvMixinPathPrefix(options, envMixin);
} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin)) {
newArgs = originalArgs;
}
Expand Down Expand Up @@ -230,6 +236,39 @@ export function getShellIntegrationInjection(
return undefined;
}

/**
* On macOS the profile calls path_helper which adds a bunch of standard bin directories to the
* beginning of the PATH. This causes significant problems for the environment variable
* collection API as the custom paths added to the end will now be somewhere in the middle of
* the PATH. To combat this, VSCODE_PATH_PREFIX is used to re-apply any prefix after the profile
* has run. This will cause duplication in the PATH but should fix the issue.
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
*
* See #99878 for more information.
*/
function addEnvMixinPathPrefix(options: ITerminalProcessOptions, envMixin: IProcessEnvironment): void {
if (isMacintosh && options.environmentVariableCollections) {
// Deserialize and merge
const deserialized = deserializeEnvironmentVariableCollections(options.environmentVariableCollections);
const merged = new MergedEnvironmentVariableCollection(deserialized);

// Get all prepend PATH entries
const pathEntry = merged.map.get('PATH');
const prependToPath: string[] = [];
if (pathEntry) {
for (const mutator of pathEntry) {
if (mutator.type === EnvironmentVariableMutatorType.Prepend) {
prependToPath.push(mutator.value);
}
}
}

// Add to the environment mixin to be applied in the shell integration script
if (prependToPath.length > 0) {
envMixin['VSCODE_PATH_PREFIX'] = prependToPath.join('');
}
}
}

enum ShellIntegrationExecutable {
WindowsPwsh = 'windows-pwsh',
WindowsPwshLogin = 'windows-pwsh-login',
Expand Down
2 changes: 1 addition & 1 deletion src/vs/platform/terminal/node/terminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess

let injection: IShellIntegrationConfigInjection | undefined;
if (this._options.shellIntegration.enabled) {
injection = getShellIntegrationInjection(this.shellLaunchConfig, { shellIntegration: this._options.shellIntegration, windowsEnableConpty: this._options.windowsEnableConpty }, this._ptyOptions.env, this._logService, this._productService);
injection = getShellIntegrationInjection(this.shellLaunchConfig, this._options, this._ptyOptions.env, this._logService, this._productService);
if (injection) {
this._onDidChangeProperty.fire({ type: ProcessPropertyType.UsedShellIntegrationInjection, value: true });
if (injection.envMixin) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { IProductService } from 'vs/platform/product/common/productService';
import { ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
import { getShellIntegrationInjection, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';

const enabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: true };
const disabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: false }, windowsEnableConpty: true };
const winptyProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: false };
const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: true, environmentVariableCollections: undefined };
const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined };
const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: false, environmentVariableCollections: undefined };
const pwshExe = process.platform === 'win32' ? 'pwsh.exe' : 'pwsh';
const repoRoot = process.platform === 'win32' ? process.cwd()[0].toLowerCase() + process.cwd().substring(1) : process.cwd();
const logService = new NullLogService();
Expand Down
6 changes: 3 additions & 3 deletions src/vs/server/node/remoteTerminalChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import { IGetTerminalLayoutInfoArgs, ISetTerminalLayoutInfoArgs } from 'vs/platf
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
import { createURITransformer } from 'vs/workbench/api/node/uriTransformer';
import { CLIServerBase, ICommandsExecuter } from 'vs/workbench/api/node/extHostCLIServer';
import { IEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { MergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableCollection';
import { deserializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
import { IEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';
import { deserializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
import { ICreateTerminalProcessArguments, ICreateTerminalProcessResult, IWorkspaceFolderData } from 'vs/workbench/contrib/terminal/common/remoteTerminalChannel';
import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
import { AbstractVariableResolverService } from 'vs/workbench/services/configurationResolver/common/variableResolver';
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/api/browser/mainThreadTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { IProcessProperty, IShellLaunchConfig, IShellLaunchConfigDto, ProcessPro
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
import { ITerminalEditorService, ITerminalExternalLinkProvider, ITerminalGroupService, ITerminalInstance, ITerminalLink, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy';
import { IEnvironmentVariableService, ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
import { IStartExtensionTerminalRequest, ITerminalProcessExtHostProxy, ITerminalProfileResolverService, ITerminalProfileService, ITerminalQuickFixService } from 'vs/workbench/contrib/terminal/common/terminal';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { withNullAsUndefined } from 'vs/base/common/types';
Expand All @@ -26,6 +26,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
import { ITerminalCommand } from 'vs/platform/terminal/common/capabilities/capabilities';
import { ITerminalOutputMatch, ITerminalOutputMatcher, ITerminalQuickFix, ITerminalQuickFixOptions } from 'vs/platform/terminal/common/xterm/terminalQuickFix';
import { TerminalQuickFixType } from 'vs/workbench/api/common/extHostTypes';
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';


@extHostNamedCustomer(MainContext.MainThreadTerminalService)
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
import { OutputChannelUpdateMode } from 'vs/workbench/services/output/common/output';
import { InputValidationType } from 'vs/workbench/contrib/scm/common/scm';
import { IWorkspaceSymbol } from 'vs/workbench/contrib/search/common/search';
import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
import { CoverageDetails, ExtensionRunTestsRequest, ICallProfileRunHandler, IFileCoverage, ISerializedTestResults, ITestItem, ITestMessage, ITestRunProfile, ITestRunTask, ResolvedTestRunRequest, IStartControllerTests, TestResultState, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testTypes';
import { Timeline, TimelineChangeEvent, TimelineOptions, TimelineProviderDescriptor } from 'vs/workbench/contrib/timeline/common/timeline';
import { TypeHierarchyItem } from 'vs/workbench/contrib/typeHierarchy/common/typeHierarchy';
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/common/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import { Disposable as VSCodeDisposable, EnvironmentVariableMutatorType, Termina
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
import { localize } from 'vs/nls';
import { NotSupportedError } from 'vs/base/common/errors';
import { serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
import { serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { generateUuid } from 'vs/base/common/uuid';
import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
import { ICreateContributedTerminalProfileOptions, IProcessReadyEvent, IShellLaunchConfigDto, ITerminalChildProcess, ITerminalLaunchError, ITerminalProfile, TerminalIcon, TerminalLocation, IProcessProperty, ProcessPropertyType, IProcessPropertyMap } from 'vs/platform/terminal/common/terminal';
import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
import { ThemeColor } from 'vs/platform/theme/common/themeService';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { EnvironmentVariableMutatorType, IEnvironmentVariableInfo, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
import { TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
import { ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
import { localize } from 'vs/nls';
import { ThemeIcon } from 'vs/platform/theme/common/themeService';
import { Codicon } from 'vs/base/common/codicons';
import { IHoverAction } from 'vs/workbench/services/hover/browser/hover';
import { EnvironmentVariableMutatorType, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';

export class EnvironmentVariableInfoStale implements IEnvironmentVariableInfo {
readonly requiresAction = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ fi

VSCODE_SHELL_INTEGRATION=1

echo "before $PATH"
Tyriar marked this conversation as resolved.
Show resolved Hide resolved

Tyriar marked this conversation as resolved.
Show resolved Hide resolved
# Run relevant rc/profile only if shell integration has been injected, not when run manually
if [ "$VSCODE_INJECTION" == "1" ]; then
if [ -z "$VSCODE_SHELL_LOGIN" ]; then
Expand All @@ -31,10 +33,18 @@ if [ "$VSCODE_INJECTION" == "1" ]; then
. ~/.profile
fi
builtin unset VSCODE_SHELL_LOGIN

# Apply any explicit path prefix (see #99878)
if [ -n "$VSCODE_PATH_PREFIX" ]; then
export PATH=$VSCODE_PATH_PREFIX$PATH
builtin unset VSCODE_PATH_PREFIX
fi
fi
builtin unset VSCODE_INJECTION
fi

echo "after $PATH"

Tyriar marked this conversation as resolved.
Show resolved Hide resolved
if [ -z "$VSCODE_SHELL_INTEGRATION" ]; then
builtin return
fi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,10 @@ if [[ $options[norcs] = off && -o "login" && -f $USER_ZDOTDIR/.zprofile ]]; the
ZDOTDIR=$USER_ZDOTDIR
. $USER_ZDOTDIR/.zprofile
ZDOTDIR=$VSCODE_ZDOTDIR

# Apply any explicit path prefix (see #99878)
if (( ${+VSCODE_PATH_PREFIX} )); then
export PATH=$VSCODE_PATH_PREFIX$PATH
fi
builtin unset VSCODE_PATH_PREFIX
fi
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ or exit

set --global VSCODE_SHELL_INTEGRATION 1

# Apply any explicit path prefix (see #99878)
if status --is-login; and set -q VSCODE_PATH_PREFIX
fish_add_path -p $VSCODE_PATH_PREFIX
end
set -e VSCODE_PATH_PREFIX

# Helper function
function __vsc_esc -d "Emit escape sequences for VS Code shell integration"
builtin printf "\e]633;%s\a" (string join ";" $argv)
Expand Down
Loading