Ensure workspace pipenv environment is not labeled as a virtual env

news/2 Fixes/2223.md

@@ -0,0 +1 @@
+Ensure workspace `pipenv` environment is not labeled as a `virtual env`. 

src/client/common/installer/pythonInstallation.ts

@@ -2,8 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 
-import { IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
-import { isMacDefaultPythonPath } from '../../interpreter/locators/helpers';
+import { IInterpreterHelper, IInterpreterLocatorService, IInterpreterService, INTERPRETER_LOCATOR_SERVICE, InterpreterType } from '../../interpreter/contracts';
 import { IServiceContainer } from '../../ioc/types';
 import { IApplicationShell } from '../application/types';
 import { IPlatformService } from '../platform/types';
@@ -25,7 +24,8 @@ export class PythonInstaller {
         const interpreters = await this.locator.getInterpreters();
         if (interpreters.length > 0) {
             const platform = this.serviceContainer.get<IPlatformService>(IPlatformService);
-            if (platform.isMac && isMacDefaultPythonPath(settings.pythonPath)) {
+            const helper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
+            if (platform.isMac && helper.isMacDefaultPythonPath(settings.pythonPath)) {
                 const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
                 const interpreter = await interpreterService.getActiveInterpreter();
                 if (interpreter && interpreter.type === InterpreterType.Unknown) {

src/client/interpreter/configuration/interpreterSelector.ts

@@ -4,7 +4,6 @@ import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri }
 import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
 import * as settings from '../../common/configSettings';
 import { Commands } from '../../common/constants';
-import { IFileSystem } from '../../common/platform/types';
 import { IServiceContainer } from '../../ioc/types';
 import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts';
 import { IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';
@@ -20,14 +19,12 @@ export class InterpreterSelector implements IInterpreterSelector {
     private readonly workspaceService: IWorkspaceService;
     private readonly applicationShell: IApplicationShell;
     private readonly documentManager: IDocumentManager;
-    private readonly fileSystem: IFileSystem;
 
     constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
         this.interpreterManager = serviceContainer.get<IInterpreterService>(IInterpreterService);
         this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         this.applicationShell = this.serviceContainer.get<IApplicationShell>(IApplicationShell);
         this.documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
-        this.fileSystem = this.serviceContainer.get<IFileSystem>(IFileSystem);
 
         const commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
         this.disposables.push(commandManager.registerCommand(Commands.Set_Interpreter, this.setInterpreter.bind(this)));
@@ -38,8 +35,7 @@ export class InterpreterSelector implements IInterpreterSelector {
     }
 
     public async getSuggestions(resourceUri?: Uri) {
-        let interpreters = await this.interpreterManager.getInterpreters(resourceUri);
-        interpreters = await this.removeDuplicates(interpreters);
+        const interpreters = await this.interpreterManager.getInterpreters(resourceUri);
         // tslint:disable-next-line:no-non-null-assertion
         interpreters.sort((a, b) => a.displayName! > b.displayName! ? 1 : -1);
         return Promise.all(interpreters.map(item => this.suggestionToQuickPickItem(item, resourceUri)));
@@ -74,17 +70,6 @@ export class InterpreterSelector implements IInterpreterSelector {
         };
     }
 
-    private async removeDuplicates(interpreters: PythonInterpreter[]): Promise<PythonInterpreter[]> {
-        const result: PythonInterpreter[] = [];
-        interpreters.forEach(x => {
-            if (result.findIndex(a => a.displayName === x.displayName
-                && a.type === x.type && this.fileSystem.arePathsSame(path.dirname(a.path), path.dirname(x.path))) < 0) {
-                result.push(x);
-            }
-        });
-        return result;
-    }
-
     private async setInterpreter() {
         const setInterpreterGlobally = !Array.isArray(this.workspaceService.workspaceFolders) || this.workspaceService.workspaceFolders.length === 0;
         let configTarget = ConfigurationTarget.Global;

src/client/interpreter/contracts.ts

@@ -78,7 +78,7 @@ export interface IInterpreterService {
     getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
     autoSetInterpreter(): Promise<void>;
     getActiveInterpreter(resource?: Uri): Promise<PythonInterpreter | undefined>;
-    getInterpreterDetails(pythonPath: string): Promise<Partial<PythonInterpreter>>;
+    getInterpreterDetails(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
     refresh(): Promise<void>;
     initialize(): void;
 }
@@ -97,9 +97,15 @@ export const IInterpreterHelper = Symbol('IInterpreterHelper');
 export interface IInterpreterHelper {
     getActiveWorkspaceUri(): WorkspacePythonPath | undefined;
     getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>>;
+    isMacDefaultPythonPath(pythonPath: string): Boolean;
 }
 
 export const IPipEnvService = Symbol('IPipEnvService');
 export interface IPipEnvService {
     isRelatedPipEnvironment(dir: string, pythonPath: string): Promise<boolean>;
 }
+
+export const IInterpreterLocatorHelper = Symbol('IInterpreterLocatorHelper');
+export interface IInterpreterLocatorHelper {
+    mergeInterpreters(interpreters: PythonInterpreter[]): PythonInterpreter[];
+}

src/client/interpreter/helpers.ts

@@ -2,7 +2,7 @@ import { inject, injectable } from 'inversify';
 import { ConfigurationTarget } from 'vscode';
 import { IDocumentManager, IWorkspaceService } from '../common/application/types';
 import { IFileSystem } from '../common/platform/types';
-import { IPythonExecutionFactory } from '../common/process/types';
+import { InterpreterInfomation, IPythonExecutionFactory } from '../common/process/types';
 import { IPersistentStateFactory } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
 import { IInterpreterHelper, PythonInterpreter, WorkspacePythonPath } from './contracts';
@@ -33,7 +33,7 @@ export class InterpreterHelper implements IInterpreterHelper {
         if (!workspaceService.hasWorkspaceFolders) {
             return;
         }
-        if (workspaceService.workspaceFolders.length === 1) {
+        if (Array.isArray(workspaceService.workspaceFolders) && workspaceService.workspaceFolders.length === 1) {
             return { folderUri: workspaceService.workspaceFolders[0].uri, configTarget: ConfigurationTarget.Workspace };
         }
         if (documentManager.activeTextEditor) {
@@ -44,15 +44,16 @@ export class InterpreterHelper implements IInterpreterHelper {
         }
     }
     public async getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>> {
-        const fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
+        let fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
+        fileHash = fileHash ? fileHash : '';
         const store = this.persistentFactory.createGlobalPersistentState<CachedPythonInterpreter>(pythonPath, undefined, EXPITY_DURATION);
-        if (store.value && store.value.fileHash === fileHash) {
+        if (store.value && (!fileHash || store.value.fileHash === fileHash)) {
             return store.value;
         }
         const processService = await this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory).create({ pythonPath });
 
         try {
-            const info = await processService.getInterpreterInformation().catch(() => undefined);
+            const info = await processService.getInterpreterInformation().catch<InterpreterInfomation | undefined>(() => undefined);
             if (!info) {
                 return;
             }
@@ -67,4 +68,7 @@ export class InterpreterHelper implements IInterpreterHelper {
             return {};
         }
     }
+    public isMacDefaultPythonPath(pythonPath: string) {
+        return pythonPath === 'python' || pythonPath === '/usr/bin/python';
+    }
 }

src/client/interpreter/interpreterService.ts

@@ -53,7 +53,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
         if (!activeWorkspace) {
             return;
         }
-        // Check pipenv first
+        // Check pipenv first.
         const pipenvService = this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE);
         let interpreters = await pipenvService.getInterpreters(activeWorkspace.folderUri);
         if (interpreters.length > 0) {
@@ -102,7 +102,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
 
         return this.getInterpreterDetails(fullyQualifiedPath, resource);
     }
-    public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise<PythonInterpreter> {
+    public async getInterpreterDetails(pythonPath: string, resource?: Uri): Promise<PythonInterpreter | undefined> {
         const interpreters = await this.getInterpreters(resource);
         const interpreter = interpreters.find(i => utils.arePathsSame(i.path, pythonPath));
 
@@ -116,7 +116,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
             virtualEnvManager.getEnvironmentName(pythonPath),
             virtualEnvManager.getEnvironmentType(pythonPath)
         ]);
-        if (details) {
+        if (!details) {
             return;
         }
         const dislayNameSuffix = virtualEnvName.length > 0 ? ` (${virtualEnvName})` : '';

src/client/interpreter/locators/helpers.ts

@@ -1,7 +1,10 @@
+import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { getArchitectureDislayName } from '../../common/platform/registry';
+import { IFileSystem, IPlatformService } from '../../common/platform/types';
 import { fsReaddirAsync, IS_WINDOWS } from '../../common/utils';
-import { PythonInterpreter } from '../contracts';
+import { IServiceContainer } from '../../ioc/types';
+import { IInterpreterHelper, IInterpreterLocatorHelper, InterpreterType, PythonInterpreter } from '../contracts';
 
 const CheckPythonInterpreterRegEx = IS_WINDOWS ? /^python(\d+(.\d+)?)?\.exe$/ : /^python(\d+(.\d+)?)?$/;
 
@@ -23,7 +26,47 @@ export function fixInterpreterDisplayName(item: PythonInterpreter) {
     }
     return item;
 }
+@injectable()
+export class InterpreterLocatorHelper implements IInterpreterLocatorHelper {
+    private readonly platform: IPlatformService;
+    private readonly fs: IFileSystem;
+    private readonly helper: IInterpreterHelper;
 
-export function isMacDefaultPythonPath(p: string) {
-    return p === 'python' || p === '/usr/bin/python';
+    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
+        this.helper = serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
+        this.fs = serviceContainer.get<IFileSystem>(IFileSystem);
+    }
+    public mergeInterpreters(interpreters: PythonInterpreter[]) {
+        return interpreters
+            .map(item => { return { ...item }; })
+            .map(fixInterpreterDisplayName)
+            .map(item => { item.path = path.normalize(item.path); return item; })
+            .reduce<PythonInterpreter[]>((accumulator, current) => {
+                if (this.platform.isMac && this.helper.isMacDefaultPythonPath(current.path)) {
+                    return accumulator;
+                }
+                const currentVersion = Array.isArray(current.version_info) ? current.version_info.join('.') : undefined;
+                const existingItem = accumulator.find(item => {
+                    // If same version and same base path, then ignore.
+                    // Could be Python 3.6 with path = python.exe, and Python 3.6 and path = python3.exe.
+                    if (Array.isArray(item.version_info) && item.version_info.join('.') === currentVersion &&
+                        item.path && current.path &&
+                        this.fs.arePathsSame(path.dirname(item.path), path.dirname(current.path))) {
+                        return true;
+                    }
+                    return false;
+                });
+                if (!existingItem) {
+                    accumulator.push(current);
+                } else {
+                    // Preserve type information.
+                    // Possible we identified environment as unknown, but a later provider has identified env type.
+                    if (existingItem.type === InterpreterType.Unknown && current.type !== InterpreterType.Unknown) {
+                        existingItem.type = current.type;
+                    }
+                }
+                return accumulator;
+            }, []);
+    }
 }

src/client/interpreter/locators/index.ts

@@ -1,34 +1,33 @@
 import { inject, injectable } from 'inversify';
 import * as _ from 'lodash';
-import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
 import { IPlatformService } from '../../common/platform/types';
 import { IDisposableRegistry } from '../../common/types';
-import { arePathsSame } from '../../common/utils';
 import { IServiceContainer } from '../../ioc/types';
 import {
     CONDA_ENV_FILE_SERVICE,
     CONDA_ENV_SERVICE,
     CURRENT_PATH_SERVICE,
     GLOBAL_VIRTUAL_ENV_SERVICE,
+    IInterpreterLocatorHelper,
     IInterpreterLocatorService,
-    InterpreterType,
     KNOWN_PATH_SERVICE,
     PIPENV_SERVICE,
     PythonInterpreter,
     WINDOWS_REGISTRY_SERVICE,
     WORKSPACE_VIRTUAL_ENV_SERVICE
 } from '../contracts';
-import { fixInterpreterDisplayName, isMacDefaultPythonPath } from './helpers';
 
 @injectable()
 export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
-    private disposables: Disposable[] = [];
-    private platform: IPlatformService;
+    private readonly disposables: Disposable[] = [];
+    private readonly platform: IPlatformService;
+    private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
 
     constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
         serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
         this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
+        this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
     }
     public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
         return this.getInterpretersPerResource(resource);
@@ -41,27 +40,10 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         const promises = locators.map(async provider => provider.getInterpreters(resource));
         const listOfInterpreters = await Promise.all(promises);
 
-        // tslint:disable-next-line:underscore-consistent-invocation
-        return _.flatten(listOfInterpreters)
+        const items = _.flatten(listOfInterpreters)
             .filter(item => !!item)
-            .map(item => item!)
-            .map(fixInterpreterDisplayName)
-            .map(item => { item.path = path.normalize(item.path); return item; })
-            .reduce<PythonInterpreter[]>((accumulator, current) => {
-                if (this.platform.isMac && isMacDefaultPythonPath(current.path)) {
-                    return accumulator;
-                }
-                const existingItem = accumulator.find(item => arePathsSame(item.path, current.path));
-                if (!existingItem) {
-                    accumulator.push(current);
-                } else {
-                    // Preserve type information.
-                    if (existingItem.type === InterpreterType.Unknown && current.type !== InterpreterType.Unknown) {
-                        existingItem.type = current.type;
-                    }
-                }
-                return accumulator;
-            }, []);
+            .map(item => item!);
+        return this.interpreterLocatorHelper.mergeInterpreters(items);
     }
     private getLocators(): IInterpreterLocatorService[] {
         const locators: IInterpreterLocatorService[] = [];
@@ -74,14 +56,14 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         }
         locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_SERVICE));
         locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_FILE_SERVICE));
+        locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE));
         locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, GLOBAL_VIRTUAL_ENV_SERVICE));
         locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE));
 
         if (!this.platform.isWindows) {
             locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, KNOWN_PATH_SERVICE));
         }
         locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CURRENT_PATH_SERVICE));
