Skip to content

Commit 355f114

Browse files
DonJayamanneDonJayamanne
authored
Ensure workspace pipenv environment is not labeled as a virtual env (#2239)
Fixes #2223
1 parent 2c95ff8 commit 355f114

16 files changed

+490
-82
lines changed

‎news/2 Fixes/2223.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure workspace `pipenv` environment is not labeled as a `virtual env`.

‎src/client/common/installer/pythonInstallation.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
// Licensed under the MIT License.
33
'use strict';
44

5-
import { IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
6-
import { isMacDefaultPythonPath } from '../../interpreter/locators/helpers';
5+
import { IInterpreterHelper, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
76
import { IServiceContainer } from '../../ioc/types';
87
import { IApplicationShell } from '../application/types';
98
import { IPlatformService } from '../platform/types';
@@ -25,7 +24,8 @@ export class PythonInstaller {
2524
const interpreters = await this.locator.getInterpreters();
2625
if (interpreters.length > 0) {
2726
const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
28-
if (platform.isMac && isMacDefaultPythonPath(settings.pythonPath)) {
27+
const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
28+
if (platform.isMac && helper.isMacDefaultPythonPath(settings.pythonPath)) {
2929
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
3030
const interpreter = await interpreterService.getActiveInterpreter();
3131
if (interpreter && interpreter.type === InterpreterType.Unknown) {

‎src/client/interpreter/configuration/interpreterSelector.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri }
44
import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
55
import * as settings from '../../common/configSettings';
66
import { Commands } from '../../common/constants';
7-
import { IFileSystem } from '../../common/platform/types';
87
import { IServiceContainer } from '../../ioc/types';
98
import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts';
109
import { IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';
@@ -20,14 +19,12 @@ export class InterpreterSelector implements IInterpreterSelector {
2019
private readonly workspaceService: IWorkspaceService;
2120
private readonly applicationShell: IApplicationShell;
2221
private readonly documentManager: IDocumentManager;
23-
private readonly fileSystem: IFileSystem;
2422

2523
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
2624
this.interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
2725
this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
2826
this.applicationShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
2927
this.documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
30-
this.fileSystem = this.serviceContainer.get<IFileSystem>(IFileSystem);
3128

3229
const commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
3330
this.disposables.push(commandManager.registerCommand(Commands.Set_Interpreter, this.setInterpreter.bind(this)));
@@ -38,8 +35,7 @@ export class InterpreterSelector implements IInterpreterSelector {
3835
}
3936

4037
public async getSuggestions(resourceUri?: Uri) {
41-
let interpreters = await this.interpreterManager.getInterpreters(resourceUri);
42-
interpreters = await this.removeDuplicates(interpreters);
38+
const interpreters = await this.interpreterManager.getInterpreters(resourceUri);
4339
// tslint:disable-next-line:no-non-null-assertion
4440
interpreters.sort((a, b) => a.displayName! > b.displayName! ? 1 : -1);
4541
return Promise.all(interpreters.map(item => this.suggestionToQuickPickItem(item, resourceUri)));
@@ -74,17 +70,6 @@ export class InterpreterSelector implements IInterpreterSelector {
7470
};
7571
}
7672

77-
private async removeDuplicates(interpreters: PythonInterpreter[]): Promise<PythonInterpreter[]> {
78-
const result: PythonInterpreter[] = [];
79-
interpreters.forEach(x => {
80-
if (result.findIndex(a => a.displayName === x.displayName
81-
&& a.type === x.type && this.fileSystem.arePathsSame(path.dirname(a.path), path.dirname(x.path))) < 0) {
82-
result.push(x);
83-
}
84-
});
85-
return result;
86-
}
87-
8873
private async setInterpreter() {
8974
const setInterpreterGlobally = !Array.isArray(this.workspaceService.workspaceFolders) || this.workspaceService.workspaceFolders.length === 0;
9075
let configTarget = ConfigurationTarget.Global;

‎src/client/interpreter/contracts.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export interface IInterpreterService {
7878
getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
7979
autoSetInterpreter(): Promise<void>;
8080
getActiveInterpreter(resource?: Uri): Promise<PythonInterpreter | undefined>;
81-
getInterpreterDetails(pythonPath: string): Promise<Partial<PythonInterpreter>>;
81+
getInterpreterDetails(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
8282
refresh(): Promise<void>;
8383
initialize(): void;
8484
}
@@ -97,9 +97,15 @@ export const IInterpreterHelper = Symbol('IInterpreterHelper');
9797
export interface IInterpreterHelper {
9898
getActiveWorkspaceUri(): WorkspacePythonPath | undefined;
9999
getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
100+
isMacDefaultPythonPath(pythonPath: string): Boolean;
100101
}
101102

102103
export const IPipEnvService = Symbol('IPipEnvService');
103104
export interface IPipEnvService {
104105
isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean>;
105106
}
107+
108+
export const IInterpreterLocatorHelper = Symbol('IInterpreterLocatorHelper');
109+
export interface IInterpreterLocatorHelper {
110+
mergeInterpreters(interpreters: PythonInterpreter[]): PythonInterpreter[];
111+
}

‎src/client/interpreter/helpers.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { inject, injectable } from 'inversify';
22
import { ConfigurationTarget } from 'vscode';
33
import { IDocumentManager, IWorkspaceService } from '../common/application/types';
44
import { IFileSystem } from '../common/platform/types';
5-
import { IPythonExecutionFactory } from '../common/process/types';
5+
import { InterpreterInfomation, IPythonExecutionFactory } from '../common/process/types';
66
import { IPersistentStateFactory } from '../common/types';
77
import { IServiceContainer } from '../ioc/types';
88
import { IInterpreterHelper, PythonInterpreter, WorkspacePythonPath } from './contracts';
@@ -33,7 +33,7 @@ export class InterpreterHelper implements IInterpreterHelper {
3333
if (!workspaceService.hasWorkspaceFolders) {
3434
return;
3535
}
36-
if (workspaceService.workspaceFolders.length === 1) {
36+
if (Array.isArray(workspaceService.workspaceFolders) && workspaceService.workspaceFolders.length === 1) {
3737
return { folderUri: workspaceService.workspaceFolders[0].uri, configTarget: ConfigurationTarget.Workspace };
3838
}
3939
if (documentManager.activeTextEditor) {
@@ -44,15 +44,16 @@ export class InterpreterHelper implements IInterpreterHelper {
4444
}
4545
}
4646
public async getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>> {
47-
const fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
47+
let fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
48+
fileHash = fileHash ? fileHash : '';
4849
const store = this.persistentFactory.createGlobalPersistentState<CachedPythonInterpreter>(pythonPath, undefined, EXPITY_DURATION);
49-
if (store.value && store.value.fileHash === fileHash) {
50+
if (store.value && (!fileHash || store.value.fileHash === fileHash)) {
5051
return store.value;
5152
}
5253
const processService = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create({ pythonPath });
5354

5455
try {
55-
const info = await processService.getInterpreterInformation().catch(() => undefined);
56+
const info = await processService.getInterpreterInformation().catch<InterpreterInfomation | undefined>(() => undefined);
5657
if (!info) {
5758
return;
5859
}
@@ -67,4 +68,7 @@ export class InterpreterHelper implements IInterpreterHelper {
6768
return {};
6869
}
6970
}
71+
public isMacDefaultPythonPath(pythonPath: string) {
72+
return pythonPath === 'python' || pythonPath === '/usr/bin/python';
73+
}
7074
}

‎src/client/interpreter/interpreterService.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
5353
if (!activeWorkspace) {
5454
return;
5555
}
56-
// Check pipenv first
56+
// Check pipenv first.
5757
const pipenvService = this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE);
5858
let interpreters = await pipenvService.getInterpreters(activeWorkspace.folderUri);
5959
if (interpreters.length > 0) {
@@ -102,7 +102,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
102102

103103
return this.getInterpreterDetails(fullyQualifiedPath, resource);
104104
}
105-
public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise<PythonInterpreter> {
105+
public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise<PythonInterpreter | undefined> {
106106
const interpreters = await this.getInterpreters(resource);
107107
const interpreter = interpreters.find(i => utils.arePathsSame(i.path, pythonPath));
108108

@@ -116,7 +116,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
116116
virtualEnvManager.getEnvironmentName(pythonPath),
117117
virtualEnvManager.getEnvironmentType(pythonPath)
118118
]);
119-
if (details) {
119+
if (!details) {
120120
return;
121121
}
122122
const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : '';

‎src/client/interpreter/locators/helpers.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
import { inject, injectable } from 'inversify';
12
import * as path from 'path';
23
import { getArchitectureDislayName } from '../../common/platform/registry';
4+
import { IFileSystem, IPlatformService } from '../../common/platform/types';
35
import { fsReaddirAsync, IS_WINDOWS } from '../../common/utils';
4-
import { PythonInterpreter } from '../contracts';
6+
import { IServiceContainer } from '../../ioc/types';
7+
import { IInterpreterHelper, IInterpreterLocatorHelper, InterpreterType, PythonInterpreter } from '../contracts';
58

69
const CheckPythonInterpreterRegEx = IS_WINDOWS ? /^python(\d+(.\d+)?)?\.exe$/ : /^python(\d+(.\d+)?)?$/;
710

@@ -23,7 +26,47 @@ export function fixInterpreterDisplayName(item: PythonInterpreter) {
2326
}
2427
return item;
2528
}
29+
@injectable()
30+
export class InterpreterLocatorHelper implements IInterpreterLocatorHelper {
31+
private readonly platform: IPlatformService;
32+
private readonly fs: IFileSystem;
33+
private readonly helper: IInterpreterHelper;
2634

27-
export function isMacDefaultPythonPath(p: string) {
28-
return p === 'python' || p === '/usr/bin/python';
35+
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
36+
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
37+
this.helper = serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
38+
this.fs = serviceContainer.get<IFileSystem>(IFileSystem);
39+
}
40+
public mergeInterpreters(interpreters: PythonInterpreter[]) {
41+
return interpreters
42+
.map(item => { return { ...item }; })
43+
.map(fixInterpreterDisplayName)
44+
.map(item => { item.path = path.normalize(item.path); return item; })
45+
.reduce<PythonInterpreter[]>((accumulator, current) => {
46+
if (this.platform.isMac && this.helper.isMacDefaultPythonPath(current.path)) {
47+
return accumulator;
48+
}
49+
const currentVersion = Array.isArray(current.version_info) ? current.version_info.join('.') : undefined;
50+
const existingItem = accumulator.find(item => {
51+
// If same version and same base path, then ignore.
52+
// Could be Python 3.6 with path = python.exe, and Python 3.6 and path = python3.exe.
53+
if (Array.isArray(item.version_info) && item.version_info.join('.') === currentVersion &&
54+
item.path && current.path &&
55+
this.fs.arePathsSame(path.dirname(item.path), path.dirname(current.path))) {
56+
return true;
57+
}
58+
return false;
59+
});
60+
if (!existingItem) {
61+
accumulator.push(current);
62+
} else {
63+
// Preserve type information.
64+
// Possible we identified environment as unknown, but a later provider has identified env type.
65+
if (existingItem.type === InterpreterType.Unknown && current.type !== InterpreterType.Unknown) {
66+
existingItem.type = current.type;
67+
}
68+
}
69+
return accumulator;
70+
}, []);
71+
}
2972
}

‎src/client/interpreter/locators/index.ts

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,33 @@
11
import { inject, injectable } from 'inversify';
22
import * as _ from 'lodash';
3-
import * as path from 'path';
43
import { Disposable, Uri } from 'vscode';
54
import { IPlatformService } from '../../common/platform/types';
65
import { IDisposableRegistry } from '../../common/types';
7-
import { arePathsSame } from '../../common/utils';
86
import { IServiceContainer } from '../../ioc/types';
97
import {
108
CONDA_ENV_FILE_SERVICE,
119
CONDA_ENV_SERVICE,
1210
CURRENT_PATH_SERVICE,
1311
GLOBAL_VIRTUAL_ENV_SERVICE,
12+
IInterpreterLocatorHelper,
1413
IInterpreterLocatorService,
15-
InterpreterType,
1614
KNOWN_PATH_SERVICE,
1715
PIPENV_SERVICE,
1816
PythonInterpreter,
1917
WINDOWS_REGISTRY_SERVICE,
2018
WORKSPACE_VIRTUAL_ENV_SERVICE
2119
} from '../contracts';
22-
import { fixInterpreterDisplayName, isMacDefaultPythonPath } from './helpers';
2320

2421
@injectable()
2522
export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
26-
private disposables: Disposable[] = [];
27-
private platform: IPlatformService;
23+
private readonly disposables: Disposable[] = [];
24+
private readonly platform: IPlatformService;
25+
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
2826

2927
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
3028
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
3129
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
30+
this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
3231
}
3332
public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
3433
return this.getInterpretersPerResource(resource);
@@ -41,27 +40,10 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
4140
const promises = locators.map(async provider => provider.getInterpreters(resource));
4241
const listOfInterpreters = await Promise.all(promises);
4342

44-
// tslint:disable-next-line:underscore-consistent-invocation
45-
return _.flatten(listOfInterpreters)
43+
const items = _.flatten(listOfInterpreters)
4644
.filter(item => !!item)
47-
.map(item => item!)
48-
.map(fixInterpreterDisplayName)
49-
.map(item => { item.path = path.normalize(item.path); return item; })
50-
.reduce<PythonInterpreter[]>((accumulator, current) => {
51-
if (this.platform.isMac && isMacDefaultPythonPath(current.path)) {
52-
return accumulator;
53-
}
54-
const existingItem = accumulator.find(item => arePathsSame(item.path, current.path));
55-
if (!existingItem) {
56-
accumulator.push(current);
57-
} else {
58-
// Preserve type information.
59-
if (existingItem.type === InterpreterType.Unknown && current.type !== InterpreterType.Unknown) {
60-
existingItem.type = current.type;
61-
}
62-
}
63-
return accumulator;
64-
}, []);
45+
.map(item => item!);
46+
return this.interpreterLocatorHelper.mergeInterpreters(items);
6547
}
6648
private getLocators(): IInterpreterLocatorService[] {
6749
const locators: IInterpreterLocatorService[] = [];
@@ -74,14 +56,14 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
7456
}
7557
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_SERVICE));
7658
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_FILE_SERVICE));
59+
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE));
7760
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, GLOBAL_VIRTUAL_ENV_SERVICE));
7861
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE));
7962

8063
if (!this.platform.isWindows) {
8164
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, KNOWN_PATH_SERVICE));
8265
}
8366
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CURRENT_PATH_SERVICE));
84-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE));
8567

