Skip to content

Commit

Permalink
Fix issue with sys.prefix when getting environment details. (microsof…
Browse files Browse the repository at this point in the history
…t#16386)

* Fix issue with sys.prefix when getting environment details.

* Fix typo

* Fix typo again
  • Loading branch information
karthiknadig authored Jun 5, 2021
1 parent 2411004 commit 547243f
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 17 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/16355.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue with sys.prefix when getting environment details.
7 changes: 3 additions & 4 deletions src/client/pythonEnvironments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { PosixKnownPathsLocator } from './discovery/locators/services/posixKnown
import { PyenvLocator } from './discovery/locators/services/pyenvLocator';
import { WindowsRegistryLocator } from './discovery/locators/services/windowsRegistryLocator';
import { WindowsStoreLocator } from './discovery/locators/services/windowsStoreLocator';
import { EnvironmentInfoService } from './info/environmentInfoService';
import { getEnvironmentInfoService } from './info/environmentInfoService';
import { isComponentEnabled, registerLegacyDiscoveryForIOC, registerNewDiscoveryForIOC } from './legacyIOC';
import { EnvironmentsSecurity, IEnvironmentsSecurity } from './security';
import { PoetryLocator } from './discovery/locators/services/poetryLocator';
Expand All @@ -37,7 +37,7 @@ export async function initialize(ext: ExtensionState): Promise<PythonEnvironment
const environmentsSecurity = new EnvironmentsSecurity();
const api = new PythonEnvironments(
() => createLocators(ext, environmentsSecurity),
// Other sub-commonents (e.g. config, "current" env) will go here.
// Other sub-components (e.g. config, "current" env) will go here.
);
ext.disposables.push(api);

Expand Down Expand Up @@ -94,8 +94,7 @@ async function createLocators(
);

// Create the env info service used by ResolvingLocator and CachingLocator.
const envInfoService = new EnvironmentInfoService();
ext.disposables.push(envInfoService);
const envInfoService = getEnvironmentInfoService(ext.disposables);

// Build the stack of composite locators.
locators = new PythonEnvsReducer(locators);
Expand Down
18 changes: 17 additions & 1 deletion src/client/pythonEnvironments/info/environmentInfoService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import { traceVerbose } from '../../common/logger';
import { IDisposableRegistry } from '../../common/types';
import { createDeferred, Deferred } from '../../common/utils/async';
import { createRunningWorkerPool, IWorkerPool, QueuePosition } from '../../common/utils/workerPool';
import { getInterpreterInfo, InterpreterInformation } from '../base/info/interpreter';
Expand Down Expand Up @@ -32,7 +33,7 @@ async function buildEnvironmentInfo(interpreterPath: string): Promise<Interprete
return interpreterInfo;
}

