Skip to content

Commit 5b977f8

Browse files
author
Kartik Raj
committed
Restore old code
1 parent ca46f49 commit 5b977f8

File tree

2 files changed

+65
-28
lines changed

2 files changed

+65
-28
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

src/client/interpreter/activation/service.ts

+65-27
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1+
/* eslint-disable max-classes-per-file */
12
// Copyright (c) Microsoft Corporation. All rights reserved.
23
// Licensed under the MIT License.
4+
35
'use strict';
6+
47
import '../../common/extensions';
58

69
import { inject, injectable } from 'inversify';
710

11+
import { ProgressLocation, ProgressOptions } from 'vscode';
812
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
913
import { PYTHON_WARNINGS } from '../../common/constants';
1014
import { IPlatformService } from '../../common/platform/types';
@@ -33,7 +37,6 @@ import {
3337
import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda';
3438
import { StopWatch } from '../../common/utils/stopWatch';
3539
import { Interpreters } from '../../common/utils/localize';
36-
import { ProgressLocation, ProgressOptions } from 'vscode';
3740
import { TerminalAutoActivation } from '../../common/experiments/groups';
3841
import { IExtensionSingleActivationService } from '../../activation/types';
3942

@@ -55,6 +58,8 @@ const condaRetryMessages = [
5558
'The directory is not empty',
5659
];
5760

61+
type EnvironmentVariables = { [key: string]: string | undefined };
62+
5863
/**
5964
* This class exists so that the environment variable fetching can be cached in between tests. Normally
6065
* this cache resides in memory for the duration of the EnvironmentActivationService's lifetime, but in the case
@@ -63,39 +68,43 @@ const condaRetryMessages = [
6368
*/
6469
export class EnvironmentActivationServiceCache {
6570
private static useStatic = false;
71+
6672
private static staticMap = new Map<string, InMemoryCache<NodeJS.ProcessEnv | undefined>>();
73+
6774
private normalMap = new Map<string, InMemoryCache<NodeJS.ProcessEnv | undefined>>();
6875

69-
public static forceUseStatic() {
76+
public static forceUseStatic(): void {
7077
EnvironmentActivationServiceCache.useStatic = true;
7178
}
72-
public static forceUseNormal() {
79+
80+
public static forceUseNormal(): void {
7381
EnvironmentActivationServiceCache.useStatic = false;
7482
}
83+
7584
public get(key: string): InMemoryCache<NodeJS.ProcessEnv | undefined> | undefined {
7685
if (EnvironmentActivationServiceCache.useStatic) {
7786
return EnvironmentActivationServiceCache.staticMap.get(key);
7887
}
7988
return this.normalMap.get(key);
8089
}
8190

82-
public set(key: string, value: InMemoryCache<NodeJS.ProcessEnv | undefined>) {
91+
public set(key: string, value: InMemoryCache<NodeJS.ProcessEnv | undefined>): void {
8392
if (EnvironmentActivationServiceCache.useStatic) {
8493
EnvironmentActivationServiceCache.staticMap.set(key, value);
8594
} else {
8695
this.normalMap.set(key, value);
8796
}
8897
}
8998

90-
public delete(key: string) {
99+
public delete(key: string): void {
91100
if (EnvironmentActivationServiceCache.useStatic) {
92101
EnvironmentActivationServiceCache.staticMap.delete(key);
93102
} else {
94103
this.normalMap.delete(key);
95104
}
96105
}
97106

98-
public clear() {
107+
public clear(): void {
99108
// Don't clear during a test as the environment isn't going to change
100109
if (!EnvironmentActivationServiceCache.useStatic) {
101110
this.normalMap.clear();
@@ -110,10 +119,15 @@ export class EnvironmentActivationService
110119
untrustedWorkspace: false,
111120
virtualWorkspace: false,
112121
};
122+
113123
private readonly disposables: IDisposable[] = [];
124+
114125
private deferred: Deferred<void> | undefined;
115-
private previousEnvVars = process.env;
126+
127+
private previousEnvVars = normCaseKeys(process.env);
128+
116129
private readonly activatedEnvVariablesCache = new EnvironmentActivationServiceCache();
130+
117131
constructor(
118132
@inject(ITerminalHelper) private readonly helper: ITerminalHelper,
119133
@inject(IPlatformService) private readonly platform: IPlatformService,
@@ -164,6 +178,7 @@ export class EnvironmentActivationService
164178
public dispose(): void {
165179
this.disposables.forEach((d) => d.dispose());
166180
}
181+
167182
@traceDecoratorVerbose('getActivatedEnvironmentVariables', TraceOptions.Arguments)
168183
public async getActivatedEnvironmentVariables(
169184
resource: Resource,
@@ -203,6 +218,7 @@ export class EnvironmentActivationService
203218
throw ex;
204219
});
205220
}
221+
206222
public async getEnvironmentActivationShellCommands(
207223
resource: Resource,
208224
interpreter?: PythonEnvironment,
@@ -213,18 +229,19 @@ export class EnvironmentActivationService
213229
}
214230
return this.helper.getEnvironmentActivationShellCommands(resource, shellInfo.shellType, interpreter);
215231
}
232+
216233
public async getActivatedEnvironmentVariablesImpl(
217234
resource: Resource,
218235
interpreter?: PythonEnvironment,
219236
allowExceptions?: boolean,
220237
): Promise<NodeJS.ProcessEnv | undefined> {
221238
const shellInfo = defaultShells[this.platform.osType];
222239
if (!shellInfo) {
223-
return;
240+
return undefined;
224241
}
225242
try {
226243
let command: string | undefined;
227-
let [args, parse] = internalScripts.printEnvVariables();
244+
const [args, parse] = internalScripts.printEnvVariables();
228245
args.forEach((arg, i) => {
229246
args[i] = arg.toCommandArgumentForPythonExt();
230247
});
@@ -247,10 +264,10 @@ export class EnvironmentActivationService
247264
);
248265
traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`);
249266
if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
250-
return;
267+
return undefined;
251268
}
252269
// Run the activate command collect the environment from it.
253-
const activationCommand = this.fixActivationCommands(activationCommands).join(' && ');
270+
const activationCommand = fixActivationCommands(activationCommands).join(' && ');
254271
// In order to make sure we know where the environment output is,
255272
// put in a dummy echo we can look for
256273
command = `${activationCommand} && echo '${ENVIRONMENT_PREFIX}' && python ${args.join(' ')}`;
@@ -342,15 +359,13 @@ export class EnvironmentActivationService
342359
throw e;
343360
}
344361
}
362+
return undefined;
345363
}
346364

347-
protected fixActivationCommands(commands: string[]): string[] {
348-
// Replace 'source ' with '. ' as that works in shell exec
349-
return commands.map((cmd) => cmd.replace(/^source\s+/, '. '));
350-
}
351365
@traceDecoratorError('Failed to parse Environment variables')
352366
@traceDecoratorVerbose('parseEnvironmentOutput', TraceOptions.None)
353-
protected parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) {
367+
// eslint-disable-next-line class-methods-use-this
368+
private parseEnvironmentOutput(output: string, parse: (out: string) => NodeJS.ProcessEnv | undefined) {
354369
if (output.indexOf(ENVIRONMENT_PREFIX) === -1) {
355370
return parse(output);
356371
}
@@ -363,24 +378,31 @@ export class EnvironmentActivationService
363378
const env = await this.getActivatedEnvironmentVariables(resource);
364379
if (!env) {
365380
this.context.environmentVariableCollection.clear();
366-
this.previousEnvVars = process.env;
381+
this.previousEnvVars = normCaseKeys(process.env);
367382
return;
368383
}
369384
const previousEnv = this.previousEnvVars;
370-
Object.keys(previousEnv).forEach((key) => {
371-
// If the previous env var is not in the current env, then explicitly add it so it can cleared later.
372-
if (!(key in env)) {
373-
env[key] = '';
374-
}
375-
});
376385
this.previousEnvVars = env;
377-
for (const key in env) {
386+
Object.keys(env).forEach((key) => {
378387
const value = env[key];
379388
const prevValue = previousEnv[key];
380-
if (value !== undefined && prevValue !== value) {
381-
this.context.environmentVariableCollection.replace(key, value);
389+
if (prevValue !== value) {
390+
if (value !== undefined) {
391+
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
392+
this.context.environmentVariableCollection.replace(key, value);
393+
} else {
394+
traceVerbose(`Clearing environment variable ${key} from collection`);
395+
this.context.environmentVariableCollection.delete(key);
396+
}
382397
}
383-
}
398+
});
399+
Object.keys(previousEnv).forEach((key) => {
400+
// If the previous env var is not in the current env, clear it from collection.
401+
if (!(key in env)) {
402+
traceVerbose(`Clearing environment variable ${key} from collection`);
403+
this.context.environmentVariableCollection.delete(key);
404+
}
405+
});
384406
}
385407

386408
@traceDecoratorVerbose('Display activating terminals')
@@ -409,3 +431,19 @@ export class EnvironmentActivationService
409431
});
410432
}
411433
}
434+
435+
function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
436+
const result: EnvironmentVariables = {};
437+
Object.keys(env).forEach((key) => {
438+
// `os.environ` script used to get env vars normalizes keys to upper case:
439+
// https://github.com/python/cpython/issues/101754
440+
// So convert `process.env` keys to upper case to match.
441+
result[key.toUpperCase()] = env[key];
442+
});
443+
return result;
444+
}
445+
446+
function fixActivationCommands(commands: string[]): string[] {
447+
// Replace 'source ' with '. ' as that works in shell exec
448+
return commands.map((cmd) => cmd.replace(/^source\s+/, '. '));
449+
}

0 commit comments

Comments
 (0)