8668
return locators;
8769
}

‎src/client/interpreter/locators/services/currentPathService.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ export class CurrentPathService extends CacheableLocatorService {
3535
.then(listOfInterpreters => _.flatten(listOfInterpreters))
3636
.then(interpreters => interpreters.filter(item => item.length > 0))
3737
// tslint:disable-next-line:promise-function-async
38-
.then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter, resource))));
38+
.then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter, resource))))
39+
.then(interpreters => interpreters.filter(item => !!item).map(item => item!));
3940
}
40-
private async getInterpreterDetails(interpreter: string, resource?: Uri): Promise<PythonInterpreter> {
41+
private async getInterpreterDetails(interpreter: string, resource?: Uri): Promise<PythonInterpreter | undefined> {
4142
return Promise.all([
4243
this.helper.getInterpreterInformation(interpreter),
4344
this.virtualEnvMgr.getEnvironmentName(interpreter),

‎src/client/interpreter/serviceRegistry.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
ICondaService,
1616
IInterpreterDisplay,
1717
IInterpreterHelper,
18+
IInterpreterLocatorHelper,
1819
IInterpreterLocatorService,
1920
IInterpreterService,
2021
IInterpreterVersionService,
@@ -33,6 +34,7 @@ import { ShebangCodeLensProvider } from './display/shebangCodeLensProvider';
3334
import { InterpreterHelper } from './helpers';
3435
import { InterpreterService } from './interpreterService';
3536
import { InterpreterVersionService } from './interpreterVersion';
37+
import { InterpreterLocatorHelper } from './locators/helpers';
3638
import { PythonInterpreterLocatorService } from './locators/index';
3739
import { CondaEnvFileService } from './locators/services/condaEnvFileService';
3840
import { CondaEnvService } from './locators/services/condaEnvService';
@@ -79,4 +81,5 @@ export function registerTypes(serviceManager: IServiceManager) {
7981
serviceManager.addSingleton<IInterpreterSelector>(IInterpreterSelector, InterpreterSelector);
8082
serviceManager.addSingleton<IShebangCodeLensProvider>(IShebangCodeLensProvider, ShebangCodeLensProvider);
8183
serviceManager.addSingleton<IInterpreterHelper>(IInterpreterHelper, InterpreterHelper);
84+
serviceManager.addSingleton<IInterpreterLocatorHelper>(IInterpreterLocatorHelper, InterpreterLocatorHelper);
8285
}

0 commit comments

Comments
 (0)