Skip to content

Commit

Permalink
Refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj committed Feb 25, 2023
1 parent 02fba08 commit 8e3908c
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 130 deletions.
130 changes: 4 additions & 126 deletions src/client/interpreter/activation/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ import '../../common/extensions';

import { inject, injectable } from 'inversify';

import { ProgressLocation, ProgressOptions } from 'vscode';
import { IApplicationEnvironment, IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { IWorkspaceService } from '../../common/application/types';
import { PYTHON_WARNINGS } from '../../common/constants';
import { IPlatformService } from '../../common/platform/types';
import * as internalScripts from '../../common/process/internal/scripts';
import { ExecutionResult, IProcessServiceFactory } from '../../common/process/types';
import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types';
import { ICurrentProcess, IDisposable, IExperimentService, IExtensionContext, Resource } from '../../common/types';
import { createDeferred, Deferred, sleep } from '../../common/utils/async';
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';
Expand All @@ -36,9 +35,6 @@ import {
} from '../../logging';
import { Conda } from '../../pythonEnvironments/common/environmentManagers/conda';
import { StopWatch } from '../../common/utils/stopWatch';
import { Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';

const ENVIRONMENT_PREFIX = 'e8b39361-0157-4923-80e1-22d70d46dee6';
Expand All @@ -59,8 +55,6 @@ const condaRetryMessages = [
'The directory is not empty',
];

type EnvironmentVariables = { [key: string]: string | undefined };

/**
* This class exists so that the environment variable fetching can be cached in between tests. Normally
* this cache resides in memory for the duration of the EnvironmentActivationService's lifetime, but in the case
Expand Down Expand Up @@ -114,19 +108,14 @@ export class EnvironmentActivationServiceCache {
}

@injectable()
export class EnvironmentActivationService
implements IEnvironmentActivationService, IExtensionSingleActivationService, IDisposable {
export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable {
public readonly supportedWorkspaceTypes: { untrustedWorkspace: boolean; virtualWorkspace: boolean } = {
untrustedWorkspace: false,
virtualWorkspace: false,
};

private readonly disposables: IDisposable[] = [];

private deferred: Deferred<void> | undefined;

private previousEnvVars = normCaseKeys(process.env);

private readonly activatedEnvVariablesCache = new EnvironmentActivationServiceCache();

constructor(
Expand All @@ -137,10 +126,6 @@ export class EnvironmentActivationService
@inject(IWorkspaceService) private workspace: IWorkspaceService,
@inject(IInterpreterService) private interpreterService: IInterpreterService,
@inject(IEnvironmentVariablesProvider) private readonly envVarsService: IEnvironmentVariablesProvider,
@inject(IExtensionContext) private context: IExtensionContext,
@inject(IApplicationShell) private shell: IApplicationShell,
@inject(IExperimentService) private experimentService: IExperimentService,
@inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment,
) {
this.envVarsService.onDidEnvironmentVariablesChange(
() => this.activatedEnvVariablesCache.clear(),
Expand All @@ -149,38 +134,6 @@ export class EnvironmentActivationService
);
}

public async activate(): Promise<void> {
if (!this.isEnvCollectionEnabled()) {
this.context.environmentVariableCollection.clear();
return;
}
this.interpreterService.onDidChangeInterpreter(
async (resource) => {
this.showProgress();
this.activatedEnvVariablesCache.clear();
await this.applyCollection(resource);
this.hideProgress();
},
this,
this.disposables,
);
this.applicationEnvironment.onDidChangeShell(
async (shell: string) => {
// 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);
},
this,
this.disposables,
);

this.applyCollection(undefined).ignoreErrors();
}

private isEnvCollectionEnabled() {
return inTerminalEnvVarExperiment(this.experimentService);
}

public dispose(): void {
this.disposables.forEach((d) => d.dispose());
}
Expand Down Expand Up @@ -385,81 +338,6 @@ export class EnvironmentActivationService
const js = output.substring(output.indexOf('{')).trim();
return parse(js);
}

private async applyCollection(resource: Resource, shell = this.applicationEnvironment.shell) {
const env = await this.getActivatedEnvironmentVariables(resource, undefined, undefined, shell);
if (!env) {
const shellType = identifyShellFromShellPath(shell);
if (defaultShells[this.platform.osType]?.shellType !== shellType) {
// Commands to fetch env vars may fail in custom shells due to unknown reasons, in that case
// fallback to default shells as they are known to work better.
await this.applyCollection(resource);
return;
}
this.context.environmentVariableCollection.clear();
this.previousEnvVars = normCaseKeys(process.env);
return;
}
const previousEnv = this.previousEnvVars;
this.previousEnvVars = env;
Object.keys(env).forEach((key) => {
const value = env[key];
const prevValue = previousEnv[key];
if (prevValue !== value) {
if (value !== undefined) {
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
this.context.environmentVariableCollection.replace(key, value);
} else {
traceVerbose(`Clearing environment variable ${key} from collection`);
this.context.environmentVariableCollection.delete(key);
}
}
});
Object.keys(previousEnv).forEach((key) => {
// 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`);
this.context.environmentVariableCollection.delete(key);
}
});
}

@traceDecoratorVerbose('Display activating terminals')
private showProgress(): void {
if (!this.deferred) {
this.createProgress();
}
}

@traceDecoratorVerbose('Hide activating terminals')
private hideProgress(): void {
if (this.deferred) {
this.deferred.resolve();
this.deferred = undefined;
}
}

private createProgress() {
const progressOptions: ProgressOptions = {
location: ProgressLocation.Window,
title: Interpreters.activatingTerminals,
};
this.shell.withProgress(progressOptions, () => {
this.deferred = createDeferred();
return this.deferred.promise;
});
}
}

function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
const result: EnvironmentVariables = {};
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 fixActivationCommands(commands: string[]): string[] {
Expand Down
150 changes: 150 additions & 0 deletions src/client/interpreter/activation/terminalEnvVarCollectionService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { ProgressOptions, ProgressLocation } from 'vscode';
import { IExtensionSingleActivationService } from '../../activation/types';
import { IApplicationShell, IApplicationEnvironment } from '../../common/application/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { IPlatformService } from '../../common/platform/types';
import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';
import { IExtensionContext, IExperimentService, Resource, IDisposableRegistry } from '../../common/types';
import { Deferred, createDeferred } from '../../common/utils/async';
import { Interpreters } from '../../common/utils/localize';
import { EnvironmentVariables } from '../../common/variables/types';
import { traceDecoratorVerbose, traceVerbose } from '../../logging';
import { IInterpreterService } from '../contracts';
import { defaultShells } from './service';
import { IEnvironmentActivationService } from './types';

@injectable()
export class TerminalEnvVarCollectionService implements IExtensionSingleActivationService {
public readonly supportedWorkspaceTypes: { untrustedWorkspace: boolean; virtualWorkspace: boolean } = {
untrustedWorkspace: false,
virtualWorkspace: false,
};

private deferred: Deferred<void> | undefined;

private previousEnvVars = normCaseKeys(process.env);

constructor(
@inject(IPlatformService) private readonly platform: IPlatformService,
@inject(IInterpreterService) private interpreterService: IInterpreterService,
@inject(IExtensionContext) private context: IExtensionContext,
@inject(IApplicationShell) private shell: IApplicationShell,
@inject(IExperimentService) private experimentService: IExperimentService,
@inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment,
@inject(IDisposableRegistry) private disposables: IDisposableRegistry,
@inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService,
) {}

public async activate(): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
this.context.environmentVariableCollection.clear();
return;
}
this.interpreterService.onDidChangeInterpreter(
async (resource) => {
this.showProgress();
await this.applyCollection(resource);
this.hideProgress();
},
this,
this.disposables,
);
this.applicationEnvironment.onDidChangeShell(
async (shell: string) => {
this.showProgress();
// 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);
this.hideProgress();
},
this,
this.disposables,
);

this.applyCollection(undefined).ignoreErrors();
}

private async applyCollection(resource: Resource, shell = this.applicationEnvironment.shell) {
const env = await this.environmentActivationService.getActivatedEnvironmentVariables(
resource,
undefined,
undefined,
shell,
);
if (!env) {
const shellType = identifyShellFromShellPath(shell);
if (defaultShells[this.platform.osType]?.shellType !== shellType) {
// Commands to fetch env vars may fail in custom shells due to unknown reasons, in that case
// fallback to default shells as they are known to work better.
await this.applyCollection(resource);
return;
}
this.context.environmentVariableCollection.clear();
this.previousEnvVars = normCaseKeys(process.env);
return;
}
const previousEnv = this.previousEnvVars;
this.previousEnvVars = env;
Object.keys(env).forEach((key) => {
const value = env[key];
const prevValue = previousEnv[key];
if (prevValue !== value) {
if (value !== undefined) {
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
this.context.environmentVariableCollection.replace(key, value);
} else {
traceVerbose(`Clearing environment variable ${key} from collection`);
this.context.environmentVariableCollection.delete(key);
}
}
});
Object.keys(previousEnv).forEach((key) => {
// 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`);
this.context.environmentVariableCollection.delete(key);
}
});
}

@traceDecoratorVerbose('Display activating terminals')
private showProgress(): void {
if (!this.deferred) {
this.createProgress();
}
}

@traceDecoratorVerbose('Hide activating terminals')
private hideProgress(): void {
if (this.deferred) {
this.deferred.resolve();
this.deferred = undefined;
}
}

private createProgress() {
const progressOptions: ProgressOptions = {
location: ProgressLocation.Window,
title: Interpreters.activatingTerminals,
};
this.shell.withProgress(progressOptions, () => {
this.deferred = createDeferred();
return this.deferred.promise;
});
}
}

function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
const result: EnvironmentVariables = {};
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;
}
1 change: 1 addition & 0 deletions src/client/interpreter/activation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface IEnvironmentActivationService {
resource: Resource,
interpreter?: PythonEnvironment,
allowExceptions?: boolean,
shell?: string,
): Promise<NodeJS.ProcessEnv | undefined>;
getEnvironmentActivationShellCommands(
resource: Resource,
Expand Down
6 changes: 5 additions & 1 deletion src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types';
import { IServiceManager } from '../ioc/types';
import { EnvironmentActivationService } from './activation/service';
import { TerminalEnvVarCollectionService } from './activation/terminalEnvVarCollectionService';
import { IEnvironmentActivationService } from './activation/types';
import { InterpreterAutoSelectionService } from './autoSelection/index';
import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy';
Expand Down Expand Up @@ -108,5 +109,8 @@ export function registerTypes(serviceManager: IServiceManager): void {
IEnvironmentActivationService,
EnvironmentActivationService,
);
serviceManager.addBinding(IEnvironmentActivationService, IExtensionSingleActivationService);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
TerminalEnvVarCollectionService,
);
}
6 changes: 3 additions & 3 deletions src/test/interpreters/activation/service.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { ProcessServiceFactory } from '../../../client/common/process/processFac
import { IProcessService, IProcessServiceFactory } from '../../../client/common/process/types';
import { TerminalHelper } from '../../../client/common/terminal/helper';
import { ITerminalHelper } from '../../../client/common/terminal/types';
import { ICurrentProcess } from '../../../client/common/types';
import { ICurrentProcess, Resource } from '../../../client/common/types';
import { getNamesAndValues } from '../../../client/common/utils/enum';
import { Architecture, OSType } from '../../../client/common/utils/platform';
import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider';
Expand Down Expand Up @@ -48,7 +48,7 @@ suite('Interpreters Activation - Python Environment Variables', () => {
let workspace: IWorkspaceService;
let interpreterService: IInterpreterService;
let onDidChangeEnvVariables: EventEmitter<Uri | undefined>;
let onDidChangeInterpreter: EventEmitter<void>;
let onDidChangeInterpreter: EventEmitter<Resource>;
const pythonInterpreter: PythonEnvironment = {
path: '/foo/bar/python.exe',
version: new SemVer('3.6.6-final'),
Expand All @@ -68,7 +68,7 @@ suite('Interpreters Activation - Python Environment Variables', () => {
interpreterService = mock(InterpreterService);
workspace = mock(WorkspaceService);
onDidChangeEnvVariables = new EventEmitter<Uri | undefined>();
onDidChangeInterpreter = new EventEmitter<void>();
onDidChangeInterpreter = new EventEmitter<Resource>();
when(envVarsService.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event);
when(interpreterService.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event);
when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter);
Expand Down

0 comments on commit 8e3908c

Please sign in to comment.