-        locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE));
 
         return locators;
     }

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

@@ -35,9 +35,10 @@ export class CurrentPathService extends CacheableLocatorService {
             .then(listOfInterpreters => _.flatten(listOfInterpreters))
             .then(interpreters => interpreters.filter(item => item.length > 0))
             // tslint:disable-next-line:promise-function-async
-            .then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter, resource))));
+            .then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter, resource))))
+            .then(interpreters => interpreters.filter(item => !!item).map(item => item!));
     }
-    private async getInterpreterDetails(interpreter: string, resource?: Uri): Promise<PythonInterpreter> {
+    private async getInterpreterDetails(interpreter: string, resource?: Uri): Promise<PythonInterpreter | undefined> {
         return Promise.all([
             this.helper.getInterpreterInformation(interpreter),
             this.virtualEnvMgr.getEnvironmentName(interpreter),

src/client/interpreter/serviceRegistry.ts

@@ -15,6 +15,7 @@ import {
     ICondaService,
     IInterpreterDisplay,
     IInterpreterHelper,
+    IInterpreterLocatorHelper,
     IInterpreterLocatorService,
     IInterpreterService,
     IInterpreterVersionService,
@@ -33,6 +34,7 @@ import { ShebangCodeLensProvider } from './display/shebangCodeLensProvider';
 import { InterpreterHelper } from './helpers';
 import { InterpreterService } from './interpreterService';
 import { InterpreterVersionService } from './interpreterVersion';
+import { InterpreterLocatorHelper } from './locators/helpers';
 import { PythonInterpreterLocatorService } from './locators/index';
 import { CondaEnvFileService } from './locators/services/condaEnvFileService';
 import { CondaEnvService } from './locators/services/condaEnvService';
@@ -79,4 +81,5 @@ export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingleton<IInterpreterSelector>(IInterpreterSelector, InterpreterSelector);
     serviceManager.addSingleton<IShebangCodeLensProvider>(IShebangCodeLensProvider, ShebangCodeLensProvider);
     serviceManager.addSingleton<IInterpreterHelper>(IInterpreterHelper, InterpreterHelper);
+    serviceManager.addSingleton<IInterpreterLocatorHelper>(IInterpreterLocatorHelper, InterpreterLocatorHelper);
 }