Skip to content
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

Leave pipenv in a corner until the user decides to select an interpreter #11654

Merged
2 changes: 1 addition & 1 deletion src/client/activation/activationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);

// Get latest interpreter list in the background.
this.interpreterService.getInterpreters(resource, { onActivation: true }).ignoreErrors();
this.interpreterService.getInterpreters(resource).ignoreErrors();

await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
return [];
}

const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
const interpreters = await this.interpreterService.getInterpreters(resource);
if (interpreters.filter((i) => !this.helper.isMacDefaultPythonPath(i.path)).length === 0) {
return [
new InvalidMacPythonInterpreterDiagnostic(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,16 @@

'use strict';

import { inject, injectable } from 'inversify';
import { inject, injectable, named } from 'inversify';
import { Uri } from 'vscode';
import '../../../common/extensions';
import { IInterpreterService, InterpreterType, IPipEnvService } from '../../../interpreter/contracts';
import {
IInterpreterLocatorService,
IInterpreterService,
InterpreterType,
IPipEnvService,
PIPENV_SERVICE
} from '../../../interpreter/contracts';
import { IWorkspaceService } from '../../application/types';
import { IFileSystem } from '../../platform/types';
import { ITerminalActivationCommandProvider, TerminalShellType } from '../types';
Expand All @@ -15,7 +21,9 @@ import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'
export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider {
constructor(
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IPipEnvService) private readonly pipenvService: IPipEnvService,
@inject(IInterpreterLocatorService)
@named(PIPENV_SERVICE)
private readonly pipenvService: IPipEnvService,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IFileSystem) private readonly fs: IFileSystem
) {}
Expand Down
2 changes: 1 addition & 1 deletion src/client/interpreter/autoSelection/rules/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class SystemWideInterpretersAutoSelectionRule extends BaseRuleService {
resource: Resource,
manager?: IInterpreterAutoSelectionService
): Promise<NextAction> {
const interpreters = await this.interpreterService.getInterpreters(resource, { onActivation: true });
const interpreters = await this.interpreterService.getInterpreters(resource);
// Exclude non-local interpreters.
const filteredInterpreters = interpreters.filter(
(int) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class InterpreterSelector implements IInterpreterSelector {
}

public async getSuggestions(resource: Resource) {
let interpreters = await this.interpreterManager.getInterpreters(resource);
let interpreters = await this.interpreterManager.getInterpreters(resource, { onSuggestion: true });
if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) {
interpreters = interpreters.filter((item) => this.interpreterSecurityService.isSafe(item) !== false);
}
Expand Down
5 changes: 3 additions & 2 deletions src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface IVirtualEnvironmentsSearchPathProvider {
}

export type GetInterpreterOptions = {
onActivation?: boolean;
onSuggestion?: boolean;
};

export type GetInterpreterLocatorOptions = GetInterpreterOptions & { ignoreCache?: boolean };
Expand All @@ -38,6 +38,7 @@ export const IInterpreterLocatorService = Symbol('IInterpreterLocatorService');
export interface IInterpreterLocatorService extends Disposable {
readonly onLocating: Event<Promise<PythonInterpreter[]>>;
readonly hasInterpreters: Promise<boolean>;
didTriggerInterpreterSuggestions?: boolean;
getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise<PythonInterpreter[]>;
}

Expand Down Expand Up @@ -126,7 +127,7 @@ export interface IInterpreterHelper {
}

export const IPipEnvService = Symbol('IPipEnvService');
export interface IPipEnvService {
export interface IPipEnvService extends IInterpreterLocatorService {
executable: string;
isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean>;
}
Expand Down
21 changes: 17 additions & 4 deletions src/client/interpreter/locators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ const flatten = require('lodash/flatten') as typeof import('lodash/flatten');
*/
@injectable()
export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
public didTriggerInterpreterSuggestions: boolean;

private readonly disposables: Disposable[] = [];
private readonly platform: IPlatformService;
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
private readonly _hasInterpreters: Deferred<boolean>;

constructor(
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
@inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter
Expand All @@ -42,6 +45,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
this.didTriggerInterpreterSuggestions = false;
}
/**
* This class should never emit events when we're locating.
Expand Down Expand Up @@ -106,18 +110,27 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
// The order is important because the data sources at the bottom of the list do not contain all,
// the information about the interpreters (e.g. type, environment name, etc).
// This way, the items returned from the top of the list will win, when we combine the items returned.
const keys: [string | undefined, OSType | undefined][] = [
const keys: [string, OSType | undefined][] = [
[WINDOWS_REGISTRY_SERVICE, OSType.Windows],
[CONDA_ENV_SERVICE, undefined],
[CONDA_ENV_FILE_SERVICE, undefined],
options?.onActivation ? [undefined, undefined] : [PIPENV_SERVICE, undefined],
[PIPENV_SERVICE, undefined],
[GLOBAL_VIRTUAL_ENV_SERVICE, undefined],
[WORKSPACE_VIRTUAL_ENV_SERVICE, undefined],
[KNOWN_PATH_SERVICE, undefined],
[CURRENT_PATH_SERVICE, undefined]
];
return keys
.filter((item) => item[0] !== undefined && (item[1] === undefined || item[1] === this.platform.osType))

const locators = keys
.filter((item) => item[1] === undefined || item[1] === this.platform.osType)
.map((item) => this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, item[0]));

// Set it to true the first time the user selects an interpreter
if (!this.didTriggerInterpreterSuggestions && options?.onSuggestion === true) {
this.didTriggerInterpreterSuggestions = true;
locators.forEach((locator) => (locator.didTriggerInterpreterSuggestions = true));
}

return locators;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,24 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
private readonly handlersAddedToResource = new Set<string>();
private readonly cacheKeyPrefix: string;
private readonly locating = new EventEmitter<Promise<PythonInterpreter[]>>();
private _didTriggerInterpreterSuggestions: boolean;

constructor(
@unmanaged() private readonly name: string,
@unmanaged() protected readonly serviceContainer: IServiceContainer,
@unmanaged() private cachePerWorkspace: boolean = false
) {
this._hasInterpreters = createDeferred<boolean>();
this.cacheKeyPrefix = `INTERPRETERS_CACHE_v3_${name}`;
this._didTriggerInterpreterSuggestions = false;
}

public get didTriggerInterpreterSuggestions(): boolean {
return this._didTriggerInterpreterSuggestions;
}

public set didTriggerInterpreterSuggestions(value: boolean) {
this._didTriggerInterpreterSuggestions = value;
}

public get onLocating(): Event<Promise<PythonInterpreter[]>> {
Expand Down
17 changes: 16 additions & 1 deletion src/client/interpreter/locators/services/pipEnvService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,15 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
this.configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
this.pipEnvServiceHelper = this.serviceContainer.get<IPipEnvServiceHelper>(IPipEnvServiceHelper);
}

// tslint:disable-next-line:no-empty
public dispose() {}

public async isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean> {
if (!this.didTriggerInterpreterSuggestions) {
return false;
}

// In PipEnv, the name of the cwd is used as a prefix in the virtual env.
if (pythonPath.indexOf(`${path.sep}${path.basename(dir)}-`) === -1) {
return false;
Expand All @@ -55,10 +61,14 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
}

public get executable(): string {
return this.configService.getSettings().pipenvPath;
return this.didTriggerInterpreterSuggestions ? this.configService.getSettings().pipenvPath : '';
}

public async getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise<PythonInterpreter[]> {
if (!this.didTriggerInterpreterSuggestions) {
return [];
}

const stopwatch = new StopWatch();
const startDiscoveryTime = stopwatch.elapsedTime;

Expand All @@ -71,6 +81,10 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
}

protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
if (!this.didTriggerInterpreterSuggestions) {
return Promise.resolve([]);
}

const pipenvCwd = this.getPipenvWorkingDirectory(resource);
if (!pipenvCwd) {
return Promise.resolve([]);
Expand Down Expand Up @@ -146,6 +160,7 @@ export class PipEnvService extends CacheableLocatorService implements IPipEnvSer
}
}
}

private async checkIfPipFileExists(cwd: string): Promise<boolean> {
const currentProcess = this.serviceContainer.get<ICurrentProcess>(ICurrentProcess);
const pipFileName = currentProcess.env[pipEnvFileNameVariable];
Expand Down
2 changes: 0 additions & 2 deletions src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import {
IInterpreterWatcherBuilder,
IKnownSearchPathsForInterpreters,
INTERPRETER_LOCATOR_SERVICE,
IPipEnvService,
IShebangCodeLensProvider,
IVirtualEnvironmentsSearchPathProvider,
KNOWN_PATH_SERVICE,
Expand Down Expand Up @@ -200,7 +199,6 @@ export function registerInterpreterTypes(serviceManager: IServiceManager) {
WORKSPACE_VIRTUAL_ENV_SERVICE
);
serviceManager.addSingleton<IInterpreterLocatorService>(IInterpreterLocatorService, PipEnvService, PIPENV_SERVICE);
serviceManager.addSingleton<IInterpreterLocatorService>(IPipEnvService, PipEnvService);

serviceManager.addSingleton<IInterpreterLocatorService>(
IInterpreterLocatorService,
Expand Down
8 changes: 6 additions & 2 deletions src/client/interpreter/virtualEnvs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ICurrentProcess, IPathUtils } from '../../common/types';
import { getNamesAndValues } from '../../common/utils/enum';
import { noop } from '../../common/utils/misc';
import { IServiceContainer } from '../../ioc/types';
import { InterpreterType, IPipEnvService } from '../contracts';
import { IInterpreterLocatorService, InterpreterType, IPipEnvService, PIPENV_SERVICE } from '../contracts';
import { IVirtualEnvironmentManager } from './types';

const PYENVFILES = ['pyvenv.cfg', path.join('..', 'pyvenv.cfg')];
Expand All @@ -27,7 +27,10 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager {
constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) {
this.processServiceFactory = serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
this.fs = serviceContainer.get<IFileSystem>(IFileSystem);
this.pipEnvService = serviceContainer.get<IPipEnvService>(IPipEnvService);
this.pipEnvService = serviceContainer.get<IInterpreterLocatorService>(
IInterpreterLocatorService,
PIPENV_SERVICE
) as IPipEnvService;
this.workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
}
public async getEnvironmentName(pythonPath: string, resource?: Uri): Promise<string> {
Expand All @@ -37,6 +40,7 @@ export class VirtualEnvironmentManager implements IVirtualEnvironmentManager {
const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined;
const workspaceUri = workspaceFolder ? workspaceFolder.uri : defaultWorkspaceUri;
const grandParentDirName = path.basename(path.dirname(path.dirname(pythonPath)));

if (workspaceUri && (await this.pipEnvService.isRelatedPipEnvironment(workspaceUri.fsPath, pythonPath))) {
// In pipenv, return the folder name of the workspace.
return path.basename(workspaceUri.fsPath);
Expand Down
4 changes: 1 addition & 3 deletions src/client/startupTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer):
.then((ver) => (ver ? ver.raw : ''))
.catch<string>(() => ''),
interpreterService.getActiveInterpreter().catch<PythonInterpreter | undefined>(() => undefined),
interpreterService
.getInterpreters(mainWorkspaceUri, { onActivation: true })
.catch<PythonInterpreter[]>(() => [])
interpreterService.getInterpreters(mainWorkspaceUri).catch<PythonInterpreter[]>(() => [])
]);
const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0;
const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined;
Expand Down
8 changes: 4 additions & 4 deletions src/test/activation/activationManager.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ suite('Activation Manager', () => {
when(workspaceService.getWorkspaceFolder(resource)).thenReturn(folder2);
when(activationService1.activate(resource)).thenResolve();
when(activationService2.activate(resource)).thenResolve();
when(interpreterService.getInterpreters(anything(), anything())).thenResolve();
when(interpreterService.getInterpreters(anything())).thenResolve();
autoSelection
.setup((a) => a.autoSelectInterpreter(resource))
.returns(() => Promise.resolve())
Expand Down Expand Up @@ -232,7 +232,7 @@ suite('Activation Manager', () => {
const resource = Uri.parse('two');
when(activationService1.activate(resource)).thenResolve();
when(activationService2.activate(resource)).thenResolve();
when(interpreterService.getInterpreters(anything(), anything())).thenResolve();
when(interpreterService.getInterpreters(anything())).thenResolve();
autoSelection
.setup((a) => a.autoSelectInterpreter(resource))
.returns(() => Promise.resolve())
Expand All @@ -252,7 +252,7 @@ suite('Activation Manager', () => {
const resource = Uri.parse('two');
when(activationService1.activate(resource)).thenResolve();
when(activationService2.activate(resource)).thenResolve();
when(interpreterService.getInterpreters(anything(), anything())).thenResolve();
when(interpreterService.getInterpreters(anything())).thenResolve();
when(experiments.inExperiment(DeprecatePythonPath.experiment)).thenReturn(true);
interpreterPathService
.setup((i) => i.copyOldInterpreterStorageValuesToNew(resource))
Expand All @@ -278,7 +278,7 @@ suite('Activation Manager', () => {
const resource = Uri.parse('two');
when(activationService1.activate(resource)).thenResolve();
when(activationService2.activate(resource)).thenResolve();
when(interpreterService.getInterpreters(anything(), anything())).thenResolve();
when(interpreterService.getInterpreters(anything())).thenResolve();
autoSelection
.setup((a) => a.autoSelectInterpreter(resource))
.returns(() => Promise.resolve())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
.returns(() => Promise.resolve(true))
.verifiable(typemoq.Times.once());
interpreterService
.setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true }))
.setup((i) => i.getInterpreters(typemoq.It.isAny()))
.returns(() => Promise.resolve([{} as any]))
.verifiable(typemoq.Times.never());
interpreterService
Expand Down Expand Up @@ -204,7 +204,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
.returns(() => Promise.resolve(true))
.verifiable(typemoq.Times.once());
interpreterService
.setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true }))
.setup((i) => i.getInterpreters(typemoq.It.isAny()))
.returns(() => Promise.resolve([{} as any]))
.verifiable(typemoq.Times.never());
interpreterService
Expand Down Expand Up @@ -235,7 +235,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
.returns(() => false)
.verifiable(typemoq.Times.once());
interpreterService
.setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true }))
.setup((i) => i.getInterpreters(typemoq.It.isAny()))
.returns(() => Promise.resolve([{ path: pythonPath } as any, { path: pythonPath } as any]))
.verifiable(typemoq.Times.once());
interpreterService
Expand Down Expand Up @@ -275,7 +275,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
.returns(() => false)
.verifiable(typemoq.Times.once());
interpreterService
.setup((i) => i.getInterpreters(typemoq.It.isAny(), { onActivation: true }))
.setup((i) => i.getInterpreters(typemoq.It.isAny()))
.returns(() =>
Promise.resolve([
{ path: pythonPath } as any,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ suite('Interpreters - selector', () => {
return { ...info, ...item };
});
interpreterService
.setup((x) => x.getInterpreters(TypeMoq.It.isAny()))
.setup((x) => x.getInterpreters(TypeMoq.It.isAny(), { onSuggestion: true }))
.returns(() => new Promise((resolve) => resolve(initial)));

const actual = await selector.getSuggestions(undefined);
Expand Down Expand Up @@ -139,7 +139,9 @@ suite('Interpreters - selector', () => {
test('When in Deprecate PythonPath experiment, remove unsafe interpreters from the suggested interpreters list', async () => {
// tslint:disable-next-line: no-any
const interpreterList = ['interpreter1', 'interpreter2', 'interpreter3'] as any;
interpreterService.setup((i) => i.getInterpreters(folder1.uri)).returns(() => interpreterList);
interpreterService
.setup((i) => i.getInterpreters(folder1.uri, { onSuggestion: true }))
.returns(() => interpreterList);
// tslint:disable-next-line: no-any
interpreterSecurityService.setup((i) => i.isSafe('interpreter1' as any)).returns(() => true);
// tslint:disable-next-line: no-any
Expand Down
Loading