export class EnvironmentInfoService implements IEnvironmentInfoService {
class EnvironmentInfoService implements IEnvironmentInfoService {
// Caching environment here in-memory. This is so that we don't have to run this on the same
// path again and again in a given session. This information will likely not change in a given
// session. There are definitely cases where this will change. But a simple reload should address
Expand Down Expand Up @@ -84,3 +85,18 @@ export class EnvironmentInfoService implements IEnvironmentInfoService {
return !!(result && result.completed);
}
}

let envInfoService: IEnvironmentInfoService | undefined;
export function getEnvironmentInfoService(disposables?: IDisposableRegistry): IEnvironmentInfoService {
if (envInfoService === undefined) {
const service = new EnvironmentInfoService();
disposables?.push({
dispose: () => {
service.dispose();
envInfoService = undefined;
},
});
envInfoService = service;
}
return envInfoService;
}
16 changes: 13 additions & 3 deletions src/client/pythonEnvironments/legacyIOC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import { EnvironmentsSecurity, IEnvironmentsSecurity } from './security';
import { toSemverLikeVersion } from './base/info/pythonVersion';
import { PythonVersion } from './info/pythonVersion';
import { IExtensionSingleActivationService } from '../activation/types';
import { EnvironmentInfoServiceQueuePriority, getEnvironmentInfoService } from './info/environmentInfoService';

const convertedKinds = new Map(
Object.entries({
Expand Down Expand Up @@ -195,14 +196,23 @@ class ComponentAdapter implements IComponentAdapter {

// We use the same getInterpreters() here as for IInterpreterLocatorService.
public async getInterpreterDetails(pythonPath: string, resource?: vscode.Uri): Promise<PythonEnvironment> {
const info = buildEnvInfo({ executable: pythonPath });
const envInfo = buildEnvInfo({ executable: pythonPath });
if (resource !== undefined) {
const wsFolder = vscode.workspace.getWorkspaceFolder(resource);
if (wsFolder !== undefined) {
info.searchLocation = wsFolder.uri;
envInfo.searchLocation = wsFolder.uri;
}
}

const env = (await this.api.resolveEnv(envInfo)) ?? envInfo;
if (env.executable.sysPrefix) {
const execInfoService = getEnvironmentInfoService();
const info = await execInfoService.getEnvironmentInfo(pythonPath, EnvironmentInfoServiceQueuePriority.High);
if (info) {
env.executable.sysPrefix = info.executable.sysPrefix;
env.version = info.version;
}
}
const env = await this.api.resolveEnv(info);
return convertEnvInfo(env ?? buildEnvInfo({ executable: pythonPath }));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as sinon from 'sinon';
import { ImportMock } from 'ts-mock-imports';
import { EventEmitter } from 'vscode';
import { ExecutionResult } from '../../../../../client/common/process/types';
import { IDisposableRegistry } from '../../../../../client/common/types';
import { createDeferred } from '../../../../../client/common/utils/async';
import { Architecture } from '../../../../../client/common/utils/platform';
import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info';
Expand All @@ -17,19 +18,24 @@ import { PythonEnvsResolver } from '../../../../../client/pythonEnvironments/bas
import { getEnvs as getEnvsWithUpdates } from '../../../../../client/pythonEnvironments/base/locatorUtils';
import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher';
import * as ExternalDep from '../../../../../client/pythonEnvironments/common/externalDependencies';
import { EnvironmentInfoService } from '../../../../../client/pythonEnvironments/info/environmentInfoService';
import {
getEnvironmentInfoService,
IEnvironmentInfoService,
} from '../../../../../client/pythonEnvironments/info/environmentInfoService';
import { sleep } from '../../../../core';
import { createNamedEnv, getEnvs, SimpleLocator } from '../../common';

suite('Python envs locator - Environments Resolver', () => {
let envInfoService: EnvironmentInfoService;
let envInfoService: IEnvironmentInfoService;
let disposables: IDisposableRegistry;

setup(() => {
envInfoService = new EnvironmentInfoService();
disposables = [];
envInfoService = getEnvironmentInfoService(disposables);
});
teardown(() => {
sinon.restore();
envInfoService.dispose();
disposables.forEach((d) => d.dispose());
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@ import * as assert from 'assert';
import * as sinon from 'sinon';
import { ImportMock } from 'ts-mock-imports';
import { ExecutionResult } from '../../../client/common/process/types';
import { IDisposableRegistry } from '../../../client/common/types';
import { Architecture } from '../../../client/common/utils/platform';
import { InterpreterInformation } from '../../../client/pythonEnvironments/base/info/interpreter';
import { parseVersion } from '../../../client/pythonEnvironments/base/info/pythonVersion';
import * as ExternalDep from '../../../client/pythonEnvironments/common/externalDependencies';
import {
EnvironmentInfoService,
EnvironmentInfoServiceQueuePriority,
getEnvironmentInfoService,
} from '../../../client/pythonEnvironments/info/environmentInfoService';

suite('Environment Info Service', () => {
let stubShellExec: sinon.SinonStub;
let disposables: IDisposableRegistry;

function createExpectedEnvInfo(executable: string): InterpreterInformation {
return {
Expand All @@ -36,6 +38,7 @@ suite('Environment Info Service', () => {
}

setup(() => {
disposables = [];
stubShellExec = ImportMock.mockFunction(
ExternalDep,
'shellExecute',
Expand All @@ -49,9 +52,10 @@ suite('Environment Info Service', () => {
});
teardown(() => {
stubShellExec.restore();
disposables.forEach((d) => d.dispose());
});
test('Add items to queue and get results', async () => {
const envService = new EnvironmentInfoService();
const envService = getEnvironmentInfoService(disposables);
const promises: Promise<InterpreterInformation | undefined>[] = [];
const expected: InterpreterInformation[] = [];
for (let i = 0; i < 10; i = i + 1) {
Expand All @@ -74,7 +78,7 @@ suite('Environment Info Service', () => {
});

test('Add same item to queue', async () => {
const envService = new EnvironmentInfoService();
const envService = getEnvironmentInfoService(disposables);
const promises: Promise<InterpreterInformation | undefined>[] = [];
const expected: InterpreterInformation[] = [];

Expand All @@ -96,7 +100,7 @@ suite('Environment Info Service', () => {
});

test('isInfoProvided() returns true for items already processed', async () => {
const envService = new EnvironmentInfoService();
const envService = getEnvironmentInfoService(disposables);
let result: boolean;
const promises: Promise<InterpreterInformation | undefined>[] = [];
const path1 = 'any-path1';
Expand All @@ -113,7 +117,7 @@ suite('Environment Info Service', () => {
});

test('isInfoProvided() returns false otherwise', async () => {
const envService = new EnvironmentInfoService();
const envService = getEnvironmentInfoService(disposables);
const promises: Promise<InterpreterInformation | undefined>[] = [];
const path1 = 'any-path1';
const path2 = 'any-path2';
Expand Down

0 comments on commit 547243f

Please sign in to comment.