Skip to content

Commit 8a80ebe

Browse files
author
Kartik Raj
authored
Add experiment to implicitly use environment variables for environment activation (#20651)
1 parent 7aac96a commit 8a80ebe

File tree

33 files changed

+688
-154
lines changed

33 files changed

+688
-154
lines changed

‎.eslintignore

-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ src/client/interpreter/configuration/services/workspaceUpdaterService.ts
146146
src/client/interpreter/configuration/services/workspaceFolderUpdaterService.ts
147147
src/client/interpreter/helpers.ts
148148
src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts
149-
src/client/interpreter/activation/service.ts
150149
src/client/interpreter/display/index.ts
151150

152151
src/client/api.ts

‎package-lock.json

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+11-6
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"publisher": "ms-python",
2020
"enabledApiProposals": [
2121
"quickPickSortByLabel",
22+
"envShellEvent",
2223
"testObserver"
2324
],
2425
"author": {
@@ -40,7 +41,7 @@
4041
"theme": "dark"
4142
},
4243
"engines": {
43-
"vscode": "^1.75.0-20230123"
44+
"vscode": "^1.76.0"
4445
},
4546
"keywords": [
4647
"python",
@@ -426,12 +427,14 @@
426427
"enum": [
427428
"All",
428429
"pythonSurveyNotification",
429-
"pythonPromptNewToolsExt"
430+
"pythonPromptNewToolsExt",
431+
"pythonTerminalEnvVarActivation"
430432
],
431433
"enumDescriptions": [
432434
"%python.experiments.All.description%",
433435
"%python.experiments.pythonSurveyNotification.description%",
434-
"%python.experiments.pythonPromptNewToolsExt.description%"
436+
"%python.experiments.pythonPromptNewToolsExt.description%",
437+
"%python.experiments.pythonTerminalEnvVarActivation.description%"
435438
]
436439
},
437440
"scope": "machine",
@@ -445,12 +448,14 @@
445448
"enum": [
446449
"All",
447450
"pythonSurveyNotification",
448-
"pythonPromptNewToolsExt"
451+
"pythonPromptNewToolsExt",
452+
"pythonTerminalEnvVarActivation"
449453
],
450454
"enumDescriptions": [
451455
"%python.experiments.All.description%",
452456
"%python.experiments.pythonSurveyNotification.description%",
453-
"%python.experiments.pythonPromptNewToolsExt.description%"
457+
"%python.experiments.pythonPromptNewToolsExt.description%",
458+
"%python.experiments.pythonTerminalEnvVarActivation.description%"
454459
]
455460
},
456461
"scope": "machine",
@@ -1868,7 +1873,7 @@
18681873
"@types/stack-trace": "0.0.29",
18691874
"@types/tmp": "^0.0.33",
18701875
"@types/uuid": "^8.3.4",
1871-
"@types/vscode": "^1.74.0",
1876+
"@types/vscode": "^1.75.0",
18721877
"@types/which": "^2.0.1",
18731878
"@types/winreg": "^1.2.30",
18741879
"@types/xml2js": "^0.4.2",

‎package.nls.json

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"python.experiments.All.description": "Combined list of all experiments.",
3838
"python.experiments.pythonSurveyNotification.description": "Denotes the Python Survey Notification experiment.",
3939
"python.experiments.pythonPromptNewToolsExt.description": "Denotes the Python Prompt New Tools Extension experiment.",
40+
"python.experiments.pythonTerminalEnvVarActivation.description": "Enables use of environment variables to activate terminals instead of sending activation commands.",
4041
"python.formatting.autopep8Args.description": "Arguments passed in. Each argument is a separate item in the array.",
4142
"python.formatting.autopep8Path.description": "Path to autopep8, you can use a custom version of autopep8 by modifying this setting to include the full path.",
4243
"python.formatting.blackArgs.description": "Arguments passed in. Each argument is a separate item in the array.",

‎src/client/common/application/applicationEnvironment.ts

