Skip to content

Cherry-pick pipenv changes and pythonpath prompt changes to release #11700

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 2 commits into from
May 8, 2020
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
6 changes: 3 additions & 3 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,9 @@
"DataScience.liveShareServiceFailure": "Failure starting '{0}' service during live share connection.",
"DataScience.documentMismatch": "Cannot run cells, duplicate documents for {0} found.",
"DataScience.pythonInteractiveCreateFailed": "Failure to create a 'Python Interactive' window. Try reinstalling the Python extension.",
"diagnostics.removePythonPathSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your settings.json.",
"diagnostics.removePythonPathCodeWorkspace": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your .code-workspace file.",
"diagnostics.removePythonPathCodeWorkspaceAndSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the \"python.pythonPath\" entry from your settings.json and .code-workspace file.",
"diagnostics.removePythonPathSettingsJson": "The setting \"python.pythonPath\" defined in your settings.json is now deprecated. Do you want us to delete it from your settings.json only? [Learn more](https://aka.ms/AA7jfor).",
"diagnostics.removePythonPathCodeWorkspace": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file only? [Learn more](https://aka.ms/AA7jfor).",
"diagnostics.removePythonPathCodeWorkspaceAndSettingsJson": "The setting \"python.pythonPath\" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file and settings.json? [Learn more](https://aka.ms/AA7jfor).",
"diagnostics.warnSourceMaps": "Source map support is enabled in the Python Extension, this will adversely impact performance of the extension.",
"diagnostics.disableSourceMaps": "Disable Source Map Support",
"diagnostics.warnBeforeEnablingSourceMaps": "Enabling source map support in the Python Extension will adversely impact performance of the extension.",
Expand Down
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 @@ -9,7 +9,6 @@ import { IWorkspaceService } from '../../../common/application/types';
import { DeprecatePythonPath } from '../../../common/experimentGroups';
import { IDisposableRegistry, IExperimentsManager, Resource } from '../../../common/types';
import { Common, Diagnostics } from '../../../common/utils/localize';
import { learnMoreOnInterpreterSecurityURI } from '../../../interpreter/autoSelection/constants';
import { IServiceContainer } from '../../../ioc/types';
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
import { IDiagnosticsCommandFactory } from '../commands/types';
Expand Down Expand Up @@ -97,13 +96,6 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic
{
prompt: Common.noIWillDoItLater()
},
{
prompt: Common.moreInfo(),
command: commandFactory.createCommand(diagnostic, {
type: 'launch',
options: learnMoreOnInterpreterSecurityURI
})
},
{
prompt: Common.doNotShowAgain(),
command: commandFactory.createCommand(diagnostic, { type: 'ignore', options: DiagnosticScope.Global })
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
6 changes: 3 additions & 3 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ export namespace Diagnostics {
);
export const removePythonPathSettingsJson = localize(
'diagnostics.removePythonPathSettingsJson',
'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your settings.json.'
'The setting "python.pythonPath" defined in your settings.json is now deprecated. Do you want us to delete it from your settings.json only? [Learn more](https://aka.ms/AA7jfor).'
);
export const removePythonPathCodeWorkspace = localize(
'diagnostics.removePythonPathCodeWorkspace',
'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your .code-workspace file.'
'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file only? [Learn more](https://aka.ms/AA7jfor).'
);
export const removePythonPathCodeWorkspaceAndSettingsJson = localize(
'diagnostics.removePythonPathCodeWorkspaceAndSettingsJson',
'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it? This will only remove the "python.pythonPath" entry from your settings.json and .code-workspace file.'
'The setting "python.pythonPath" defined in your workspace settings is now deprecated. Do you want us to delete it from your .code-workspace file and settings.json? [Learn more](https://aka.ms/AA7jfor).'
);
export const invalidPythonPathInDebuggerSettings = localize(
'diagnostics.invalidPythonPathInDebuggerSettings',
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
Loading