From 9ba5b3db5c508715428d877eb7843ce9c1fa3f59 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 11 Jul 2023 13:12:36 -0700 Subject: [PATCH 01/16] Revert "Dev Container for VS Code Python Extension (#21435)" This reverts commit bd5b622a2456be181d215a1d758a0456412d7c72. --- .devcontainer/Dockerfile | 18 ------------------ .devcontainer/devcontainer.json | 30 ------------------------------ 2 files changed, 48 deletions(-) delete mode 100644 .devcontainer/Dockerfile delete mode 100644 .devcontainer/devcontainer.json diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile deleted file mode 100644 index 5fbf068de65f..000000000000 --- a/.devcontainer/Dockerfile +++ /dev/null @@ -1,18 +0,0 @@ -FROM mcr.microsoft.com/devcontainers/typescript-node:16-bookworm - -RUN apt-get install -y wget bzip2 - -# Run in silent mode and save downloaded script as anaconda.sh. -# Run with /bin/bash and run in silent mode to /opt/conda. -# Also get rid of installation script after finishing. -RUN wget --quiet https://repo.anaconda.com/archive/Anaconda3-2023.07-1-Linux-x86_64.sh -O ~/anaconda.sh && \ - /bin/bash ~/anaconda.sh -b -p /opt/conda && \ - rm ~/anaconda.sh - -ENV PATH="/opt/conda/bin:$PATH" - -# Sudo apt update needs to run in order for installation of fish to work . -RUN sudo apt update && \ - sudo apt install fish -y - - diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json deleted file mode 100644 index fe15f35764e6..000000000000 --- a/.devcontainer/devcontainer.json +++ /dev/null @@ -1,30 +0,0 @@ -// For format details, see https://aka.ms/devcontainer.json. -{ - "name": "VS Code Python Dev Container", - // Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile - "build": { - "dockerfile": "./Dockerfile", - "context": ".." - }, - "customizations": { - "vscode": { - "extensions": [ - "editorconfig.editorconfig", - "esbenp.prettier-vscode", - "dbaeumer.vscode-eslint", - "ms-python.python", - "ms-python.black-formatter", - "ms-python.vscode-pylance", - "charliermarsh.ruff" - ] - } - }, - // Commands to execute on container creation,start. - "postCreateCommand": "bash scripts/postCreateCommand.sh", - "onCreateCommand": "bash scripts/onCreateCommand.sh", - - "containerEnv": { - "CI_PYTHON_PATH": "/workspaces/vscode-python/.venv/bin/python" - } - -} From d39baecab29d49baa5194d7e382e9f63cb20a608 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 24 Jul 2023 23:15:46 +0000 Subject: [PATCH 02/16] Improve how to get process env variables --- src/client/interpreter/activation/service.ts | 34 ++++++++++++++++-- .../terminalEnvVarCollectionService.ts | 35 +++++++++++-------- src/client/interpreter/activation/types.ts | 2 ++ ...rminalEnvVarCollectionService.unit.test.ts | 16 ++++++--- 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index e5da57227b19..d17a2cc4138f 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -19,8 +19,8 @@ import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; import { sleep } from '../../common/utils/async'; import { InMemoryCache } from '../../common/utils/cacheUtils'; import { OSType } from '../../common/utils/platform'; -import { IEnvironmentVariablesProvider } from '../../common/variables/types'; -import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info'; +import { EnvironmentVariables, IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { EnvironmentType, PythonEnvironment, virtualEnvTypes } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { IInterpreterService } from '../contracts'; @@ -38,6 +38,7 @@ import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda import { StopWatch } from '../../common/utils/stopWatch'; import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector'; import { getSearchPathEnvVarNames } from '../../common/utils/exec'; +import { cache } from '../../common/utils/decorators'; const ENVIRONMENT_PREFIX = 'e8b39361-0157-4923-80e1-22d70d46dee6'; const CACHE_DURATION = 10 * 60 * 1000; @@ -176,6 +177,35 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi }); } + @cache(-1, true) + public async getProcessEnvironmentVariables(resource: Resource, shell?: string): Promise { + // Try to get the process environment variables using Python by printing variables, that can be little different + // from `process.env` and is preferred when calculating diff. + const globalInterpreters = this.interpreterService + .getInterpreters() + .filter((i) => !virtualEnvTypes.includes(i.envType)); + const interpreterPath = + globalInterpreters.length > 0 && globalInterpreters[0] ? globalInterpreters[0].path : 'python'; + try { + const [args, parse] = internalScripts.printEnvVariables(); + args.forEach((arg, i) => { + args[i] = arg.toCommandArgumentForPythonExt(); + }); + const command = `echo '${ENVIRONMENT_PREFIX}' && ${interpreterPath} ${args.join(' ')}`; + const processService = await this.processServiceFactory.create(resource); + const result = await processService.shellExec(command, { + shell: shell, + timeout: ENVIRONMENT_TIMEOUT, + maxBuffer: 1000 * 1000, + throwOnStdErr: false, + }); + const returnedEnv = this.parseEnvironmentOutput(result.stdout, parse); + return returnedEnv ?? process.env; + } catch (ex) { + return process.env; + } + } + public async getEnvironmentActivationShellCommands( resource: Resource, interpreter?: PythonEnvironment, diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 85b393425836..1209040ab5af 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -33,6 +33,7 @@ import { defaultShells } from './service'; import { IEnvironmentActivationService } from './types'; import { EnvironmentType } from '../../pythonEnvironments/info'; import { getSearchPathEnvVarNames } from '../../common/utils/exec'; +import { EnvironmentVariables } from '../../common/variables/types'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService { @@ -45,7 +46,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private registeredOnce = false; - private previousEnvVars = _normCaseKeys(process.env); + private previousEnvVars: EnvironmentVariables | undefined; constructor( @inject(IPlatformService) private readonly platform: IPlatformService, @@ -127,12 +128,21 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } envVarCollection.clear(); - this.previousEnvVars = _normCaseKeys(process.env); + this.previousEnvVars = undefined; return; } + if (!this.previousEnvVars) { + this.previousEnvVars = await this.environmentActivationService.getProcessEnvironmentVariables( + resource, + shell, + ); + } const previousEnv = this.previousEnvVars; this.previousEnvVars = env; - Object.keys(env).forEach((key) => { + for (const key in env) { + if (shouldSkip(key)) { + continue; + } const value = env[key]; const prevValue = previousEnv[key]; if (prevValue !== value) { @@ -144,14 +154,16 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ envVarCollection.delete(key); } } - }); - Object.keys(previousEnv).forEach((key) => { + } + + for (const key in previousEnv) { // If the previous env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); envVarCollection.delete(key); } - }); + } + const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``); envVarCollection.description = description; @@ -224,13 +236,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } } -export function _normCaseKeys(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { - const result: NodeJS.ProcessEnv = {}; - Object.keys(env).forEach((key) => { - // `os.environ` script used to get env vars normalizes keys to upper case: - // https://github.com/python/cpython/issues/101754 - // So convert `process.env` keys to upper case to match. - result[key.toUpperCase()] = env[key]; - }); - return result; +function shouldSkip(env: string) { + return ['_', 'SHLVL'].includes(env); } diff --git a/src/client/interpreter/activation/types.ts b/src/client/interpreter/activation/types.ts index d8e4ae16dbca..e00ef9b62b3f 100644 --- a/src/client/interpreter/activation/types.ts +++ b/src/client/interpreter/activation/types.ts @@ -4,10 +4,12 @@ 'use strict'; import { Resource } from '../../common/types'; +import { EnvironmentVariables } from '../../common/variables/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; export const IEnvironmentActivationService = Symbol('IEnvironmentActivationService'); export interface IEnvironmentActivationService { + getProcessEnvironmentVariables(resource: Resource, shell?: string): Promise; getActivatedEnvironmentVariables( resource: Resource, interpreter?: PythonEnvironment, diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index cb0b6b02f288..9ee70d3a7c66 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -31,14 +31,22 @@ import { import { Interpreters } from '../../../client/common/utils/localize'; import { OSType, getOSType } from '../../../client/common/utils/platform'; import { defaultShells } from '../../../client/interpreter/activation/service'; -import { - TerminalEnvVarCollectionService, - _normCaseKeys, -} from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; +import { TerminalEnvVarCollectionService } from '../../../client/interpreter/activation/terminalEnvVarCollectionService'; import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types'; import { IInterpreterService } from '../../../client/interpreter/contracts'; import { PathUtils } from '../../../client/common/platform/pathUtils'; +function _normCaseKeys(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const result: NodeJS.ProcessEnv = {}; + Object.keys(env).forEach((key) => { + // `os.environ` script used to get env vars normalizes keys to upper case: + // https://github.com/python/cpython/issues/101754 + // So convert `process.env` keys to upper case to match. + result[key.toUpperCase()] = env[key]; + }); + return result; +} + suite('Terminal Environment Variable Collection Service', () => { let platform: IPlatformService; let interpreterService: IInterpreterService; From 2092cead6696319809622e73e3cab1da620adf9f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 26 Jul 2023 12:45:30 -0700 Subject: [PATCH 03/16] Fix activation for powershell --- src/client/interpreter/activation/service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index d17a2cc4138f..c70270b4f609 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -191,7 +191,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi args.forEach((arg, i) => { args[i] = arg.toCommandArgumentForPythonExt(); }); - const command = `echo '${ENVIRONMENT_PREFIX}' && ${interpreterPath} ${args.join(' ')}`; + const command = `${interpreterPath} ${args.join(' ')}`; const processService = await this.processServiceFactory.create(resource); const result = await processService.shellExec(command, { shell: shell, @@ -277,7 +277,8 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const activationCommand = fixActivationCommands(activationCommands).join(' && '); // In order to make sure we know where the environment output is, // put in a dummy echo we can look for - command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`; + const commandSeparator = [TerminalShellType.powershell, TerminalShellType.powershellCore].includes(shellInfo.shellType) ? ';' : '&&'; + command = `${activationCommand} ${commandSeparator} echo '${ENVIRONMENT_PREFIX}' ${commandSeparator} python ${args.join(' ')}`; } // Make sure python warnings don't interfere with getting the environment. However From 43e5f6d0ece2605c665fe6763c26a96798a93bf2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 26 Jul 2023 20:25:17 +0000 Subject: [PATCH 04/16] Correctly apply PS1 --- .../activation/terminalEnvVarCollectionService.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 1209040ab5af..957eac71d1f5 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -147,8 +147,16 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const prevValue = previousEnv[key]; if (prevValue !== value) { if (value !== undefined) { - traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - envVarCollection.replace(key, value, { applyAtShellIntegration: true }); + if (key === 'PS1') { + traceVerbose(`Prepending environment variable ${key} in collection with ${value}`); + envVarCollection.prepend(key, value, { + applyAtShellIntegration: true, + applyAtProcessCreation: false, + }); + } else { + traceVerbose(`Setting environment variable ${key} in collection to ${value}`); + envVarCollection.replace(key, value, { applyAtShellIntegration: true }); + } } else { traceVerbose(`Clearing environment variable ${key} from collection`); envVarCollection.delete(key); From f6a14bb8a7fc99bd33faaa3b3403b67d950f9913 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Aug 2023 18:52:46 +0000 Subject: [PATCH 05/16] Change how we fetch previously set env vars --- package.json | 2 +- .../activation/terminalEnvVarCollectionService.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index b6a4b65b42a9..ef1e11981638 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.79.0-20230526" + "vscode": "^1.81" }, "enableTelemetry": false, "keywords": [ diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 957eac71d1f5..9ca10e0991ca 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -164,13 +164,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } } - for (const key in previousEnv) { - // If the previous env var is not in the current env, clear it from collection. + envVarCollection.forEach((key) => { + // If a previously set env var is not in the current env, clear it from collection. if (!(key in env)) { traceVerbose(`Clearing environment variable ${key} from collection`); envVarCollection.delete(key); } - } + }); const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``); From 6ed78a3d8b0b5cd5a0cb2fef5b27466253188981 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Aug 2023 18:56:50 +0000 Subject: [PATCH 06/16] Revert "Revert "Dev Container for VS Code Python Extension (#21435)"" This reverts commit 9ba5b3db5c508715428d877eb7843ce9c1fa3f59. --- .devcontainer/Dockerfile | 18 ++++++++++++++++++ .devcontainer/devcontainer.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 .devcontainer/Dockerfile create mode 100644 .devcontainer/devcontainer.json diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile new file mode 100644 index 000000000000..5fbf068de65f --- /dev/null +++ b/.devcontainer/Dockerfile @@ -0,0 +1,18 @@ +FROM mcr.microsoft.com/devcontainers/typescript-node:16-bookworm + +RUN apt-get install -y wget bzip2 + +# Run in silent mode and save downloaded script as anaconda.sh. +# Run with /bin/bash and run in silent mode to /opt/conda. +# Also get rid of installation script after finishing. +RUN wget --quiet https://repo.anaconda.com/archive/Anaconda3-2023.07-1-Linux-x86_64.sh -O ~/anaconda.sh && \ + /bin/bash ~/anaconda.sh -b -p /opt/conda && \ + rm ~/anaconda.sh + +ENV PATH="/opt/conda/bin:$PATH" + +# Sudo apt update needs to run in order for installation of fish to work . +RUN sudo apt update && \ + sudo apt install fish -y + + diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 000000000000..fe15f35764e6 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,30 @@ +// For format details, see https://aka.ms/devcontainer.json. +{ + "name": "VS Code Python Dev Container", + // Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile + "build": { + "dockerfile": "./Dockerfile", + "context": ".." + }, + "customizations": { + "vscode": { + "extensions": [ + "editorconfig.editorconfig", + "esbenp.prettier-vscode", + "dbaeumer.vscode-eslint", + "ms-python.python", + "ms-python.black-formatter", + "ms-python.vscode-pylance", + "charliermarsh.ruff" + ] + } + }, + // Commands to execute on container creation,start. + "postCreateCommand": "bash scripts/postCreateCommand.sh", + "onCreateCommand": "bash scripts/onCreateCommand.sh", + + "containerEnv": { + "CI_PYTHON_PATH": "/workspaces/vscode-python/.venv/bin/python" + } + +} From 46004922b33399e1406e2c46b04dbf12756df6e5 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Aug 2023 20:46:28 +0000 Subject: [PATCH 07/16] Fix --- package.json | 2 +- .../activation/terminalEnvVarCollectionService.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index ef1e11981638..4bd880570134 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.81" + "vscode": "^1.81.0" }, "enableTelemetry": false, "keywords": [ diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 9ca10e0991ca..10cd6c69ef96 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -139,9 +139,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } const previousEnv = this.previousEnvVars; this.previousEnvVars = env; - for (const key in env) { + Object.keys(env).forEach((key) => { if (shouldSkip(key)) { - continue; + return; } const value = env[key]; const prevValue = previousEnv[key]; @@ -162,7 +162,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ envVarCollection.delete(key); } } - } + }); envVarCollection.forEach((key) => { // If a previously set env var is not in the current env, clear it from collection. From 85c0d3258895fd11e4b0d8bf7f786dd579e1b2cd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Aug 2023 23:24:15 +0000 Subject: [PATCH 08/16] FIX LINT --- src/client/interpreter/activation/service.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index c70270b4f609..768dfa17afea 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -155,11 +155,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } // Cache only if successful, else keep trying & failing if necessary. - const cache = new InMemoryCache(CACHE_DURATION); + const memCache = new InMemoryCache(CACHE_DURATION); return this.getActivatedEnvironmentVariablesImpl(resource, interpreter, allowExceptions, shell) .then((vars) => { - cache.data = vars; - this.activatedEnvVariablesCache.set(cacheKey, cache); + memCache.data = vars; + this.activatedEnvVariablesCache.set(cacheKey, memCache); sendTelemetryEvent( EventName.PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, stopWatch.elapsedTime, @@ -194,7 +194,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const command = `${interpreterPath} ${args.join(' ')}`; const processService = await this.processServiceFactory.create(resource); const result = await processService.shellExec(command, { - shell: shell, + shell, timeout: ENVIRONMENT_TIMEOUT, maxBuffer: 1000 * 1000, throwOnStdErr: false, @@ -277,8 +277,14 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const activationCommand = fixActivationCommands(activationCommands).join(' && '); // In order to make sure we know where the environment output is, // put in a dummy echo we can look for - const commandSeparator = [TerminalShellType.powershell, TerminalShellType.powershellCore].includes(shellInfo.shellType) ? ';' : '&&'; - command = `${activationCommand} ${commandSeparator} echo '${ENVIRONMENT_PREFIX}' ${commandSeparator} python ${args.join(' ')}`; + const commandSeparator = [TerminalShellType.powershell, TerminalShellType.powershellCore].includes( + shellInfo.shellType, + ) + ? ';' + : '&&'; + command = `${activationCommand} ${commandSeparator} echo '${ENVIRONMENT_PREFIX}' ${commandSeparator} python ${args.join( + ' ', + )}`; } // Make sure python warnings don't interfere with getting the environment. However From 7ceee81f8647a3b5992d81a2b56cd240ca3791a6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 3 Aug 2023 23:29:16 +0000 Subject: [PATCH 09/16] Clear env collection if activate terminal setting is turned off --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 10cd6c69ef96..fdb492025dad 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -107,8 +107,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { const workspaceFolder = this.getWorkspaceFolder(resource); const settings = this.configurationService.getSettings(resource); + const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); + envVarCollection.clear(); return; } const env = await this.environmentActivationService.getActivatedEnvironmentVariables( @@ -117,7 +119,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ undefined, shell, ); - const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder); if (!env) { const shellType = identifyShellFromShellPath(shell); const defaultShell = defaultShells[this.platform.osType]; From 43fd79ec02185ce7ef036189565f39a68ef9ede6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 01:13:12 +0000 Subject: [PATCH 10/16] Add support for pyenv interpreters --- src/client/interpreter/activation/service.ts | 2 +- .../terminalEnvVarCollectionService.ts | 29 ++++++------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 768dfa17afea..4364cc825f78 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -261,7 +261,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi ); traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`); if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) { - if (interpreter?.envType === EnvironmentType.Venv) { + if (interpreter && [EnvironmentType.Venv, EnvironmentType.Pyenv].includes(interpreter?.envType)) { const key = getSearchPathEnvVarNames()[0]; if (env[key]) { env[key] = `${path.dirname(interpreter.path)}${path.delimiter}${env[key]}`; diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index fdb492025dad..3af2b6edb38f 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -46,7 +46,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private registeredOnce = false; - private previousEnvVars: EnvironmentVariables | undefined; + private processEnvVars: EnvironmentVariables | undefined; constructor( @inject(IPlatformService) private readonly platform: IPlatformService, @@ -91,6 +91,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.applicationEnvironment.onDidChangeShell( async (shell: string) => { this.showProgress(); + this.processEnvVars = undefined; // Pass in the shell where known instead of relying on the application environment, because of bug // on VSCode: https://github.com/microsoft/vscode/issues/160694 await this._applyCollection(undefined, shell).ignoreErrors(); @@ -108,9 +109,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const workspaceFolder = this.getWorkspaceFolder(resource); const settings = this.configurationService.getSettings(resource); const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder); + // Clear any previously set env vars from collection. + envVarCollection.clear(); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); - envVarCollection.clear(); return; } const env = await this.environmentActivationService.getActivatedEnvironmentVariables( @@ -128,24 +130,22 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ await this._applyCollection(resource, defaultShell?.shell); return; } - envVarCollection.clear(); - this.previousEnvVars = undefined; + this.processEnvVars = undefined; return; } - if (!this.previousEnvVars) { - this.previousEnvVars = await this.environmentActivationService.getProcessEnvironmentVariables( + if (!this.processEnvVars) { + this.processEnvVars = await this.environmentActivationService.getProcessEnvironmentVariables( resource, shell, ); } - const previousEnv = this.previousEnvVars; - this.previousEnvVars = env; + const processEnv = this.processEnvVars; Object.keys(env).forEach((key) => { if (shouldSkip(key)) { return; } const value = env[key]; - const prevValue = previousEnv[key]; + const prevValue = processEnv[key]; if (prevValue !== value) { if (value !== undefined) { if (key === 'PS1') { @@ -158,21 +158,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ traceVerbose(`Setting environment variable ${key} in collection to ${value}`); envVarCollection.replace(key, value, { applyAtShellIntegration: true }); } - } else { - traceVerbose(`Clearing environment variable ${key} from collection`); - envVarCollection.delete(key); } } }); - envVarCollection.forEach((key) => { - // If a previously set env var is not in the current env, clear it from collection. - if (!(key in env)) { - traceVerbose(`Clearing environment variable ${key} from collection`); - envVarCollection.delete(key); - } - }); - const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath); const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``); envVarCollection.description = description; From 0fbd54721f8252e2408a4b2791512c2309e6361c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 01:19:49 +0000 Subject: [PATCH 11/16] Add comment --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 3af2b6edb38f..cc51973470e7 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -46,6 +46,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private registeredOnce = false; + /** + * Carries default environment variables for the currently selected shell. + */ private processEnvVars: EnvironmentVariables | undefined; constructor( From a006c5e919228fe404c4662c2650449e8a830cf8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 02:13:32 +0000 Subject: [PATCH 12/16] Fix unit tests --- .../terminalEnvVarCollectionService.ts | 6 +- ...rminalEnvVarCollectionService.unit.test.ts | 76 ++++--------------- 2 files changed, 16 insertions(+), 66 deletions(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index cc51973470e7..f4c29f03707d 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -157,10 +157,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ applyAtShellIntegration: true, applyAtProcessCreation: false, }); - } else { - traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - envVarCollection.replace(key, value, { applyAtShellIntegration: true }); + return; } + traceVerbose(`Setting environment variable ${key} in collection to ${value}`); + envVarCollection.replace(key, value, { applyAtShellIntegration: true }); } } }); diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 9ee70d3a7c66..3ee56dfd3fbf 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -36,17 +36,6 @@ import { IEnvironmentActivationService } from '../../../client/interpreter/activ import { IInterpreterService } from '../../../client/interpreter/contracts'; import { PathUtils } from '../../../client/common/platform/pathUtils'; -function _normCaseKeys(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { - const result: NodeJS.ProcessEnv = {}; - Object.keys(env).forEach((key) => { - // `os.environ` script used to get env vars normalizes keys to upper case: - // https://github.com/python/cpython/issues/101754 - // So convert `process.env` keys to upper case to match. - result[key.toUpperCase()] = env[key]; - }); - return result; -} - suite('Terminal Environment Variable Collection Service', () => { let platform: IPlatformService; let interpreterService: IInterpreterService; @@ -97,11 +86,16 @@ suite('Terminal Environment Variable Collection Service', () => { }) .thenResolve(); environmentActivationService = mock(); + when(environmentActivationService.getProcessEnvironmentVariables(anything(), anything())).thenResolve( + process.env, + ); configService = mock(); when(configService.getSettings(anything())).thenReturn(({ terminal: { activateEnvironment: true }, pythonPath: displayPath, } as unknown) as IPythonSettings); + when(collection.clear()).thenResolve(); + when(scopedCollection.clear()).thenResolve(); terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( instance(platform), instance(interpreterService), @@ -177,8 +171,7 @@ suite('Terminal Environment Variable Collection Service', () => { }); test('If activated variables are returned for custom shell, apply it correctly to the collection', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; - delete envVars.PATH; + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; when( environmentActivationService.getActivatedEnvironmentVariables( anything(), @@ -193,13 +186,12 @@ suite('Terminal Environment Variable Collection Service', () => { await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + verify(collection.clear()).once(); verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); - verify(collection.delete('PATH')).once(); }); test('Verify envs are not applied if env activation is disabled', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; - delete envVars.PATH; + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; when( environmentActivationService.getActivatedEnvironmentVariables( anything(), @@ -219,13 +211,12 @@ suite('Terminal Environment Variable Collection Service', () => { await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + verify(collection.clear()).once(); verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).never(); - verify(collection.delete('PATH')).never(); }); test('Verify correct options are used when applying envs and setting description', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; - delete envVars.PATH; + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; const resource = Uri.file('a'); const workspaceFolder: WorkspaceFolder = { uri: Uri.file('workspacePath'), @@ -241,51 +232,11 @@ suite('Terminal Environment Variable Collection Service', () => { assert.deepEqual(options, { applyAtShellIntegration: true }); return Promise.resolve(); }); - when(scopedCollection.delete(anything())).thenResolve(); await terminalEnvVarCollectionService._applyCollection(resource, customShell); + verify(scopedCollection.clear()).once(); verify(scopedCollection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); - verify(scopedCollection.delete('PATH')).once(); - }); - - test('Only relative changes to previously applied variables are applied to the collection', async () => { - const envVars: NodeJS.ProcessEnv = { - RANDOM_VAR: 'random', - CONDA_PREFIX: 'prefix/to/conda', - ..._normCaseKeys(process.env), - }; - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(envVars); - - when(collection.replace(anything(), anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - const newEnvVars = cloneDeep(envVars); - delete newEnvVars.CONDA_PREFIX; - newEnvVars.RANDOM_VAR = undefined; // Deleting the variable from the collection is the same as setting it to undefined. - reset(environmentActivationService); - when( - environmentActivationService.getActivatedEnvironmentVariables( - anything(), - undefined, - undefined, - customShell, - ), - ).thenResolve(newEnvVars); - - await terminalEnvVarCollectionService._applyCollection(undefined, customShell); - - verify(collection.delete('CONDA_PREFIX')).once(); - verify(collection.delete('RANDOM_VAR')).once(); }); test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { @@ -297,7 +248,7 @@ suite('Terminal Environment Variable Collection Service', () => { customShell, ), ).thenResolve(undefined); - const envVars = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) }; + const envVars = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; when( environmentActivationService.getActivatedEnvironmentVariables( anything(), @@ -308,12 +259,11 @@ suite('Terminal Environment Variable Collection Service', () => { ).thenResolve(envVars); when(collection.replace(anything(), anything(), anything())).thenResolve(); - when(collection.delete(anything())).thenResolve(); await terminalEnvVarCollectionService._applyCollection(undefined, customShell); verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); - verify(collection.delete(anything())).never(); + verify(collection.clear()).twice(); }); test('If no activated variables are returned for default shell, clear collection', async () => { From c0354228cd0a34a373b57f0f863a4377e5faa0fa Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 02:19:21 +0000 Subject: [PATCH 13/16] Add unit test for PS1 --- ...rminalEnvVarCollectionService.unit.test.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 3ee56dfd3fbf..be93a2a266d0 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -9,6 +9,7 @@ import { cloneDeep } from 'lodash'; import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; import { EnvironmentVariableCollection, + EnvironmentVariableMutatorOptions, EnvironmentVariableScope, ProgressLocation, Uri, @@ -190,6 +191,31 @@ suite('Terminal Environment Variable Collection Service', () => { verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); }); + test('If activated variables contain PS1, prefix it using shell integration', async () => { + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; + when( + environmentActivationService.getActivatedEnvironmentVariables( + anything(), + undefined, + undefined, + customShell, + ), + ).thenResolve(envVars); + + when(collection.replace(anything(), anything(), anything())).thenResolve(); + when(collection.delete(anything())).thenResolve(); + let opts: EnvironmentVariableMutatorOptions | undefined; + when(collection.prepend('PS1', '(prompt)', anything())).thenCall((_, _v, o) => { + opts = o; + }); + + await terminalEnvVarCollectionService._applyCollection(undefined, customShell); + + verify(collection.clear()).once(); + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); + assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true }); + }); + test('Verify envs are not applied if env activation is disabled', async () => { const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; when( From 3d4a77fe3bb6aca6ee5c7c66516aeeebaa61c129 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 02:20:13 +0000 Subject: [PATCH 14/16] yUP --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index be93a2a266d0..acbd7b929343 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -5,7 +5,6 @@ import * as sinon from 'sinon'; import { assert, expect } from 'chai'; -import { cloneDeep } from 'lodash'; import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; import { EnvironmentVariableCollection, From c8740dc6b9fe490a519c14affb97faa562a9d8d3 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 02:22:01 +0000 Subject: [PATCH 15/16] Do not update engine --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4bd880570134..b6a4b65b42a9 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.81.0" + "vscode": "^1.79.0-20230526" }, "enableTelemetry": false, "keywords": [ From 8cc1afeb340c921611da0a3023ceac38656b1976 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 4 Aug 2023 02:29:33 +0000 Subject: [PATCH 16/16] Fix --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index acbd7b929343..458e2b28c181 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -191,7 +191,7 @@ suite('Terminal Environment Variable Collection Service', () => { }); test('If activated variables contain PS1, prefix it using shell integration', async () => { - const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env }; + const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ...process.env, PS1: '(prompt)' }; when( environmentActivationService.getActivatedEnvironmentVariables( anything(),