Skip to content

Fix issue with sys.prefix when getting environment details. #16386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
Comment on lines +207 to +213

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karthiknadig I don't think #16355 (comment) holds true anymore as we do not return from cache if it does not have complete info:

public getCompleteInfo(path: string): PythonEnvInfo | undefined {
// `path` can either be path to environment or executable path
let env = this.envs.find((e) => arePathsSame(e.location, path));
if (env?.hasCompleteInfo) {
return env;
}
env = this.envs.find((e) => areSameEnv(e, path));
return env?.hasCompleteInfo ? env : undefined;
}

So I'm going to remove this, please correct me if I'm wrong.

}
}
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