Skip to content

Commit

Permalink
handle bash login args (#141467)
Browse files Browse the repository at this point in the history
  • Loading branch information
meganrogge committed Jan 25, 2022
1 parent c563008 commit f7b1711
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 96 deletions.
2 changes: 1 addition & 1 deletion src/vs/base/common/processes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export function sanitizeProcessEnvironment(env: IProcessEnvironment, ...preserve
}, {} as Record<string, boolean>);
const keysToRemove = [
/^ELECTRON_.+$/,
/^VSCODE_.+$/,
/^VSCODE_(?!SHELL_LOGIN).+$/,
/^SNAP(|_.*)$/,
/^GDK_PIXBUF_.+$/,
];
Expand Down
6 changes: 4 additions & 2 deletions src/vs/base/test/common/processes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ suite('Processes', () => {
VSCODE_NLS_CONFIG: 'x',
VSCODE_PORTABLE: 'x',
VSCODE_PID: 'x',
VSCODE_SHELL_LOGIN: '1',
VSCODE_CODE_CACHE_PATH: 'x',
VSCODE_NEW_VAR: 'x',
GDK_PIXBUF_MODULE_FILE: 'x',
GDK_PIXBUF_MODULEDIR: 'x',
GDK_PIXBUF_MODULEDIR: 'x'
};
processes.sanitizeProcessEnvironment(env);
assert.strictEqual(env['FOO'], 'bar');
assert.strictEqual(Object.keys(env).length, 1);
assert.strictEqual(env['VSCODE_SHELL_LOGIN'], '1');
assert.strictEqual(Object.keys(env).length, 2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ if [ -z "$VSCODE_SHELL_LOGIN" ]; then
else
# Imitate -l because --init-file doesn't support it:
# run the first of these files that exists
if [ -f "~/.bash_profile" ]; then
if [ -f ~/.bash_profile ]; then
. ~/.bash_profile
elif [ -f "~/.bash_login" ]; then
elif [ -f ~/.bash_login ]; then
. ~/.bash_login
elif [ -f "~/.profile" ]; then
elif [ -f ~/.profile ]; then
. ~/.profile
fi
VSCODE_SHELL_LOGIN=""
Expand Down
40 changes: 5 additions & 35 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storag
import { activeContrastBorder, scrollbarSliderActiveBackground, scrollbarSliderBackground, scrollbarSliderHoverBackground } from 'vs/platform/theme/common/colorRegistry';
import { ICssStyleCollector, IColorTheme, IThemeService, registerThemingParticipant, ThemeIcon } from 'vs/platform/theme/common/themeService';
import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager';
import { ITerminalProcessManager, ProcessState, TERMINAL_VIEW_ID, INavigationMode, DEFAULT_COMMANDS_TO_SKIP_SHELL, TERMINAL_CREATION_COMMANDS, ITerminalProfileResolverService, TerminalCommandId, ITerminalBackend, ITerminalCommand } from 'vs/workbench/contrib/terminal/common/terminal';
import { ITerminalProcessManager, ProcessState, TERMINAL_VIEW_ID, INavigationMode, DEFAULT_COMMANDS_TO_SKIP_SHELL, TERMINAL_CREATION_COMMANDS, ITerminalProfileResolverService, TerminalCommandId, ITerminalBackend, ITerminalCommand, ShellIntegrationExitCode } from 'vs/workbench/contrib/terminal/common/terminal';
import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper';
import { IDetectedLinks, TerminalLinkManager } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkManager';
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
Expand Down Expand Up @@ -71,8 +71,6 @@ import { fromNow } from 'vs/base/common/date';
import { ICommandService } from 'vs/platform/commands/common/commands';
import { TerminalCapabilityStoreMultiplexer } from 'vs/workbench/contrib/terminal/common/capabilities/terminalCapabilityStore';
import { TerminalCapability } from 'vs/workbench/contrib/terminal/common/capabilities/capabilities';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { injectShellIntegrationArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';

const enum Constants {
/**
Expand Down Expand Up @@ -328,8 +326,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
@IEditorService private readonly _editorService: IEditorService,
@IWorkspaceTrustRequestService private readonly _workspaceTrustRequestService: IWorkspaceTrustRequestService,
@ICommandService private readonly _commandService: ICommandService,
@IRemoteAgentService private readonly _remoteAgentService: IRemoteAgentService
@ICommandService private readonly _commandService: ICommandService
) {
super();

Expand Down Expand Up @@ -393,7 +390,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
// Wait for a period to allow a container to be ready
await this._containerReadyBarrier.wait();
if (this._configHelper.config.enableShellIntegration && !this.shellLaunchConfig.executable) {
const os = await this._getBackendOS();
const os = await this._processManager.getBackendOS();
this.shellLaunchConfig.executable = (await this._terminalProfileResolverService.getDefaultProfile({ remoteAuthority: this.remoteAuthority, os })).path;
}
await this._createProcess();
Expand Down Expand Up @@ -1282,13 +1279,10 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}

const hadIcon = !!this.shellLaunchConfig.icon;
const isBackendWindows = await this._backendIsWindows();
const shellIntegration = injectShellIntegrationArgs(this._logService, this._configHelper.config.enableShellIntegration, this.shellLaunchConfig, isBackendWindows);
this.shellLaunchConfig.args = shellIntegration.args;
const enableShellIntegration = shellIntegration.enableShellIntegration;

await this._processManager.createProcess(this._shellLaunchConfig, this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows, this._accessibilityService.isScreenReaderOptimized()).then(error => {
if (error) {
this._onProcessExit(error, enableShellIntegration);
this._onProcessExit(error, error.code === ShellIntegrationExitCode);
}
});
if (this.xterm?.shellIntegration) {
Expand All @@ -1299,30 +1293,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}
}

private async _backendIsWindows(): Promise<boolean> {
let os = OS;
if (!!this.remoteAuthority) {
const remoteEnv = await this._remoteAgentService.getEnvironment();
if (!remoteEnv) {
throw new Error(`Failed to get remote environment for remote authority "${this.remoteAuthority}"`);
}
os = remoteEnv.os;
}
return os === OperatingSystem.Windows;
}

private async _getBackendOS(): Promise<OperatingSystem> {
let os = OS;
if (!!this.remoteAuthority) {
const remoteEnv = await this._remoteAgentService.getEnvironment();
if (!remoteEnv) {
throw new Error(`Failed to get remote environment for remote authority "${this.remoteAuthority}"`);
}
os = remoteEnv.os;
}
return os;
}

private _onProcessData(ev: IProcessDataEvent): void {
const messageId = ++this._latestXtermWriteData;
if (ev.trackCommit) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
import { ProcessState, ITerminalProcessManager, ITerminalConfigHelper, IBeforeProcessDataEvent, ITerminalProfileResolverService, ITerminalBackend } from 'vs/workbench/contrib/terminal/common/terminal';
import { ProcessState, ITerminalProcessManager, ITerminalConfigHelper, IBeforeProcessDataEvent, ITerminalProfileResolverService, ITerminalBackend, ShellIntegrationExitCode } from 'vs/workbench/contrib/terminal/common/terminal';
import { ILogService } from 'vs/platform/log/common/log';
import { Emitter, Event } from 'vs/base/common/event';
import { IHistoryService } from 'vs/workbench/services/history/common/history';
Expand Down Expand Up @@ -63,6 +63,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
isDisconnected: boolean = false;
environmentVariableInfo: IEnvironmentVariableInfo | undefined;
backend: ITerminalBackend | undefined;
shellIntegrationAttempted: boolean = false;
readonly capabilities = new TerminalCapabilityStore();

private _isDisposed: boolean = false;
Expand Down Expand Up @@ -225,7 +226,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
this.os = remoteEnv.os;

// this is a copy of what the merged environment collection is on the remote side
await this._resolveEnvironment(backend, variableResolver, shellLaunchConfig);
const env = await this._resolveEnvironment(backend, variableResolver, shellLaunchConfig);

const shouldPersist = !shellLaunchConfig.isFeatureTerminal && this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.disablePersistence;
if (shellLaunchConfig.attachPersistentProcess) {
Expand All @@ -242,13 +243,29 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
os: this.os
});
try {
const shellIntegration = terminalEnvironment.injectShellIntegrationArgs(this._logService, env, this._configHelper.config.enableShellIntegration, shellLaunchConfig, this.os);
this.shellIntegrationAttempted = shellIntegration.enableShellIntegration;
if (this.shellIntegrationAttempted) {
shellLaunchConfig.args = shellIntegration.args;
// resolve the injected arguments
await this._terminalProfileResolverService.resolveShellLaunchConfig(shellLaunchConfig, {
remoteAuthority: this.remoteAuthority,
os: this.os
});
}
//TODO: fix
if (env?.['VSCODE_SHELL_LOGIN']) {
shellLaunchConfig.env = shellLaunchConfig.env || {} as IProcessEnvironment;
shellLaunchConfig.env['VSCODE_SHELL_LOGIN'] = '1';
}

newProcess = await backend.createProcess(
shellLaunchConfig,
'', // TODO: Fix cwd
cols,
rows,
this._configHelper.config.unicodeVersion,
{}, // TODO: Fix env
env, // TODO:
true, // TODO: Fix enable
shouldPersist
);
Expand Down Expand Up @@ -416,6 +433,16 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce

const env = await this._resolveEnvironment(backend, variableResolver, shellLaunchConfig);

const shellIntegration = terminalEnvironment.injectShellIntegrationArgs(this._logService, env, this._configHelper.config.enableShellIntegration, shellLaunchConfig, OS);
if (shellIntegration.enableShellIntegration) {
shellLaunchConfig.args = shellIntegration.args;
// resolve the injected arguments
await this._terminalProfileResolverService.resolveShellLaunchConfig(shellLaunchConfig, {
remoteAuthority: undefined,
os: OS
});
}
this.shellIntegrationAttempted = shellIntegration.enableShellIntegration;
const useConpty = this._configHelper.config.windowsEnableConpty && !isScreenReaderModeEnabled;
const shouldPersist = this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.isFeatureTerminal;
return await backend.createProcess(shellLaunchConfig, initialCwd, cols, rows, this._configHelper.config.unicodeVersion, env, useConpty, shouldPersist);
Expand Down Expand Up @@ -468,6 +495,18 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}));
}

async getBackendOS(): Promise<OperatingSystem> {
let os = OS;
if (!!this.remoteAuthority) {
const remoteEnv = await this._remoteAgentService.getEnvironment();
if (!remoteEnv) {
throw new Error(`Failed to get remote environment for remote authority "${this.remoteAuthority}"`);
}
os = remoteEnv.os;
}
return os;
}

setDimensions(cols: number, rows: number): Promise<void>;
setDimensions(cols: number, rows: number, sync: false): Promise<void>;
setDimensions(cols: number, rows: number, sync: true): void;
Expand Down Expand Up @@ -570,7 +609,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
this._setProcessState(ProcessState.KilledByProcess);
}

this._onProcessExit.fire(exitCode);
this._onProcessExit.fire(this.shellIntegrationAttempted ? ShellIntegrationExitCode : exitCode);
}

private _setProcessState(state: ProcessState) {
Expand Down
7 changes: 7 additions & 0 deletions src/vs/workbench/contrib/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ export interface ITerminalProfileResolverService {
createProfileFromShellAndShellArgs(shell?: unknown, shellArgs?: unknown): Promise<ITerminalProfile | string>;
}

/*
* When there were shell integration args injected
* and createProcess returns an error, this exit code will be used.
*/
export const ShellIntegrationExitCode = 633;

export interface IRegisterContributedProfileArgs {
extensionIdentifier: string, id: string, title: string, options: ICreateContributedTerminalProfileOptions;
}
Expand Down Expand Up @@ -408,6 +414,7 @@ export interface ITerminalProcessManager extends IDisposable {
getLatency(): Promise<number>;
refreshProperty<T extends ProcessPropertyType>(type: T): Promise<IProcessPropertyMap[T]>;
updateProperty<T extends ProcessPropertyType>(property: T, value: IProcessPropertyMap[T]): void;
getBackendOS(): Promise<OperatingSystem>;
}

export const enum ProcessState {
Expand Down
16 changes: 10 additions & 6 deletions src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,12 +414,12 @@ shellIntegrationArgs.set(ShellIntegrationExecutable.WindowsPwsh, ['-noexit', ' -
shellIntegrationArgs.set(ShellIntegrationExecutable.WindowsPwshLogin, ['-l', '-noexit', ' -command', '. \"${execInstallFolder}\\out\\vs\\workbench\\contrib\\terminal\\browser\\media\\shellIntegration.ps1\"']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Pwsh, ['-noexit', '-command', '. "${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1"']);
shellIntegrationArgs.set(ShellIntegrationExecutable.PwshLogin, ['-l', '-noexit', '-command', '. "${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1"']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Zsh, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/ShellIntegration-zsh.sh"; zsh -i']);
shellIntegrationArgs.set(ShellIntegrationExecutable.ZshLogin, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/ShellIntegration-zsh.sh"; zsh -il']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Bash, ['--init-file', '${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/ShellIntegration-bash.sh']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Zsh, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh"; zsh -i']);
shellIntegrationArgs.set(ShellIntegrationExecutable.ZshLogin, ['-c', '"${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh"; zsh -il']);
shellIntegrationArgs.set(ShellIntegrationExecutable.Bash, ['--init-file', '${execInstallFolder}/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh']);
const loginArgs = ['-login', '-l'];
const pwshImpliedArgs = ['-nol', '-nologo'];
export function injectShellIntegrationArgs(logService: ILogService, enableShellIntegration: boolean, shellLaunchConfig: IShellLaunchConfig, isBackendWindows?: boolean): { args: string | string[] | undefined, enableShellIntegration: boolean } {
export function injectShellIntegrationArgs(logService: ILogService, env: IProcessEnvironment, enableShellIntegration: boolean, shellLaunchConfig: IShellLaunchConfig, os?: OperatingSystem): { args: string | string[] | undefined, enableShellIntegration: boolean } {
// Shell integration arg injection is disabled when:
// - The global setting is disabled
// - There is no executable (not sure what script to run)
Expand All @@ -431,7 +431,8 @@ export function injectShellIntegrationArgs(logService: ILogService, enableShellI
const originalArgs = shellLaunchConfig.args;
const shell = path.basename(shellLaunchConfig.executable);
let newArgs: string | string[] | undefined;
if (isBackendWindows) {

if (os === OperatingSystem.Windows) {
if (shell === 'pwsh.exe') {
if (!originalArgs || arePwshImpliedArgs(originalArgs)) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.WindowsPwsh);
Expand All @@ -444,7 +445,10 @@ export function injectShellIntegrationArgs(logService: ILogService, enableShellI
} else {
switch (shell) {
case 'bash':
if (!originalArgs || originalArgs.length === 0 || areZshBashLoginArgs(originalArgs)) {
if (!originalArgs || originalArgs.length === 0) {
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
} else if (areZshBashLoginArgs(originalArgs)) {
env['VSCODE_SHELL_LOGIN'] = os === OperatingSystem.Macintosh ? '1' : '';
newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
}
break;
Expand Down

0 comments on commit f7b1711

Please sign in to comment.