+13-9
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { inject, injectable } from 'inversify';
77
import * as path from 'path';
88
import { parse } from 'semver';
99
import * as vscode from 'vscode';
10+
import { traceError } from '../../logging';
1011
import { Channel } from '../constants';
1112
import { IPlatformService } from '../platform/types';
1213
import { ICurrentProcess, IPathUtils } from '../types';
@@ -70,19 +71,22 @@ export class ApplicationEnvironment implements IApplicationEnvironment {
7071
public get extensionName(): string {
7172
return this.packageJson.displayName;
7273
}
73-
/**
74-
* At the time of writing this API, the vscode.env.shell isn't officially released in stable version of VS Code.
75-
* Using this in stable version seems to throw errors in VSC with messages being displayed to the user about use of
76-
* unstable API.
77-
* Solution - log and suppress the errors.
78-
* @readonly
79-
* @type {(string)}
80-
* @memberof ApplicationEnvironment
81-
*/
74+
8275
public get shell(): string {
8376
return vscode.env.shell;
8477
}
8578

79+
public get onDidChangeShell(): vscode.Event<string> {
80+
try {
81+
return vscode.env.onDidChangeShell;
82+
} catch (ex) {
83+
traceError('Failed to get onDidChangeShell API', ex);
84+
// `onDidChangeShell` is a proposed API at the time of writing this, so wrap this in a try...catch
85+
// block in case the API is removed or changed.
86+
return new vscode.EventEmitter<string>().event;
87+
}
88+
}
89+
8690
public get packageJson(): any {
8791
return require('../../../../package.json');
8892
}

‎src/client/common/application/types.ts

+4
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,10 @@ export interface IApplicationEnvironment {
10481048
* @memberof IApplicationShell
10491049
*/
10501050
readonly shell: string;
1051+
/**
1052+
* An {@link Event} which fires when the default shell changes.
1053+
*/
1054+
readonly onDidChangeShell: Event<string>;
10511055
/**
10521056
* Gets the vscode channel (whether 'insiders' or 'stable').
10531057
*/

‎src/client/common/experiments/groups.ts

+4
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,7 @@ export enum ShowExtensionSurveyPrompt {
66
export enum ShowToolsExtensionPrompt {
77
experiment = 'pythonPromptNewToolsExt',
88
}
9+
10+
export enum TerminalEnvVarActivation {
11+
experiment = 'pythonTerminalEnvVarActivation',
12+
}

‎src/client/common/experiments/helpers.ts

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { workspace } from 'vscode';
7+
import { isTestExecution } from '../constants';
8+
import { IExperimentService } from '../types';
9+
import { TerminalEnvVarActivation } from './groups';
10+
11+
export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean {
12+
if (workspace.workspaceFile && !isTestExecution()) {
13+
// Don't run experiment in multi-root workspaces for now, requires work on VSCode:
14+
// https://github.com/microsoft/vscode/issues/171173
15+
return false;
16+
}
17+
if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) {
18+
return false;
19+
}
20+
return true;
21+
}

‎src/client/common/process/logger.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import { inject, injectable } from 'inversify';
77
import { traceLog } from '../../logging';
88
import { IWorkspaceService } from '../application/types';
99
import { isCI, isTestExecution } from '../constants';
10-
import { Logging } from '../utils/localize';
1110
import { getOSType, getUserHomeDir, OSType } from '../utils/platform';
1211
import { IProcessLogger, SpawnOptions } from './types';
1312
import { escapeRegExp } from 'lodash';
1413
import { replaceAll } from '../stringUtils';
14+
import { identifyShellFromShellPath } from '../terminal/shellDetectors/baseShellDetector';
1515

1616
@injectable()
1717
export class ProcessLogger implements IProcessLogger {
@@ -27,8 +27,11 @@ export class ProcessLogger implements IProcessLogger {
2727
? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgumentForPythonExt()).join(' ')
2828
: fileOrCommand;
2929
const info = [`> ${this.getDisplayCommands(command)}`];
30-
if (options && options.cwd) {
31-
info.push(`${Logging.currentWorkingDirectory} ${this.getDisplayCommands(options.cwd)}`);
30+
if (options?.cwd) {
31+
info.push(`cwd: ${this.getDisplayCommands(options.cwd)}`);
32+
}
33+
if (typeof options?.shell === 'string') {
34+
info.push(`shell: ${identifyShellFromShellPath(options?.shell)}`);
3235
}
3336

3437
info.forEach((line) => {

‎src/client/common/terminal/activator/index.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
import { inject, injectable, multiInject } from 'inversify';
77
import { Terminal } from 'vscode';
8-
import { IConfigurationService } from '../../types';
8+
import { inTerminalEnvVarExperiment } from '../../experiments/helpers';
9+
import { IConfigurationService, IExperimentService } from '../../types';
910
import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types';
1011
import { BaseTerminalActivator } from './base';
1112

@@ -17,6 +18,7 @@ export class TerminalActivator implements ITerminalActivator {
1718
@inject(ITerminalHelper) readonly helper: ITerminalHelper,
1819
@multiInject(ITerminalActivationHandler) private readonly handlers: ITerminalActivationHandler[],
1920
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
21+
@inject(IExperimentService) private readonly experimentService: IExperimentService,
2022
) {
2123
this.initialize();
2224
}
@@ -37,7 +39,8 @@ export class TerminalActivator implements ITerminalActivator {
3739
options?: TerminalActivationOptions,
3840
): Promise<boolean> {
3941
const settings = this.configurationService.getSettings(options?.resource);
40-
const activateEnvironment = settings.terminal.activateEnvironment;
42+
const activateEnvironment =
43+
settings.terminal.activateEnvironment && !inTerminalEnvVarExperiment(this.experimentService);
4144
if (!activateEnvironment || options?.hideFromUser) {
4245
return false;
4346
}

‎src/client/common/terminal/shellDetectors/baseShellDetector.ts

+19-15
Original file line numberDiff line numberDiff line change
@@ -54,22 +54,26 @@ export abstract class BaseShellDetector implements IShellDetector {
5454
terminal?: Terminal,
5555
): TerminalShellType | undefined;
5656
public identifyShellFromShellPath(shellPath: string): TerminalShellType {
57-
// Remove .exe extension so shells can be more consistently detected
58-
// on Windows (including Cygwin).
59-
const basePath = shellPath.replace(/\.exe$/, '');
57+
return identifyShellFromShellPath(shellPath);
58+
}
59+
}
6060

61-
const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => {
62-
if (matchedShell === TerminalShellType.other) {
63-
const pat = detectableShells.get(shellToDetect);
64-
if (pat && pat.test(basePath)) {
65-
return shellToDetect;
66-
}
61+
export function identifyShellFromShellPath(shellPath: string): TerminalShellType {
62+
// Remove .exe extension so shells can be more consistently detected
63+
// on Windows (including Cygwin).
64+
const basePath = shellPath.replace(/\.exe$/, '');
65+
66+
const shell = Array.from(detectableShells.keys()).reduce((matchedShell, shellToDetect) => {
67+
if (matchedShell === TerminalShellType.other) {
68+
const pat = detectableShells.get(shellToDetect);
69+
if (pat && pat.test(basePath)) {
70+
return shellToDetect;
6771
}
68-
return matchedShell;
69-
}, TerminalShellType.other);
72+
}
73+
return matchedShell;
74+
}, TerminalShellType.other);
7075

71-
traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`);
72-
traceVerbose(`Shell path identified as shell '${shell}'`);
73-
return shell;
74-
}
76+
traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`);
77+
traceVerbose(`Shell path identified as shell '${shell}'`);
78+
return shell;
7579
}

‎src/client/common/utils/localize.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ export namespace Interpreters {
193193
export const condaInheritEnvMessage = l10n.t(
194194
'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings. [Learn more](https://aka.ms/AA66i8f).',
195195
);
196+
export const activatingTerminals = l10n.t('Reactivating terminals...');
196197
export const activatedCondaEnvLaunch = l10n.t(
197198
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
198199
);
@@ -243,10 +244,6 @@ export namespace OutputChannelNames {
243244
export const pythonTest = l10n.t('Python Test Log');
244245
}
245246

246-
export namespace Logging {
247-
export const currentWorkingDirectory = l10n.t('cwd:');
248-
}
249-
250247
export namespace Linters {
251248
export const selectLinter = l10n.t('Select Linter');
252249
}

‎src/client/debugger/extension/configuration/resolvers/helper.ts

+11-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ import { getSearchPathEnvVarNames } from '../../../../common/utils/exec';
1313

1414
export const IDebugEnvironmentVariablesService = Symbol('IDebugEnvironmentVariablesService');
1515
export interface IDebugEnvironmentVariablesService {
16-
getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables>;
16+
getEnvironmentVariables(
17+
args: LaunchRequestArguments,
18+
baseVars?: EnvironmentVariables,
19+
): Promise<EnvironmentVariables>;
1720
}
1821

1922
@injectable()
@@ -23,7 +26,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
2326
@inject(ICurrentProcess) private process: ICurrentProcess,
2427
) {}
2528

26-
public async getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables> {
29+
public async getEnvironmentVariables(
30+
args: LaunchRequestArguments,
31+
baseVars?: EnvironmentVariables,
32+
): Promise<EnvironmentVariables> {
2733
const pathVariableName = getSearchPathEnvVarNames()[0];
2834

2935
// Merge variables from both .env file and env json variables.
@@ -37,6 +43,9 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
3743
// "overwrite: true" to ensure that debug-configuration env variable values
3844
// take precedence over env file.
3945
this.envParser.mergeVariables(debugLaunchEnvVars, env, { overwrite: true });
46+
if (baseVars) {
47+
this.envParser.mergeVariables(baseVars, env);
48+
}
4049

4150
// Append the PYTHONPATH and PATH variables.
4251
this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]);

‎src/client/debugger/extension/configuration/resolvers/launch.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { InvalidPythonPathInDebuggerServiceId } from '../../../../application/di
99
import { IDiagnosticsService, IInvalidPythonPathInDebuggerService } from '../../../../application/diagnostics/types';
1010
import { IConfigurationService } from '../../../../common/types';
1111
import { getOSType, OSType } from '../../../../common/utils/platform';
12+
import { EnvironmentVariables } from '../../../../common/variables/types';
13+
import { IEnvironmentActivationService } from '../../../../interpreter/activation/types';
1214
import { IInterpreterService } from '../../../../interpreter/contracts';
1315
import { DebuggerTypeName } from '../../../constants';
1416
import { DebugOptions, DebugPurpose, LaunchRequestArguments } from '../../../types';
@@ -24,6 +26,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
2426
@inject(IConfigurationService) configurationService: IConfigurationService,
2527
@inject(IDebugEnvironmentVariablesService) private readonly debugEnvHelper: IDebugEnvironmentVariablesService,
2628
@inject(IInterpreterService) interpreterService: IInterpreterService,
29+
@inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService,
2730
) {
2831
super(configurationService, interpreterService);
2932
}
@@ -81,6 +84,7 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
8184
workspaceFolder: Uri | undefined,
8285
debugConfiguration: LaunchRequestArguments,
8386
): Promise<void> {
87+
const isPythonSet = debugConfiguration.python !== undefined;
8488
if (debugConfiguration.python === undefined) {
8589
debugConfiguration.python = debugConfiguration.pythonPath;
8690
}
@@ -99,10 +103,17 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
99103
const settings = this.configurationService.getSettings(workspaceFolder);
100104
debugConfiguration.envFile = settings.envFile;
101105
}
106+
let baseEnvVars: EnvironmentVariables | undefined;
107+
if (isPythonSet) {
108+
baseEnvVars = await this.environmentActivationService.getActivatedEnvironmentVariables(
109+
workspaceFolder,
110+
await this.interpreterService.getInterpreterDetails(debugConfiguration.python ?? ''),
111+
);
112+
}
102113
// Extract environment variables from .env file in the vscode context and
103114
// set the "env" debug configuration argument. This expansion should be
104115
// done here before handing of the environment settings to the debug adapter
105-
debugConfiguration.env = await this.debugEnvHelper.getEnvironmentVariables(debugConfiguration);
116+
debugConfiguration.env = await this.debugEnvHelper.getEnvironmentVariables(debugConfiguration, baseEnvVars);
106117

107118
if (typeof debugConfiguration.stopOnEntry !== 'boolean') {
108119
debugConfiguration.stopOnEntry = false;

0 commit comments

Comments
 (0)