From 171be3c909b0f4af521d19fe155b6dcf0b8d1457 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 09:35:20 -0700 Subject: [PATCH 1/9] check envfile for conda environments --- .../interpreter/{sources => }/contracts.ts | 5 +- src/client/interpreter/display/index.ts | 20 +++--- src/client/interpreter/helpers.ts | 7 +++ src/client/interpreter/index.ts | 11 ++-- src/client/interpreter/interpreterVersion.ts | 13 ++++ .../{sources => locators}/helpers.ts | 10 +-- .../{sources => locators}/index.ts | 38 ++++++------ .../services/KnownPathsService.ts} | 12 ++-- .../interpreter/locators/services/conda.ts | 6 ++ .../locators/services/condaEnvFileService.ts | 61 +++++++++++++++++++ .../services/condaEnvService.ts} | 13 ++-- .../services/currentPathService.ts} | 15 ++--- .../services/virtualEnvService.ts} | 14 +++-- .../services/windowsRegistryService.ts} | 5 +- .../providers/setInterpreterProvider.ts | 3 +- .../interpreters/condaEnvFileService.test.ts | 51 ++++++++++++++++ ...ovider.test.ts => condaEnvService.test.ts} | 4 +- src/test/interpreters/display.test.ts | 47 +++++++------- src/test/interpreters/mocks.ts | 14 ++++- ...test.ts => windowsRegistryService.test.ts} | 2 +- .../pythonFiles/environments/environments.txt | 0 21 files changed, 249 insertions(+), 102 deletions(-) rename src/client/interpreter/{sources => }/contracts.ts (61%) create mode 100644 src/client/interpreter/helpers.ts create mode 100644 src/client/interpreter/interpreterVersion.ts rename src/client/interpreter/{sources => locators}/helpers.ts (79%) rename src/client/interpreter/{sources => locators}/index.ts (55%) rename src/client/interpreter/{sources/providers/KnownPathsProvider.ts => locators/services/KnownPathsService.ts} (78%) create mode 100644 src/client/interpreter/locators/services/conda.ts create mode 100644 src/client/interpreter/locators/services/condaEnvFileService.ts rename src/client/interpreter/{sources/providers/condaEnvProvider.ts => locators/services/condaEnvService.ts} (90%) rename src/client/interpreter/{sources/providers/CurrentPathProvider.ts => locators/services/currentPathService.ts} (82%) rename src/client/interpreter/{sources/providers/virtualEnvProvider.ts => locators/services/virtualEnvService.ts} (83%) rename src/client/interpreter/{sources/providers/windowsRegistryProvider.ts => locators/services/windowsRegistryService.ts} (97%) create mode 100644 src/test/interpreters/condaEnvFileService.test.ts rename src/test/interpreters/{condaEnvProvider.test.ts => condaEnvService.test.ts} (98%) rename src/test/interpreters/{windowsRegistryProvider.test.ts => windowsRegistryService.test.ts} (99%) create mode 100644 src/test/pythonFiles/environments/environments.txt diff --git a/src/client/interpreter/sources/contracts.ts b/src/client/interpreter/contracts.ts similarity index 61% rename from src/client/interpreter/sources/contracts.ts rename to src/client/interpreter/contracts.ts index 94a53f057e6a..cb5120a549e0 100644 --- a/src/client/interpreter/sources/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -1,7 +1,6 @@ -import { PythonInterpreter } from "../index"; -import { Architecture } from "../../common/registry"; +import { Architecture } from "../common/registry"; -export interface IInterpreterProvider { +export interface IInterpreterLocatorService { getInterpreters(): Promise; } export interface PythonInterpreter { diff --git a/src/client/interpreter/display/index.ts b/src/client/interpreter/display/index.ts index e4c614f506ad..abf4e6cc22fe 100644 --- a/src/client/interpreter/display/index.ts +++ b/src/client/interpreter/display/index.ts @@ -1,17 +1,21 @@ 'use strict'; +import * as path from 'path'; +import * as utils from '../../common/utils'; +import * as child_process from 'child_process'; import { StatusBarItem, Disposable } from 'vscode'; import { PythonSettings } from '../../common/configSettings'; -import * as path from 'path'; import { EOL } from 'os'; -import { IInterpreterProvider } from '../index'; -import * as utils from '../../common/utils'; +import { IInterpreterLocatorService } from '../contracts'; +import { IInterpreterVersionService } from '../interpreterVersion'; import { VirtualEnvironmentManager } from '../virtualEnvs/index'; -import { getFirstNonEmptyLineFromMultilineString } from '../sources/helpers'; -import * as child_process from 'child_process'; +import { getFirstNonEmptyLineFromMultilineString } from '../helpers'; const settings = PythonSettings.getInstance(); export class InterpreterDisplay implements Disposable { - constructor(private statusBar: StatusBarItem, private interpreterProvoder: IInterpreterProvider, private virtualEnvMgr: VirtualEnvironmentManager) { + constructor(private statusBar: StatusBarItem, + private interpreterLocator: IInterpreterLocatorService, + private virtualEnvMgr: VirtualEnvironmentManager, + private versionProvider: IInterpreterVersionService) { this.statusBar.command = 'python.setInterpreter'; } public dispose() { @@ -21,7 +25,7 @@ export class InterpreterDisplay implements Disposable { await this.updateDisplay(pythonPath); } private getInterpreters() { - return this.interpreterProvoder.getInterpreters(); + return this.interpreterLocator.getInterpreters(); } private async updateDisplay(pythonPath: string) { const interpreters = await this.getInterpreters(); @@ -39,7 +43,7 @@ export class InterpreterDisplay implements Disposable { else { const defaultDisplayName = `${path.basename(pythonPath)} [Environment]`; const interpreterExists = utils.fsExistsAsync(pythonPath); - const displayName = utils.getInterpreterDisplayName(pythonPath).catch(() => defaultDisplayName); + const displayName = this.versionProvider.getVersion(pythonPath, defaultDisplayName); const virtualEnvName = this.getVirtualEnvironmentName(pythonPath); await Promise.all([interpreterExists, displayName, virtualEnvName]) .then(([interpreterExists, displayName, virtualEnvName]) => { diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts new file mode 100644 index 000000000000..9b135ce748f8 --- /dev/null +++ b/src/client/interpreter/helpers.ts @@ -0,0 +1,7 @@ +export function getFirstNonEmptyLineFromMultilineString(stdout: string) { + if (stdout.length === 0) { + return ''; + } + const lines = stdout.split(/\r?\n/g).filter(line => line.trim().length > 0); + return lines.length > 0 ? lines[0] : ''; +} diff --git a/src/client/interpreter/index.ts b/src/client/interpreter/index.ts index 050eb525831b..9ca434473700 100644 --- a/src/client/interpreter/index.ts +++ b/src/client/interpreter/index.ts @@ -1,26 +1,27 @@ 'use strict'; +import { InterpreterVersionService } from './interpreterVersion'; import { VirtualEnv } from './virtualEnvs/virtualEnv'; import { VEnv } from './virtualEnvs/venv'; import { Disposable, window, StatusBarAlignment, workspace } from 'vscode'; import { PythonSettings } from '../common/configSettings'; import { InterpreterDisplay } from './display'; -import { PythonInterpreterProvider } from './sources'; +import { PythonInterpreterLocatorService } from './locators'; import { VirtualEnvironmentManager } from './virtualEnvs/index'; import { IS_WINDOWS } from '../common/utils'; import * as path from 'path'; -export * from './sources'; const settings = PythonSettings.getInstance(); export class InterpreterManager implements Disposable { private disposables: Disposable[] = []; private display: InterpreterDisplay | null | undefined; - private interpreterProvider: PythonInterpreterProvider; + private interpreterProvider: PythonInterpreterLocatorService; constructor() { const virtualEnvMgr = new VirtualEnvironmentManager([new VEnv(), new VirtualEnv()]); const statusBar = window.createStatusBarItem(StatusBarAlignment.Left); - this.interpreterProvider = new PythonInterpreterProvider(virtualEnvMgr); - this.display = new InterpreterDisplay(statusBar, this.interpreterProvider, virtualEnvMgr); + this.interpreterProvider = new PythonInterpreterLocatorService(virtualEnvMgr); + const versionService = new InterpreterVersionService(); + this.display = new InterpreterDisplay(statusBar, this.interpreterProvider, virtualEnvMgr, versionService); settings.addListener('change', this.onConfigChanged.bind(this)); this.display.refresh(); diff --git a/src/client/interpreter/interpreterVersion.ts b/src/client/interpreter/interpreterVersion.ts new file mode 100644 index 000000000000..be5684e584ea --- /dev/null +++ b/src/client/interpreter/interpreterVersion.ts @@ -0,0 +1,13 @@ +import * as path from 'path'; +import { getInterpreterDisplayName } from '../common/utils'; + +export interface IInterpreterVersionService { + getVersion(pythonPath: string, defaultValue: string): Promise; +} + +export class InterpreterVersionService implements IInterpreterVersionService { + getVersion(pythonPath: string, defaultValue: string): Promise { + return getInterpreterDisplayName(pythonPath) + .catch(() => path.basename(defaultValue)); + } +} \ No newline at end of file diff --git a/src/client/interpreter/sources/helpers.ts b/src/client/interpreter/locators/helpers.ts similarity index 79% rename from src/client/interpreter/sources/helpers.ts rename to src/client/interpreter/locators/helpers.ts index 7974563d39b1..9f12c850a972 100644 --- a/src/client/interpreter/sources/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -1,4 +1,4 @@ -import { PythonInterpreter } from "../index"; +import { PythonInterpreter } from "../contracts"; import { IS_WINDOWS, fsReaddirAsync } from "../../common/utils"; import * as path from 'path'; import { getArchitectureDislayName } from "../../common/registry"; @@ -25,11 +25,3 @@ export function fixInterpreterPath(item: PythonInterpreter) { item.path = IS_WINDOWS ? item.path.replace(/\//g, "\\") : item.path; return item; } - -export function getFirstNonEmptyLineFromMultilineString(stdout: string) { - if (stdout.length === 0) { - return ''; - } - const lines = stdout.split(/\r?\n/g).filter(line => line.trim().length > 0); - return lines.length > 0 ? lines[0] : ''; -} diff --git a/src/client/interpreter/sources/index.ts b/src/client/interpreter/locators/index.ts similarity index 55% rename from src/client/interpreter/sources/index.ts rename to src/client/interpreter/locators/index.ts index 2adeb6e644e4..b8fe5306782e 100644 --- a/src/client/interpreter/sources/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -1,47 +1,49 @@ "use strict"; import * as _ from 'lodash'; import { fixInterpreterPath, fixInterpreterDisplayName } from './helpers'; -import { IInterpreterProvider } from './contracts'; +import { IInterpreterLocatorService, PythonInterpreter } from '../contracts'; +import { InterpreterVersionService } from '../interpreterVersion'; import { IS_WINDOWS, Is_64Bit, arePathsSame, areBasePathsSame } from '../../common/utils'; import { RegistryImplementation } from '../../common/registry'; -import { CondaEnvProvider } from './providers/condaEnvProvider'; -import { PythonInterpreter } from '../index'; -import { VirtualEnvProvider, getKnownSearchPathsForVirtualEnvs } from './providers/virtualEnvProvider'; -import { KnownPathsProvider, getKnownSearchPathsForInterpreters } from './providers/KnownPathsProvider'; -import { CurrentPathProvider } from './providers/CurrentPathProvider'; -import { WindowsRegistryProvider } from './providers/windowsRegistryProvider'; +import { CondaEnvProvider } from './services/condaEnvService'; +import { VirtualEnvProvider, getKnownSearchPathsForVirtualEnvs } from './services/virtualEnvService'; +import { KnownPathsProvider, getKnownSearchPathsForInterpreters } from './services/KnownPathsService'; +import { CurrentPathProvider } from './services/CurrentPathService'; +import { WindowsRegistryProvider } from './services/windowsRegistryService'; import { VirtualEnvironmentManager } from '../virtualEnvs'; -export * from './contracts'; +import { CondaEnvFileProvider, getEnvironmentsFile as getCondaEnvFile } from './services/condaEnvFileService'; -export class PythonInterpreterProvider implements IInterpreterProvider { +export class PythonInterpreterLocatorService implements IInterpreterLocatorService { private interpreters: PythonInterpreter[] = []; - private providers: IInterpreterProvider[] = []; + private locators: IInterpreterLocatorService[] = []; constructor(private virtualEnvMgr: VirtualEnvironmentManager) { - // The order of the providers is important + const versionService = new InterpreterVersionService(); + // The order of the services is important if (IS_WINDOWS) { const windowsRegistryProvider = new WindowsRegistryProvider(new RegistryImplementation(), Is_64Bit); - this.providers.push(windowsRegistryProvider); - this.providers.push(new CondaEnvProvider(windowsRegistryProvider)); + this.locators.push(windowsRegistryProvider); + this.locators.push(new CondaEnvProvider(windowsRegistryProvider)); + this.locators.push(new CondaEnvFileProvider(getCondaEnvFile(), versionService)); } else { - this.providers.push(new CondaEnvProvider()); + this.locators.push(new CondaEnvProvider()); } - this.providers.push(new VirtualEnvProvider(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr)); + this.locators.push(new VirtualEnvProvider(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr, versionService)); if (!IS_WINDOWS) { // This must be last, it is possible we have paths returned here that are already returned // in one of the above lists - this.providers.push(new KnownPathsProvider(getKnownSearchPathsForInterpreters())); + this.locators.push(new KnownPathsProvider(getKnownSearchPathsForInterpreters(), versionService)); } // This must be last, it is possible we have paths returned here that are already returned // in one of the above lists - this.providers.push(new CurrentPathProvider(this.virtualEnvMgr)); + this.locators.push(new CurrentPathProvider(this.virtualEnvMgr, versionService)); } public getInterpreters() { if (this.interpreters.length > 0) { return Promise.resolve(this.interpreters); } - const promises = this.providers.map(provider => provider.getInterpreters()); + const promises = this.locators.map(provider => provider.getInterpreters()); return Promise.all(promises) .then(interpreters => _.flatten(interpreters)) .then(items => items.map(fixInterpreterDisplayName)) diff --git a/src/client/interpreter/sources/providers/KnownPathsProvider.ts b/src/client/interpreter/locators/services/KnownPathsService.ts similarity index 78% rename from src/client/interpreter/sources/providers/KnownPathsProvider.ts rename to src/client/interpreter/locators/services/KnownPathsService.ts index 1d4c0fe86371..4235deb5c906 100644 --- a/src/client/interpreter/sources/providers/KnownPathsProvider.ts +++ b/src/client/interpreter/locators/services/KnownPathsService.ts @@ -1,13 +1,15 @@ "use strict"; import * as path from 'path'; import * as _ from 'lodash'; -import { IInterpreterProvider } from '../contracts'; -import { fsExistsAsync, getInterpreterDisplayName, IS_WINDOWS } from '../../../common/utils'; +import { IInterpreterLocatorService } from '../../contracts'; +import { IInterpreterVersionService } from '../../interpreterVersion'; +import { fsExistsAsync, IS_WINDOWS } from '../../../common/utils'; import { lookForInterpretersInDirectory } from '../helpers'; const untildify = require('untildify'); -export class KnownPathsProvider implements IInterpreterProvider { - public constructor(private knownSearchPaths: string[]) { } +export class KnownPathsProvider implements IInterpreterLocatorService { + public constructor(private knownSearchPaths: string[], + private versionProvider: IInterpreterVersionService) { } public getInterpreters() { return this.suggestionsFromKnownPaths(); } @@ -20,7 +22,7 @@ export class KnownPathsProvider implements IInterpreterProvider { .then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter)))); } private getInterpreterDetails(interpreter: string) { - return getInterpreterDisplayName(interpreter).catch(() => path.basename(interpreter)) + return this.versionProvider.getVersion(interpreter, path.basename(interpreter)) .then(displayName => { return { displayName, diff --git a/src/client/interpreter/locators/services/conda.ts b/src/client/interpreter/locators/services/conda.ts new file mode 100644 index 000000000000..4b2d38c63615 --- /dev/null +++ b/src/client/interpreter/locators/services/conda.ts @@ -0,0 +1,6 @@ +import { IS_WINDOWS } from "../../../common/utils"; + +// where to find the Python binary within a conda env +export const CONDA_RELATIVE_PY_PATH = IS_WINDOWS ? ['python.exe'] : ['bin', 'python']; +export const AnacondaCompanyName = 'Continuum Analytics, Inc.'; +export const AnacondaDisplayName = 'Anaconda'; diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts new file mode 100644 index 000000000000..3980d1207e40 --- /dev/null +++ b/src/client/interpreter/locators/services/condaEnvFileService.ts @@ -0,0 +1,61 @@ +"use strict"; +import * as fs from 'fs-extra'; +import * as path from 'path'; +import { IInterpreterVersionService } from '../../interpreterVersion'; +import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; +import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH } from './conda'; + +export class CondaEnvFileProvider implements IInterpreterLocatorService { + constructor(private condaEnvironmentFile: string, + private versionService: IInterpreterVersionService) { + } + public getInterpreters() { + return this.getSuggestionsFromConda(); + } + + private getSuggestionsFromConda(): Promise { + return fs.pathExists(this.condaEnvironmentFile) + .then(exists => exists ? this.getEnvironmentsFromFile(this.condaEnvironmentFile) : Promise.resolve([])); + } + private getEnvironmentsFromFile(envFile: string) { + return fs.readFile(envFile) + .then(buffer => buffer.toString().split(/\r?\n/g)) + .then(lines => { + return lines.map(line => path.join(line, ...CONDA_RELATIVE_PY_PATH)); + }) + .then(interpreterPaths => { + return interpreterPaths.map(item => fs.pathExists(item).then(exists => exists ? item : '')); + }) + .then(promises => Promise.all(promises)) + .then(interpreterPaths => { + return interpreterPaths.filter(item => item.trim().length > 0); + }) + .then(interpreterPaths => interpreterPaths.map(item => this.getInterpreterDetails(item))) + .then(promises => Promise.all(promises)); + } + private getInterpreterDetails(interpreter: string) { + return this.versionService.getVersion(interpreter, path.basename(interpreter)) + .then(version => { + // Strip company name from version + const startOfCompanyName = version.indexOf(`:: ${AnacondaCompanyName}`); + version = startOfCompanyName > 0 ? version.substring(0, startOfCompanyName).trim() : version; + const envName = this.getEnvironmentRootDirectory(interpreter); + const info: PythonInterpreter = { + displayName: `${AnacondaDisplayName} ${version} (${envName})`, + path: interpreter, + companyDisplayName: AnacondaCompanyName, + version: version + }; + return info; + }); + } + private getEnvironmentRootDirectory(interpreter: string) { + const envDir = interpreter.substring(0, interpreter.length - path.join(...CONDA_RELATIVE_PY_PATH).length); + return path.basename(envDir); + } +} + +export function getEnvironmentsFile() { + const profileDir = process.env['USERPROFILE']; + return profileDir ? path.join(profileDir, '.conda', 'environments.txt') : ''; +} diff --git a/src/client/interpreter/sources/providers/condaEnvProvider.ts b/src/client/interpreter/locators/services/condaEnvService.ts similarity index 90% rename from src/client/interpreter/sources/providers/condaEnvProvider.ts rename to src/client/interpreter/locators/services/condaEnvService.ts index 287dd14511c5..f9967f03aea2 100644 --- a/src/client/interpreter/sources/providers/condaEnvProvider.ts +++ b/src/client/interpreter/locators/services/condaEnvService.ts @@ -2,22 +2,17 @@ import * as child_process from 'child_process'; import * as fs from 'fs-extra'; import * as path from 'path'; -import { IInterpreterProvider, PythonInterpreter } from '../contracts'; -import { IS_WINDOWS } from "../../../common/utils"; +import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; import { VersionUtils } from "../../../common/versionUtils"; - -// where to find the Python binary within a conda env -const CONDA_RELATIVE_PY_PATH = IS_WINDOWS ? ['python.exe'] : ['bin', 'python']; -const AnacondaCompanyName = 'Continuum Analytics, Inc.'; -const AnacondaDisplayName = 'Anaconda'; +import { AnacondaCompanyName, AnacondaDisplayName, CONDA_RELATIVE_PY_PATH } from './conda'; type CondaInfo = { envs?: string[]; "sys.version"?: string; default_prefix?: string; } -export class CondaEnvProvider implements IInterpreterProvider { - constructor(private registryLookupForConda?: IInterpreterProvider) { +export class CondaEnvProvider implements IInterpreterLocatorService { + constructor(private registryLookupForConda?: IInterpreterLocatorService) { } public getInterpreters() { return this.getSuggestionsFromConda(); diff --git a/src/client/interpreter/sources/providers/CurrentPathProvider.ts b/src/client/interpreter/locators/services/currentPathService.ts similarity index 82% rename from src/client/interpreter/sources/providers/CurrentPathProvider.ts rename to src/client/interpreter/locators/services/currentPathService.ts index 533da3060d2b..e2154ea3e850 100644 --- a/src/client/interpreter/sources/providers/CurrentPathProvider.ts +++ b/src/client/interpreter/locators/services/currentPathService.ts @@ -1,17 +1,18 @@ "use strict"; -import * as path from 'path'; import * as _ from 'lodash'; -import { IInterpreterProvider } from '../contracts'; -import { getInterpreterDisplayName } from '../../../common/utils'; -import { getFirstNonEmptyLineFromMultilineString } from '../helpers'; +import * as path from 'path'; import * as child_process from 'child_process'; +import { IInterpreterLocatorService } from '../../contracts'; +import { IInterpreterVersionService } from '../../interpreterVersion'; +import { getFirstNonEmptyLineFromMultilineString } from '../../helpers'; import { VirtualEnvironmentManager } from '../../virtualEnvs'; import { PythonSettings } from '../../../common/configSettings'; const settings = PythonSettings.getInstance(); -export class CurrentPathProvider implements IInterpreterProvider { - public constructor(private virtualEnvMgr: VirtualEnvironmentManager) { } +export class CurrentPathProvider implements IInterpreterLocatorService { + public constructor(private virtualEnvMgr: VirtualEnvironmentManager, + private versionProvider: IInterpreterVersionService) { } public getInterpreters() { return this.suggestionsFromKnownPaths(); } @@ -28,7 +29,7 @@ export class CurrentPathProvider implements IInterpreterProvider { } private getInterpreterDetails(interpreter: string) { const virtualEnv = this.virtualEnvMgr.detect(interpreter); - const displayName = getInterpreterDisplayName(interpreter).catch(() => path.basename(interpreter)); + const displayName = this.versionProvider.getVersion(interpreter, path.basename(interpreter)); return Promise.all([displayName, virtualEnv]) .then(([displayName, virtualEnv]) => { displayName += virtualEnv ? ` (${virtualEnv.name})` : ''; diff --git a/src/client/interpreter/sources/providers/virtualEnvProvider.ts b/src/client/interpreter/locators/services/virtualEnvService.ts similarity index 83% rename from src/client/interpreter/sources/providers/virtualEnvProvider.ts rename to src/client/interpreter/locators/services/virtualEnvService.ts index 19f4d3c96199..41bf0eb95ed9 100644 --- a/src/client/interpreter/sources/providers/virtualEnvProvider.ts +++ b/src/client/interpreter/locators/services/virtualEnvService.ts @@ -3,15 +3,17 @@ import * as _ from 'lodash'; import * as path from 'path'; import * as settings from './../../../common/configSettings'; import { VirtualEnvironmentManager } from '../../virtualEnvs'; -import { IInterpreterProvider } from '../contracts'; -import { IS_WINDOWS, fsReaddirAsync, getInterpreterDisplayName } from "../../../common/utils"; -import { PythonInterpreter } from '../index'; +import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; +import { IInterpreterVersionService } from '../../interpreterVersion'; +import { IS_WINDOWS, fsReaddirAsync } from "../../../common/utils"; import { lookForInterpretersInDirectory } from '../helpers'; import { workspace } from 'vscode'; const untildify = require('untildify'); -export class VirtualEnvProvider implements IInterpreterProvider { - public constructor(private knownSearchPaths: string[], private virtualEnvMgr: VirtualEnvironmentManager) { } +export class VirtualEnvProvider implements IInterpreterLocatorService { + public constructor(private knownSearchPaths: string[], + private virtualEnvMgr: VirtualEnvironmentManager, + private versionProvider: IInterpreterVersionService) { } public getInterpreters() { return this.suggestionsFromKnownVenvs(); } @@ -39,7 +41,7 @@ export class VirtualEnvProvider implements IInterpreterProvider { })); } private async getVirtualEnvDetails(interpreter: string): Promise { - const displayName = getInterpreterDisplayName(interpreter).catch(() => path.basename(interpreter)); + const displayName = this.versionProvider.getVersion(interpreter, path.basename(interpreter)); const virtualEnv = this.virtualEnvMgr.detect(interpreter); return Promise.all([displayName, virtualEnv]) .then(([displayName, virtualEnv]) => { diff --git a/src/client/interpreter/sources/providers/windowsRegistryProvider.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts similarity index 97% rename from src/client/interpreter/sources/providers/windowsRegistryProvider.ts rename to src/client/interpreter/locators/services/windowsRegistryService.ts index 23fe1e611281..37dc8ed2a9e1 100644 --- a/src/client/interpreter/sources/providers/windowsRegistryProvider.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -2,8 +2,7 @@ import * as path from 'path'; import * as _ from 'lodash'; import * as fs from 'fs-extra'; import { Architecture, Hive, IRegistry } from '../../../common/registry'; -import { IInterpreterProvider } from '../contracts'; -import { PythonInterpreter } from '../index'; +import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; const DefaultPythonExecutable = 'python.exe'; const CompaniesToIgnore = ['PYLAUNCHER']; @@ -16,7 +15,7 @@ type CompanyInterpreter = { arch?: Architecture }; -export class WindowsRegistryProvider implements IInterpreterProvider { +export class WindowsRegistryProvider implements IInterpreterLocatorService { constructor(private registry: IRegistry, private is64Bit: boolean) { } diff --git a/src/client/providers/setInterpreterProvider.ts b/src/client/providers/setInterpreterProvider.ts index 654e6d642485..6021a71be60f 100644 --- a/src/client/providers/setInterpreterProvider.ts +++ b/src/client/providers/setInterpreterProvider.ts @@ -2,7 +2,8 @@ import * as path from 'path'; import * as vscode from 'vscode'; import * as settings from './../common/configSettings'; -import { PythonInterpreter, InterpreterManager } from '../interpreter'; +import { InterpreterManager } from '../interpreter'; +import { PythonInterpreter } from '../interpreter/contracts'; interface PythonPathQuickPickItem extends vscode.QuickPickItem { diff --git a/src/test/interpreters/condaEnvFileService.test.ts b/src/test/interpreters/condaEnvFileService.test.ts new file mode 100644 index 000000000000..8f8c60d688cd --- /dev/null +++ b/src/test/interpreters/condaEnvFileService.test.ts @@ -0,0 +1,51 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import * as fs from 'fs-extra'; +import { EOL } from 'os'; +import { initialize } from '../initialize'; +import { MockInterpreterVersionProvider } from './mocks'; +import { CondaEnvFileProvider } from '../../client/interpreter/locators/services/condaEnvFileService'; +import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH, } from '../../client/interpreter/locators/services/conda'; + +const environmentsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'environments'); +const environmentsFilePath = path.join(environmentsPath, 'environments.txt'); + +suite('Interpreters from Conda Environments Text File', () => { + suiteSetup(done => { + initialize() + .then(() => done()) + .catch(() => done()); + }); + suiteTeardown(async () => { + // Clear the file so we don't get unwanted changes prompting for a checkin of this file + await updateEnvWithInterpreters([]); + }); + + async function updateEnvWithInterpreters(envs: string[]) { + await fs.writeFile(environmentsFilePath, envs.join(EOL), { flag: 'w' }); + } + test('Must return an empty list for an empty file', async () => { + await updateEnvWithInterpreters([]); + const displayNameProvider = new MockInterpreterVersionProvider('Mock Name'); + const condaFileProvider = new CondaEnvFileProvider(environmentsFilePath, displayNameProvider); + const interpreters = await condaFileProvider.getInterpreters(); + assert.equal(interpreters.length, 0, 'Incorrect number of entries'); + }); + test('Must return filter files in the list and return valid items', async () => { + const interpreterPaths = [ + path.join(environmentsPath, 'path1'), + path.join('Invalid and non existent'), + path.join(environmentsPath, 'path2'), + path.join(environmentsPath, 'conda', 'envs', 'numpy'), + path.join('Another Invalid and non existent') + ]; + await updateEnvWithInterpreters(interpreterPaths); + const displayNameProvider = new MockInterpreterVersionProvider('Mock Name'); + const condaFileProvider = new CondaEnvFileProvider(environmentsFilePath, displayNameProvider); + const interpreters = await condaFileProvider.getInterpreters(); + assert.equal(interpreters.length, 3, 'Incorrect number of entries'); + assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Name (path1)`, 'Incorrect display name'); + assert.equal(interpreters[1].companyDisplayName, AnacondaCompanyName, 'Incorrect display name'); + assert.equal(interpreters[1].path, path.join(interpreterPaths[2], ...CONDA_RELATIVE_PY_PATH), 'Incorrect company display name'); + }); +}); diff --git a/src/test/interpreters/condaEnvProvider.test.ts b/src/test/interpreters/condaEnvService.test.ts similarity index 98% rename from src/test/interpreters/condaEnvProvider.test.ts rename to src/test/interpreters/condaEnvService.test.ts index 18048b93d72b..dce9984ae75d 100644 --- a/src/test/interpreters/condaEnvProvider.test.ts +++ b/src/test/interpreters/condaEnvService.test.ts @@ -3,9 +3,9 @@ import * as path from 'path'; import * as settings from '../../client/common/configSettings'; import { initialize, setPythonExecutable } from '../initialize'; import { IS_WINDOWS } from '../../client/common/utils'; -import { CondaEnvProvider } from '../../client/interpreter/sources/providers/condaEnvProvider'; +import { CondaEnvProvider } from '../../client/interpreter/locators/services/condaEnvService'; import { MockProvider } from './mocks'; -import { PythonInterpreter } from '../../client/interpreter'; +import { PythonInterpreter } from '../../client/interpreter/contracts'; const pythonSettings = settings.PythonSettings.getInstance(); const originalPythonPath = pythonSettings.pythonPath; diff --git a/src/test/interpreters/display.test.ts b/src/test/interpreters/display.test.ts index b917bbf28869..ebbe64db64f3 100644 --- a/src/test/interpreters/display.test.ts +++ b/src/test/interpreters/display.test.ts @@ -1,17 +1,17 @@ import { initialize, setPythonExecutable } from '../initialize'; -import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; import * as assert from 'assert'; import * as child_process from 'child_process'; - import * as settings from '../../client/common/configSettings'; +import * as path from 'path'; +import * as utils from '../../client/common/utils'; import { MockStatusBarItem } from '../mockClasses'; +import { MockInterpreterVersionProvider } from './mocks'; import { InterpreterDisplay } from '../../client/interpreter/display'; import { MockProvider, MockVirtualEnv } from './mocks'; -import * as path from 'path'; import { EOL } from 'os'; -import * as utils from '../../client/common/utils'; -import { getFirstNonEmptyLineFromMultilineString } from '../../client/interpreter/sources/helpers'; +import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; +import { getFirstNonEmptyLineFromMultilineString } from '../../client/interpreter/helpers'; let pythonSettings = settings.PythonSettings.getInstance(); const originalPythonPath = pythonSettings.pythonPath; @@ -32,17 +32,18 @@ suite('Interpreters Display', () => { test('Must have command name', () => { const statusBar = new MockStatusBarItem(); - new InterpreterDisplay(statusBar, new MockProvider([]), new VirtualEnvironmentManager([])); + const displayNameProvider = new MockInterpreterVersionProvider(''); + new InterpreterDisplay(statusBar, new MockProvider([]), new VirtualEnvironmentManager([]), displayNameProvider); assert.equal(statusBar.command, 'python.setInterpreter', 'Incorrect command name'); }); test('Must get display name from interpreter itself', async () => { const statusBar = new MockStatusBarItem(); const provider = new MockProvider([]); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([])); + const displayName = 'Mock Display Name'; + const displayNameProvider = new MockInterpreterVersionProvider(displayName); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([]), displayNameProvider); await display.refresh(); - const pythonPath = pythonSettings.pythonPath; - const displayName = await getInterpreterDisplayName(pythonPath, ''); assert.equal(statusBar.text, displayName, 'Incorrect display name'); }); test('Must suffix display name with name of interpreter', async () => { @@ -51,24 +52,24 @@ suite('Interpreters Display', () => { const env1 = new MockVirtualEnv(false, 'Mock 1'); const env2 = new MockVirtualEnv(true, 'Mock 2'); const env3 = new MockVirtualEnv(true, 'Mock 3'); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([env1, env2, env3])); + const displayName = 'Mock Display Name'; + const displayNameProvider = new MockInterpreterVersionProvider(displayName); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([env1, env2, env3]), displayNameProvider); await display.refresh(); - const pythonPath = pythonSettings.pythonPath; - - const displayName = await getInterpreterDisplayName(pythonPath, ''); assert.equal(statusBar.text, `${displayName} (${env2.name})`, 'Incorrect display name'); }); test(`Must display default 'Display name' for unknown interpreter`, async () => { const statusBar = new MockStatusBarItem(); const provider = new MockProvider([]); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([])); + const displayName = 'Mock Display Name'; + const displayNameProvider = new MockInterpreterVersionProvider(displayName, true); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([]), displayNameProvider); // Change interpreter to an invalid value const pythonPath = pythonSettings.pythonPath = 'c:/some/unknonw/Python Interpreter.exe'; await display.refresh(); const defaultDisplayName = `${path.basename(pythonPath)} [Environment]`; - const displayName = await getInterpreterDisplayName(pythonPath, defaultDisplayName); - assert.equal(statusBar.text, displayName, 'Incorrect display name'); + assert.equal(statusBar.text, defaultDisplayName, 'Incorrect display name'); }); test('Must get display name from a list of interpreters', async () => { const statusBar = new MockStatusBarItem(); @@ -78,12 +79,12 @@ suite('Interpreters Display', () => { { displayName: 'Three', path: 'c:/path3/three.exe', type: 'Three 3' }, ]; const provider = new MockProvider(interpreters); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([])); - // Change interpreter to an invalid value - pythonSettings.pythonPath = 'c:/some/unknonw/Python Interpreter.exe'; + const displayName = 'Mock Display Name'; + const displayNameProvider = new MockInterpreterVersionProvider(displayName, true); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([]), displayNameProvider); await display.refresh(); - assert.equal(statusBar.text, '$(alert) Select Python Environment', 'Incorrect display name'); + assert.equal(statusBar.text, interpreters[1].displayName, 'Incorrect display name'); }); test('Must suffix tooltip with the companyDisplayName of interpreter', async () => { const pythonPath = await new Promise(resolve => { @@ -99,7 +100,8 @@ suite('Interpreters Display', () => { { displayName: 'Three', path: 'c:/path3/three.exe', companyDisplayName: 'Three 3' }, ]; const provider = new MockProvider(interpreters); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([])); + const displayNameProvider = new MockInterpreterVersionProvider(''); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([]), displayNameProvider); await display.refresh(); assert.equal(statusBar.text, interpreters[1].displayName, 'Incorrect display name'); @@ -113,7 +115,8 @@ suite('Interpreters Display', () => { { displayName: 'Three', path: 'c:/path3/three.exe', companyDisplayName: 'Three 3' }, ]; const provider = new MockProvider(interpreters); - const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([])); + const displayNameProvider = new MockInterpreterVersionProvider('', true); + const display = new InterpreterDisplay(statusBar, provider, new VirtualEnvironmentManager([]), displayNameProvider); // Change interpreter to an invalid value pythonSettings.pythonPath = 'c:/some/unknonw/Python Interpreter.exe'; await display.refresh(); diff --git a/src/test/interpreters/mocks.ts b/src/test/interpreters/mocks.ts index 9ba3aeb56d90..c4dde62edf6c 100644 --- a/src/test/interpreters/mocks.ts +++ b/src/test/interpreters/mocks.ts @@ -1,8 +1,9 @@ +import { IInterpreterVersionService } from '../../client/interpreter/interpreterVersion'; import { IVirtualEnvironment } from '../../client/interpreter/virtualEnvs/contracts'; -import { IInterpreterProvider, PythonInterpreter } from "../../client/interpreter/index"; +import { IInterpreterLocatorService, PythonInterpreter } from "../../client/interpreter/contracts"; import { IRegistry, Hive, Architecture } from "../../client/common/registry"; -export class MockProvider implements IInterpreterProvider { +export class MockProvider implements IInterpreterLocatorService { constructor(private suggestions: PythonInterpreter[]) { } getInterpreters(): Promise { @@ -48,4 +49,11 @@ export class MockVirtualEnv implements IVirtualEnvironment { detect(pythonPath: string): Promise { return Promise.resolve(this.isDetected); } -} \ No newline at end of file +} + +export class MockInterpreterVersionProvider implements IInterpreterVersionService { + constructor(private displayName: string, private useDefaultDisplayName: boolean = false) { } + getVersion(pythonPath: string, defaultDisplayName: string): Promise { + return this.useDefaultDisplayName ? Promise.resolve(defaultDisplayName) : Promise.resolve(this.displayName); + } +} diff --git a/src/test/interpreters/windowsRegistryProvider.test.ts b/src/test/interpreters/windowsRegistryService.test.ts similarity index 99% rename from src/test/interpreters/windowsRegistryProvider.test.ts rename to src/test/interpreters/windowsRegistryService.test.ts index cbbac3c7b95b..282d29a91179 100644 --- a/src/test/interpreters/windowsRegistryProvider.test.ts +++ b/src/test/interpreters/windowsRegistryService.test.ts @@ -3,7 +3,7 @@ import * as assert from 'assert'; import * as path from 'path'; import * as settings from '../../client/common/configSettings'; import { IS_WINDOWS } from '../../client/debugger/Common/Utils'; -import { WindowsRegistryProvider } from '../../client/interpreter/sources/providers/windowsRegistryProvider'; +import { WindowsRegistryProvider } from '../../client/interpreter/locators/services/windowsRegistryService'; import { MockRegistry } from './mocks'; import { Architecture, Hive } from '../../client/common/registry'; diff --git a/src/test/pythonFiles/environments/environments.txt b/src/test/pythonFiles/environments/environments.txt new file mode 100644 index 000000000000..e69de29bb2d1 From ee76e9b8987eb33f929e5ba47e8a83ffd89465d5 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 09:49:04 -0700 Subject: [PATCH 2/9] add support for linux --- .../locators/services/condaEnvFileService.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts index 3980d1207e40..062abf8c2928 100644 --- a/src/client/interpreter/locators/services/condaEnvFileService.ts +++ b/src/client/interpreter/locators/services/condaEnvFileService.ts @@ -5,6 +5,14 @@ import { IInterpreterVersionService } from '../../interpreterVersion'; import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH } from './conda'; +/** + * The command 'conda info --envs' doesn't seem to return all environments + * Environments created using the command 'conda create --p python=x.x' are not returned by the above command + * However all these environments seem to be listed in the environments.txt file (confirmed on windows and linux) + * @export + * @class CondaEnvFileProvider + * @implements {IInterpreterLocatorService} + */ export class CondaEnvFileProvider implements IInterpreterLocatorService { constructor(private condaEnvironmentFile: string, private versionService: IInterpreterVersionService) { @@ -56,6 +64,6 @@ export class CondaEnvFileProvider implements IInterpreterLocatorService { } export function getEnvironmentsFile() { - const profileDir = process.env['USERPROFILE']; - return profileDir ? path.join(profileDir, '.conda', 'environments.txt') : ''; + const homeDir = process.env.HOME || process.env.HOMEPATH || process.env.USERPROFILE; + return homeDir ? path.join(homeDir, '.conda', 'environments.txt') : ''; } From 37f6332defb4c1901f41c3ff20afb7cab24b366c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 09:57:40 -0700 Subject: [PATCH 3/9] fixed tests --- src/test/interpreters/condaEnvFileService.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/interpreters/condaEnvFileService.test.ts b/src/test/interpreters/condaEnvFileService.test.ts index 8f8c60d688cd..969c378dced9 100644 --- a/src/test/interpreters/condaEnvFileService.test.ts +++ b/src/test/interpreters/condaEnvFileService.test.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import * as fs from 'fs-extra'; import { EOL } from 'os'; import { initialize } from '../initialize'; +import { IS_WINDOWS } from '../../client/common/utils'; import { MockInterpreterVersionProvider } from './mocks'; import { CondaEnvFileProvider } from '../../client/interpreter/locators/services/condaEnvFileService'; import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH, } from '../../client/interpreter/locators/services/conda'; @@ -33,19 +34,21 @@ suite('Interpreters from Conda Environments Text File', () => { }); test('Must return filter files in the list and return valid items', async () => { const interpreterPaths = [ + path.join(environmentsPath, 'conda', 'envs', 'numpy'), path.join(environmentsPath, 'path1'), path.join('Invalid and non existent'), path.join(environmentsPath, 'path2'), - path.join(environmentsPath, 'conda', 'envs', 'numpy'), path.join('Another Invalid and non existent') ]; await updateEnvWithInterpreters(interpreterPaths); const displayNameProvider = new MockInterpreterVersionProvider('Mock Name'); const condaFileProvider = new CondaEnvFileProvider(environmentsFilePath, displayNameProvider); const interpreters = await condaFileProvider.getInterpreters(); - assert.equal(interpreters.length, 3, 'Incorrect number of entries'); - assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Name (path1)`, 'Incorrect display name'); - assert.equal(interpreters[1].companyDisplayName, AnacondaCompanyName, 'Incorrect display name'); - assert.equal(interpreters[1].path, path.join(interpreterPaths[2], ...CONDA_RELATIVE_PY_PATH), 'Incorrect company display name'); + // This is because conda environments will be under 'bin/python' however the folders path1 and path2 do not have such files + const numberOfEnvs = IS_WINDOWS ? 3 : 1; + assert.equal(interpreters.length, numberOfEnvs, 'Incorrect number of entries'); + assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Name (numpy)`, 'Incorrect display name'); + assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect display name'); + assert.equal(interpreters[0].path, path.join(interpreterPaths[0], ...CONDA_RELATIVE_PY_PATH), 'Incorrect company display name'); }); }); From fd26b238c253df1fc2e9c9746a4275d77cb0888a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 09:59:36 -0700 Subject: [PATCH 4/9] fixed typo --- src/client/interpreter/locators/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts index b8fe5306782e..9ada1cc4342c 100644 --- a/src/client/interpreter/locators/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -8,7 +8,7 @@ import { RegistryImplementation } from '../../common/registry'; import { CondaEnvProvider } from './services/condaEnvService'; import { VirtualEnvProvider, getKnownSearchPathsForVirtualEnvs } from './services/virtualEnvService'; import { KnownPathsProvider, getKnownSearchPathsForInterpreters } from './services/KnownPathsService'; -import { CurrentPathProvider } from './services/CurrentPathService'; +import { CurrentPathProvider } from './services/currentPathService'; import { WindowsRegistryProvider } from './services/windowsRegistryService'; import { VirtualEnvironmentManager } from '../virtualEnvs'; import { CondaEnvFileProvider, getEnvironmentsFile as getCondaEnvFile } from './services/condaEnvFileService'; @@ -23,11 +23,12 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi const windowsRegistryProvider = new WindowsRegistryProvider(new RegistryImplementation(), Is_64Bit); this.locators.push(windowsRegistryProvider); this.locators.push(new CondaEnvProvider(windowsRegistryProvider)); - this.locators.push(new CondaEnvFileProvider(getCondaEnvFile(), versionService)); } else { this.locators.push(new CondaEnvProvider()); } + // Supplements the above list of conda environments + this.locators.push(new CondaEnvFileProvider(getCondaEnvFile(), versionService)); this.locators.push(new VirtualEnvProvider(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr, versionService)); if (!IS_WINDOWS) { From d5298ba82ce43fe9ded45156ca25ccb5b746cb21 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 10:18:04 -0700 Subject: [PATCH 5/9] fix test on linux --- src/test/interpreters/condaEnvService.test.ts | 10 ++++++++-- src/test/interpreters/display.test.ts | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/test/interpreters/condaEnvService.test.ts b/src/test/interpreters/condaEnvService.test.ts index dce9984ae75d..520455382153 100644 --- a/src/test/interpreters/condaEnvService.test.ts +++ b/src/test/interpreters/condaEnvService.test.ts @@ -105,8 +105,14 @@ suite('Interpreters from Conda Environments', () => { const interpreters = await condaProvider.parseCondaInfo(info); assert.equal(interpreters.length, 2, 'Incorrect number of entries'); - assert.equal(path.dirname(interpreters[0].path), info.envs[0], 'Incorrect path for first env'); - assert.equal(path.dirname(interpreters[1].path), info.envs[4], 'Incorrect path for second env'); + // Go up one dir for linux (cuz exe is under a sub directory named 'bin') + let path0 = path.dirname(interpreters[0].path); + path0 = IS_WINDOWS ? path0 : path.dirname(path0); + assert.equal(path0, info.envs[0], 'Incorrect path for first env'); + // Go up one dir for linux (cuz exe is under a sub directory named 'bin') + let path1 = path.dirname(interpreters[1].path); + path1 = IS_WINDOWS ? path1 : path.dirname(path1); + assert.equal(path1, info.envs[4], 'Incorrect path for second env'); }); test('Must detect conda environments from a list', async () => { const registryInterpreters: PythonInterpreter[] = [ diff --git a/src/test/interpreters/display.test.ts b/src/test/interpreters/display.test.ts index ebbe64db64f3..5a3669289249 100644 --- a/src/test/interpreters/display.test.ts +++ b/src/test/interpreters/display.test.ts @@ -72,10 +72,15 @@ suite('Interpreters Display', () => { assert.equal(statusBar.text, defaultDisplayName, 'Incorrect display name'); }); test('Must get display name from a list of interpreters', async () => { + const pythonPath = await new Promise(resolve => { + child_process.execFile(pythonSettings.pythonPath, ["-c", "import sys;print(sys.executable)"], (_, stdout) => { + resolve(getFirstNonEmptyLineFromMultilineString(stdout)); + }); + }).then(value => value.length === 0 ? pythonSettings.pythonPath : value); const statusBar = new MockStatusBarItem(); const interpreters = [ { displayName: 'One', path: 'c:/path1/one.exe', type: 'One 1' }, - { displayName: 'Two', path: pythonSettings.pythonPath, type: 'Two 2' }, + { displayName: 'Two', path: pythonPath, type: 'Two 2' }, { displayName: 'Three', path: 'c:/path3/three.exe', type: 'Three 3' }, ]; const provider = new MockProvider(interpreters); From bdb0071e3b34460e70b2df710489de65fbd200af Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 10:23:46 -0700 Subject: [PATCH 6/9] renamed provider to service --- src/client/interpreter/locators/index.ts | 26 +++++++++---------- .../locators/services/KnownPathsService.ts | 2 +- .../locators/services/condaEnvFileService.ts | 2 +- .../locators/services/condaEnvService.ts | 2 +- .../locators/services/currentPathService.ts | 2 +- .../locators/services/virtualEnvService.ts | 2 +- .../services/windowsRegistryService.ts | 2 +- .../interpreters/condaEnvFileService.test.ts | 6 ++--- src/test/interpreters/condaEnvService.test.ts | 22 ++++++++-------- .../windowsRegistryService.test.ts | 20 +++++++------- 10 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts index 9ada1cc4342c..500ed20062f6 100644 --- a/src/client/interpreter/locators/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -5,13 +5,13 @@ import { IInterpreterLocatorService, PythonInterpreter } from '../contracts'; import { InterpreterVersionService } from '../interpreterVersion'; import { IS_WINDOWS, Is_64Bit, arePathsSame, areBasePathsSame } from '../../common/utils'; import { RegistryImplementation } from '../../common/registry'; -import { CondaEnvProvider } from './services/condaEnvService'; -import { VirtualEnvProvider, getKnownSearchPathsForVirtualEnvs } from './services/virtualEnvService'; -import { KnownPathsProvider, getKnownSearchPathsForInterpreters } from './services/KnownPathsService'; -import { CurrentPathProvider } from './services/currentPathService'; -import { WindowsRegistryProvider } from './services/windowsRegistryService'; +import { CondaEnvService } from './services/condaEnvService'; +import { VirtualEnvService, getKnownSearchPathsForVirtualEnvs } from './services/virtualEnvService'; +import { KnownPathsService, getKnownSearchPathsForInterpreters } from './services/KnownPathsService'; +import { CurrentPathService } from './services/currentPathService'; +import { WindowsRegistryService } from './services/windowsRegistryService'; import { VirtualEnvironmentManager } from '../virtualEnvs'; -import { CondaEnvFileProvider, getEnvironmentsFile as getCondaEnvFile } from './services/condaEnvFileService'; +import { CondaEnvFileService, getEnvironmentsFile as getCondaEnvFile } from './services/condaEnvFileService'; export class PythonInterpreterLocatorService implements IInterpreterLocatorService { private interpreters: PythonInterpreter[] = []; @@ -20,25 +20,25 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi const versionService = new InterpreterVersionService(); // The order of the services is important if (IS_WINDOWS) { - const windowsRegistryProvider = new WindowsRegistryProvider(new RegistryImplementation(), Is_64Bit); + const windowsRegistryProvider = new WindowsRegistryService(new RegistryImplementation(), Is_64Bit); this.locators.push(windowsRegistryProvider); - this.locators.push(new CondaEnvProvider(windowsRegistryProvider)); + this.locators.push(new CondaEnvService(windowsRegistryProvider)); } else { - this.locators.push(new CondaEnvProvider()); + this.locators.push(new CondaEnvService()); } // Supplements the above list of conda environments - this.locators.push(new CondaEnvFileProvider(getCondaEnvFile(), versionService)); - this.locators.push(new VirtualEnvProvider(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr, versionService)); + this.locators.push(new CondaEnvFileService(getCondaEnvFile(), versionService)); + this.locators.push(new VirtualEnvService(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr, versionService)); if (!IS_WINDOWS) { // This must be last, it is possible we have paths returned here that are already returned // in one of the above lists - this.locators.push(new KnownPathsProvider(getKnownSearchPathsForInterpreters(), versionService)); + this.locators.push(new KnownPathsService(getKnownSearchPathsForInterpreters(), versionService)); } // This must be last, it is possible we have paths returned here that are already returned // in one of the above lists - this.locators.push(new CurrentPathProvider(this.virtualEnvMgr, versionService)); + this.locators.push(new CurrentPathService(this.virtualEnvMgr, versionService)); } public getInterpreters() { if (this.interpreters.length > 0) { diff --git a/src/client/interpreter/locators/services/KnownPathsService.ts b/src/client/interpreter/locators/services/KnownPathsService.ts index 4235deb5c906..15bc477c810d 100644 --- a/src/client/interpreter/locators/services/KnownPathsService.ts +++ b/src/client/interpreter/locators/services/KnownPathsService.ts @@ -7,7 +7,7 @@ import { fsExistsAsync, IS_WINDOWS } from '../../../common/utils'; import { lookForInterpretersInDirectory } from '../helpers'; const untildify = require('untildify'); -export class KnownPathsProvider implements IInterpreterLocatorService { +export class KnownPathsService implements IInterpreterLocatorService { public constructor(private knownSearchPaths: string[], private versionProvider: IInterpreterVersionService) { } public getInterpreters() { diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts index 062abf8c2928..7a7c6edf6df8 100644 --- a/src/client/interpreter/locators/services/condaEnvFileService.ts +++ b/src/client/interpreter/locators/services/condaEnvFileService.ts @@ -13,7 +13,7 @@ import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH } from * @class CondaEnvFileProvider * @implements {IInterpreterLocatorService} */ -export class CondaEnvFileProvider implements IInterpreterLocatorService { +export class CondaEnvFileService implements IInterpreterLocatorService { constructor(private condaEnvironmentFile: string, private versionService: IInterpreterVersionService) { } diff --git a/src/client/interpreter/locators/services/condaEnvService.ts b/src/client/interpreter/locators/services/condaEnvService.ts index f9967f03aea2..da7cbb8f2677 100644 --- a/src/client/interpreter/locators/services/condaEnvService.ts +++ b/src/client/interpreter/locators/services/condaEnvService.ts @@ -11,7 +11,7 @@ type CondaInfo = { "sys.version"?: string; default_prefix?: string; } -export class CondaEnvProvider implements IInterpreterLocatorService { +export class CondaEnvService implements IInterpreterLocatorService { constructor(private registryLookupForConda?: IInterpreterLocatorService) { } public getInterpreters() { diff --git a/src/client/interpreter/locators/services/currentPathService.ts b/src/client/interpreter/locators/services/currentPathService.ts index e2154ea3e850..e2ddf45e4fba 100644 --- a/src/client/interpreter/locators/services/currentPathService.ts +++ b/src/client/interpreter/locators/services/currentPathService.ts @@ -10,7 +10,7 @@ import { PythonSettings } from '../../../common/configSettings'; const settings = PythonSettings.getInstance(); -export class CurrentPathProvider implements IInterpreterLocatorService { +export class CurrentPathService implements IInterpreterLocatorService { public constructor(private virtualEnvMgr: VirtualEnvironmentManager, private versionProvider: IInterpreterVersionService) { } public getInterpreters() { diff --git a/src/client/interpreter/locators/services/virtualEnvService.ts b/src/client/interpreter/locators/services/virtualEnvService.ts index 41bf0eb95ed9..64e3b7a3d4e9 100644 --- a/src/client/interpreter/locators/services/virtualEnvService.ts +++ b/src/client/interpreter/locators/services/virtualEnvService.ts @@ -10,7 +10,7 @@ import { lookForInterpretersInDirectory } from '../helpers'; import { workspace } from 'vscode'; const untildify = require('untildify'); -export class VirtualEnvProvider implements IInterpreterLocatorService { +export class VirtualEnvService implements IInterpreterLocatorService { public constructor(private knownSearchPaths: string[], private virtualEnvMgr: VirtualEnvironmentManager, private versionProvider: IInterpreterVersionService) { } diff --git a/src/client/interpreter/locators/services/windowsRegistryService.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts index 37dc8ed2a9e1..dc021c12f197 100644 --- a/src/client/interpreter/locators/services/windowsRegistryService.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -15,7 +15,7 @@ type CompanyInterpreter = { arch?: Architecture }; -export class WindowsRegistryProvider implements IInterpreterLocatorService { +export class WindowsRegistryService implements IInterpreterLocatorService { constructor(private registry: IRegistry, private is64Bit: boolean) { } diff --git a/src/test/interpreters/condaEnvFileService.test.ts b/src/test/interpreters/condaEnvFileService.test.ts index 969c378dced9..bc25e0ae2b07 100644 --- a/src/test/interpreters/condaEnvFileService.test.ts +++ b/src/test/interpreters/condaEnvFileService.test.ts @@ -5,7 +5,7 @@ import { EOL } from 'os'; import { initialize } from '../initialize'; import { IS_WINDOWS } from '../../client/common/utils'; import { MockInterpreterVersionProvider } from './mocks'; -import { CondaEnvFileProvider } from '../../client/interpreter/locators/services/condaEnvFileService'; +import { CondaEnvFileService } from '../../client/interpreter/locators/services/condaEnvFileService'; import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH, } from '../../client/interpreter/locators/services/conda'; const environmentsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'environments'); @@ -28,7 +28,7 @@ suite('Interpreters from Conda Environments Text File', () => { test('Must return an empty list for an empty file', async () => { await updateEnvWithInterpreters([]); const displayNameProvider = new MockInterpreterVersionProvider('Mock Name'); - const condaFileProvider = new CondaEnvFileProvider(environmentsFilePath, displayNameProvider); + const condaFileProvider = new CondaEnvFileService(environmentsFilePath, displayNameProvider); const interpreters = await condaFileProvider.getInterpreters(); assert.equal(interpreters.length, 0, 'Incorrect number of entries'); }); @@ -42,7 +42,7 @@ suite('Interpreters from Conda Environments Text File', () => { ]; await updateEnvWithInterpreters(interpreterPaths); const displayNameProvider = new MockInterpreterVersionProvider('Mock Name'); - const condaFileProvider = new CondaEnvFileProvider(environmentsFilePath, displayNameProvider); + const condaFileProvider = new CondaEnvFileService(environmentsFilePath, displayNameProvider); const interpreters = await condaFileProvider.getInterpreters(); // This is because conda environments will be under 'bin/python' however the folders path1 and path2 do not have such files const numberOfEnvs = IS_WINDOWS ? 3 : 1; diff --git a/src/test/interpreters/condaEnvService.test.ts b/src/test/interpreters/condaEnvService.test.ts index 520455382153..adebde109f41 100644 --- a/src/test/interpreters/condaEnvService.test.ts +++ b/src/test/interpreters/condaEnvService.test.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import * as settings from '../../client/common/configSettings'; import { initialize, setPythonExecutable } from '../initialize'; import { IS_WINDOWS } from '../../client/common/utils'; -import { CondaEnvProvider } from '../../client/interpreter/locators/services/condaEnvService'; +import { CondaEnvService } from '../../client/interpreter/locators/services/condaEnvService'; import { MockProvider } from './mocks'; import { PythonInterpreter } from '../../client/interpreter/contracts'; @@ -26,12 +26,12 @@ suite('Interpreters from Conda Environments', () => { }); test('Must return an empty list for empty json', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const interpreters = await condaProvider.parseCondaInfo({} as any) assert.equal(interpreters.length, 0, 'Incorrect number of entries'); }); test('Must extract display name from version info', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const info = { envs: [path.join(environmentsPath, 'conda', 'envs', 'numpy'), path.join(environmentsPath, 'conda', 'envs', 'scipy')], @@ -52,7 +52,7 @@ suite('Interpreters from Conda Environments', () => { assert.equal(interpreters[1].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); }); test('Must use the default display name if sys.version is invalid', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const info = { envs: [path.join(environmentsPath, 'conda', 'envs', 'numpy')], default_prefix: '', @@ -67,7 +67,7 @@ suite('Interpreters from Conda Environments', () => { assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); }); test('Must use the default display name if sys.version is empty', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const info = { envs: [path.join(environmentsPath, 'conda', 'envs', 'numpy')], }; @@ -80,7 +80,7 @@ suite('Interpreters from Conda Environments', () => { assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); }); test('Must include the default_prefix into the list of interpreters', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const info = { default_prefix: path.join(environmentsPath, 'conda', 'envs', 'numpy'), }; @@ -93,7 +93,7 @@ suite('Interpreters from Conda Environments', () => { assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); }); test('Must exclude interpreters that do not exist on disc', async () => { - const condaProvider = new CondaEnvProvider(); + const condaProvider = new CondaEnvService(); const info = { envs: [path.join(environmentsPath, 'conda', 'envs', 'numpy'), path.join(environmentsPath, 'path0', 'one.exe'), @@ -125,7 +125,7 @@ suite('Interpreters from Conda Environments', () => { { displayName: 'xnaconda', path: path.join(environmentsPath, 'path2', 'one.exe'), companyDisplayName: 'Continuum Analytics, Inc.' }, ]; const mockRegistryProvider = new MockProvider(registryInterpreters); - const condaProvider = new CondaEnvProvider(mockRegistryProvider); + const condaProvider = new CondaEnvService(mockRegistryProvider); assert.equal(condaProvider.isCondaEnvironment(registryInterpreters[0]), false, '1. Identified environment incorrectly'); assert.equal(condaProvider.isCondaEnvironment(registryInterpreters[1]), false, '2. Identified environment incorrectly'); @@ -146,7 +146,7 @@ suite('Interpreters from Conda Environments', () => { { displayName: 'Seven', path: path.join(environmentsPath, 'conda', 'envs', 'numpy'), companyDisplayName: 'Continuum Analytics, Inc.' }, ]; const mockRegistryProvider = new MockProvider(registryInterpreters); - const condaProvider = new CondaEnvProvider(mockRegistryProvider); + const condaProvider = new CondaEnvService(mockRegistryProvider); assert.equal(condaProvider.getLatestVersion(registryInterpreters)!.displayName, 'Two', 'Failed to identify latest version'); }); @@ -161,7 +161,7 @@ suite('Interpreters from Conda Environments', () => { { displayName: 'Seven', path: path.join(environmentsPath, 'conda', 'envs', 'numpy'), companyDisplayName: 'Continuum Analytics, Inc.' }, ]; const mockRegistryProvider = new MockProvider(registryInterpreters); - const condaProvider = new CondaEnvProvider(mockRegistryProvider); + const condaProvider = new CondaEnvService(mockRegistryProvider); assert.equal(condaProvider.getLatestVersion(registryInterpreters)!.displayName, 'Two', 'Failed to identify latest version'); }); @@ -174,7 +174,7 @@ suite('Interpreters from Conda Environments', () => { { displayName: 'Seven', path: path.join(environmentsPath, 'conda', 'envs', 'numpy'), companyDisplayName: 'Continuum Analytics, Inc.' }, ]; const mockRegistryProvider = new MockProvider(registryInterpreters); - const condaProvider = new CondaEnvProvider(mockRegistryProvider); + const condaProvider = new CondaEnvService(mockRegistryProvider); const condaExe = await condaProvider.getCondaFile(); assert.equal(condaExe, path.join(path.dirname(condaPythonExePath), 'conda.exe'), 'Failed to identify conda.exe'); diff --git a/src/test/interpreters/windowsRegistryService.test.ts b/src/test/interpreters/windowsRegistryService.test.ts index 282d29a91179..fc292b8031e6 100644 --- a/src/test/interpreters/windowsRegistryService.test.ts +++ b/src/test/interpreters/windowsRegistryService.test.ts @@ -3,7 +3,7 @@ import * as assert from 'assert'; import * as path from 'path'; import * as settings from '../../client/common/configSettings'; import { IS_WINDOWS } from '../../client/debugger/Common/Utils'; -import { WindowsRegistryProvider } from '../../client/interpreter/locators/services/windowsRegistryService'; +import { WindowsRegistryService } from '../../client/interpreter/locators/services/windowsRegistryService'; import { MockRegistry } from './mocks'; import { Architecture, Hive } from '../../client/common/registry'; @@ -28,14 +28,14 @@ suite('Interpreters from Windows Registry', () => { if (IS_WINDOWS) { test('Must return an empty list (x86)', async () => { const registry = new MockRegistry([], []); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); assert.equal(interpreters.length, 0, 'Incorrect number of entries'); }); test('Must return an empty list (x64)', async () => { const registry = new MockRegistry([], []); - const winRegistry = new WindowsRegistryProvider(registry, true); + const winRegistry = new WindowsRegistryService(registry, true); const interpreters = await winRegistry.getInterpreters(); assert.equal(interpreters.length, 0, 'Incorrect number of entries'); @@ -53,7 +53,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\Company One\\Tag1', hive: Hive.HKCU, arch: Architecture.x86, value: 'DisplayName.Tag1', name: 'DisplayName' }, ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -73,7 +73,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\PythonCore\\Tag1\\InstallPath', hive: Hive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -93,7 +93,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\PyLauncher\\Tag1\\InstallPath', hive: Hive.HKCU, arch: Architecture.x86, value: 'c:/temp/Install Path Tag1' } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -108,7 +108,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\Company One\\Tag1\\InstallPath', hive: Hive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -150,7 +150,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\Company A\\Another Tag\\InstallPath', hive: Hive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'conda', 'envs', 'scipy', 'python.exe') } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -208,7 +208,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\Company A\\Another Tag\\InstallPath', hive: Hive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'conda', 'envs', 'numpy') } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); @@ -266,7 +266,7 @@ suite('Interpreters from Windows Registry', () => { { key: '\\Software\\Python\\Company A\\Another Tag\\InstallPath', hive: Hive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'non-existent-path', 'envs', 'numpy') } ]; const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryProvider(registry, false); + const winRegistry = new WindowsRegistryService(registry, false); const interpreters = await winRegistry.getInterpreters(); From 690aaa9ecc23b6d1fbcd8f79c3d30dc955f6c081 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 10:28:15 -0700 Subject: [PATCH 7/9] oopsy --- src/client/interpreter/interpreterVersion.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/interpreterVersion.ts b/src/client/interpreter/interpreterVersion.ts index be5684e584ea..06f858dec24b 100644 --- a/src/client/interpreter/interpreterVersion.ts +++ b/src/client/interpreter/interpreterVersion.ts @@ -8,6 +8,6 @@ export interface IInterpreterVersionService { export class InterpreterVersionService implements IInterpreterVersionService { getVersion(pythonPath: string, defaultValue: string): Promise { return getInterpreterDisplayName(pythonPath) - .catch(() => path.basename(defaultValue)); + .catch(() => defaultValue); } } \ No newline at end of file From 2fa2475507fd7ba1626cf7198b9c31fd0f04fc39 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 13:45:25 -0700 Subject: [PATCH 8/9] code review fixes --- src/client/interpreter/helpers.ts | 4 +- src/client/interpreter/index.ts | 6 +-- src/client/interpreter/interpreterVersion.ts | 3 +- src/client/interpreter/locators/helpers.ts | 3 +- src/client/interpreter/locators/index.ts | 24 +++++------ .../locators/services/KnownPathsService.ts | 2 +- .../interpreter/locators/services/conda.ts | 5 ++- .../locators/services/condaEnvFileService.ts | 41 +++++++++---------- .../locators/services/condaEnvService.ts | 14 +++---- .../services/windowsRegistryService.ts | 8 ++-- .../interpreters/condaEnvFileService.test.ts | 23 ++++++++++- src/test/interpreters/condaEnvService.test.ts | 11 ++--- 12 files changed, 81 insertions(+), 63 deletions(-) diff --git a/src/client/interpreter/helpers.ts b/src/client/interpreter/helpers.ts index 9b135ce748f8..7bd180b5b067 100644 --- a/src/client/interpreter/helpers.ts +++ b/src/client/interpreter/helpers.ts @@ -1,7 +1,7 @@ export function getFirstNonEmptyLineFromMultilineString(stdout: string) { - if (stdout.length === 0) { + if (!stdout) { return ''; } - const lines = stdout.split(/\r?\n/g).filter(line => line.trim().length > 0); + const lines = stdout.split(/\r?\n/g).map(line => line.trim()).filter(line => line.length > 0); return lines.length > 0 ? lines[0] : ''; } diff --git a/src/client/interpreter/index.ts b/src/client/interpreter/index.ts index 9ca434473700..447946d7871c 100644 --- a/src/client/interpreter/index.ts +++ b/src/client/interpreter/index.ts @@ -42,9 +42,9 @@ export class InterpreterManager implements Disposable { return; } - // Ensure this new environment is at the same level as the current workspace - // In windows the interpreter is under scripts/python.exe on linux it is under bin/python - // Meaning the sub directory must be either scripts, bin or other (but only one level deep) + // Ensure this new environment is at the same level as the current workspace. + // In windows the interpreter is under scripts/python.exe on linux it is under bin/python. + // Meaning the sub directory must be either scripts, bin or other (but only one level deep). const pythonPath = interpretersInWorkspace[0].path; const relativePath = path.dirname(pythonPath).substring(workspace.rootPath!.length); if (relativePath.split(path.sep).filter(l => l.length > 0).length === 2) { diff --git a/src/client/interpreter/interpreterVersion.ts b/src/client/interpreter/interpreterVersion.ts index 06f858dec24b..808072270bf4 100644 --- a/src/client/interpreter/interpreterVersion.ts +++ b/src/client/interpreter/interpreterVersion.ts @@ -1,4 +1,3 @@ -import * as path from 'path'; import { getInterpreterDisplayName } from '../common/utils'; export interface IInterpreterVersionService { @@ -10,4 +9,4 @@ export class InterpreterVersionService implements IInterpreterVersionService { return getInterpreterDisplayName(pythonPath) .catch(() => defaultValue); } -} \ No newline at end of file +} diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index 9f12c850a972..1efaeca88dff 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -19,9 +19,8 @@ export function fixInterpreterDisplayName(item: PythonInterpreter) { return item; } export function fixInterpreterPath(item: PythonInterpreter) { - // For some reason anaconda seems to use \\ in the registry path + // For some reason anaconda seems to use \\ in the registry path. item.path = IS_WINDOWS ? item.path.replace(/\\\\/g, "\\") : item.path; - // Also ensure paths have back slashes instead of forward item.path = IS_WINDOWS ? item.path.replace(/\//g, "\\") : item.path; return item; } diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts index 500ed20062f6..d12e527dcb13 100644 --- a/src/client/interpreter/locators/index.ts +++ b/src/client/interpreter/locators/index.ts @@ -18,7 +18,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi private locators: IInterpreterLocatorService[] = []; constructor(private virtualEnvMgr: VirtualEnvironmentManager) { const versionService = new InterpreterVersionService(); - // The order of the services is important + // The order of the services is important. if (IS_WINDOWS) { const windowsRegistryProvider = new WindowsRegistryService(new RegistryImplementation(), Is_64Bit); this.locators.push(windowsRegistryProvider); @@ -27,35 +27,35 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi else { this.locators.push(new CondaEnvService()); } - // Supplements the above list of conda environments + // Supplements the above list of conda environments. this.locators.push(new CondaEnvFileService(getCondaEnvFile(), versionService)); this.locators.push(new VirtualEnvService(getKnownSearchPathsForVirtualEnvs(), this.virtualEnvMgr, versionService)); if (!IS_WINDOWS) { // This must be last, it is possible we have paths returned here that are already returned - // in one of the above lists + // in one of the above lists. this.locators.push(new KnownPathsService(getKnownSearchPathsForInterpreters(), versionService)); } // This must be last, it is possible we have paths returned here that are already returned - // in one of the above lists + // in one of the above lists. this.locators.push(new CurrentPathService(this.virtualEnvMgr, versionService)); } - public getInterpreters() { + public async getInterpreters() { if (this.interpreters.length > 0) { - return Promise.resolve(this.interpreters); + return this.interpreters; } const promises = this.locators.map(provider => provider.getInterpreters()); return Promise.all(promises) .then(interpreters => _.flatten(interpreters)) .then(items => items.map(fixInterpreterDisplayName)) .then(items => items.map(fixInterpreterPath)) - .then(items => items.reduce((prev, current) => { - if (prev.findIndex(item => arePathsSame(item.path, current.path)) === -1 && - prev.findIndex(item => areBasePathsSame(item.path, current.path)) === -1) { - prev.push(current); + .then(items => items.reduce((accumulator, current) => { + if (accumulator.findIndex(item => arePathsSame(item.path, current.path)) === -1 && + accumulator.findIndex(item => areBasePathsSame(item.path, current.path)) === -1) { + accumulator.push(current); } - return prev; + return accumulator; }, [])) .then(interpreters => this.interpreters = interpreters); } -} \ No newline at end of file +} diff --git a/src/client/interpreter/locators/services/KnownPathsService.ts b/src/client/interpreter/locators/services/KnownPathsService.ts index 15bc477c810d..bbfdea9ab4cd 100644 --- a/src/client/interpreter/locators/services/KnownPathsService.ts +++ b/src/client/interpreter/locators/services/KnownPathsService.ts @@ -44,7 +44,7 @@ export function getKnownSearchPathsForInterpreters(): string[] { paths.forEach(p => { paths.push(untildify('~' + p)); }); - // Add support for paths such as /Users/xxx/anaconda/bin + // Add support for paths such as /Users/xxx/anaconda/bin. if (process.env['HOME']) { paths.push(path.join(process.env['HOME'], 'anaconda', 'bin')); paths.push(path.join(process.env['HOME'], 'python', 'bin')); diff --git a/src/client/interpreter/locators/services/conda.ts b/src/client/interpreter/locators/services/conda.ts index 4b2d38c63615..6eabc7151175 100644 --- a/src/client/interpreter/locators/services/conda.ts +++ b/src/client/interpreter/locators/services/conda.ts @@ -1,6 +1,7 @@ import { IS_WINDOWS } from "../../../common/utils"; -// where to find the Python binary within a conda env +// where to find the Python binary within a conda env. export const CONDA_RELATIVE_PY_PATH = IS_WINDOWS ? ['python.exe'] : ['bin', 'python']; -export const AnacondaCompanyName = 'Continuum Analytics, Inc.'; +export const AnacondaCompanyNames = ['Anaconda, Inc.', 'Continuum Analytics, Inc.']; +export const AnacondaCompanyName = 'Anaconda, Inc.'; export const AnacondaDisplayName = 'Anaconda'; diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts index 7a7c6edf6df8..fb70eda2d2c7 100644 --- a/src/client/interpreter/locators/services/condaEnvFileService.ts +++ b/src/client/interpreter/locators/services/condaEnvFileService.ts @@ -1,18 +1,11 @@ "use strict"; import * as fs from 'fs-extra'; import * as path from 'path'; +import { IS_WINDOWS } from '../../../common/configSettings'; import { IInterpreterVersionService } from '../../interpreterVersion'; import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts'; -import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH } from './conda'; +import { AnacondaDisplayName, AnacondaCompanyName, AnacondaCompanyNames, CONDA_RELATIVE_PY_PATH } from './conda'; -/** - * The command 'conda info --envs' doesn't seem to return all environments - * Environments created using the command 'conda create --p python=x.x' are not returned by the above command - * However all these environments seem to be listed in the environments.txt file (confirmed on windows and linux) - * @export - * @class CondaEnvFileProvider - * @implements {IInterpreterLocatorService} - */ export class CondaEnvFileService implements IInterpreterLocatorService { constructor(private condaEnvironmentFile: string, private versionService: IInterpreterVersionService) { @@ -28,25 +21,18 @@ export class CondaEnvFileService implements IInterpreterLocatorService { private getEnvironmentsFromFile(envFile: string) { return fs.readFile(envFile) .then(buffer => buffer.toString().split(/\r?\n/g)) - .then(lines => { - return lines.map(line => path.join(line, ...CONDA_RELATIVE_PY_PATH)); - }) - .then(interpreterPaths => { - return interpreterPaths.map(item => fs.pathExists(item).then(exists => exists ? item : '')); - }) + .then(lines => lines.map(line => line.trim())) + .then(lines => lines.map(line => path.join(line, ...CONDA_RELATIVE_PY_PATH))) + .then(interpreterPaths => interpreterPaths.map(item => fs.pathExists(item).then(exists => exists ? item : ''))) .then(promises => Promise.all(promises)) - .then(interpreterPaths => { - return interpreterPaths.filter(item => item.trim().length > 0); - }) + .then(interpreterPaths => interpreterPaths.filter(item => item.length > 0)) .then(interpreterPaths => interpreterPaths.map(item => this.getInterpreterDetails(item))) .then(promises => Promise.all(promises)); } private getInterpreterDetails(interpreter: string) { return this.versionService.getVersion(interpreter, path.basename(interpreter)) .then(version => { - // Strip company name from version - const startOfCompanyName = version.indexOf(`:: ${AnacondaCompanyName}`); - version = startOfCompanyName > 0 ? version.substring(0, startOfCompanyName).trim() : version; + version = this.stripCompanyName(version); const envName = this.getEnvironmentRootDirectory(interpreter); const info: PythonInterpreter = { displayName: `${AnacondaDisplayName} ${version} (${envName})`, @@ -57,6 +43,17 @@ export class CondaEnvFileService implements IInterpreterLocatorService { return info; }); } + private stripCompanyName(content: string) { + // Strip company name from version. + const startOfCompanyName = AnacondaCompanyNames.reduce((index, companyName) => { + if (index > 0) { + return index; + } + return content.indexOf(`:: ${AnacondaCompanyName}`); + }, -1); + + return startOfCompanyName > 0 ? content.substring(0, startOfCompanyName).trim() : content; + } private getEnvironmentRootDirectory(interpreter: string) { const envDir = interpreter.substring(0, interpreter.length - path.join(...CONDA_RELATIVE_PY_PATH).length); return path.basename(envDir); @@ -64,6 +61,6 @@ export class CondaEnvFileService implements IInterpreterLocatorService { } export function getEnvironmentsFile() { - const homeDir = process.env.HOME || process.env.HOMEPATH || process.env.USERPROFILE; + const homeDir = IS_WINDOWS ? process.env.USERPROFILE : (process.env.HOME || process.env.HOMEPATH); return homeDir ? path.join(homeDir, '.conda', 'environments.txt') : ''; } diff --git a/src/client/interpreter/locators/services/condaEnvService.ts b/src/client/interpreter/locators/services/condaEnvService.ts index da7cbb8f2677..a11cdc54f64b 100644 --- a/src/client/interpreter/locators/services/condaEnvService.ts +++ b/src/client/interpreter/locators/services/condaEnvService.ts @@ -22,7 +22,7 @@ export class CondaEnvService implements IInterpreterLocatorService { return this.getCondaFile() .then(condaFile => { return new Promise((resolve, reject) => { - // interrogate conda (if it's on the path) to find all environments + // interrogate conda (if it's on the path) to find all environments. child_process.execFile(condaFile, ['info', '--json'], (_, stdout) => { if (stdout.length === 0) { return resolve([]); @@ -33,9 +33,9 @@ export class CondaEnvService implements IInterpreterLocatorService { resolve(this.parseCondaInfo(info)); } catch (e) { // Failed because either: - // 1. conda is not installed - // 2. `conda info --json` has changed signature - // 3. output of `conda info --json` has changed in structure + // 1. conda is not installed. + // 2. `conda info --json` has changed signature. + // 3. output of `conda info --json` has changed in structure. // In all cases, we can't offer conda pythonPath suggestions. return resolve([]); } @@ -69,11 +69,11 @@ export class CondaEnvService implements IInterpreterLocatorService { } } public async parseCondaInfo(info: CondaInfo) { - // "sys.version": "3.6.1 |Anaconda 4.4.0 (64-bit)| (default, May 11 2017, 13:25:24) [MSC v.1900 64 bit (AMD64)]", + // "sys.version": "3.6.1 |Anaconda 4.4.0 (64-bit)| (default, May 11 2017, 13:25:24) [MSC v.1900 64 bit (AMD64)]". const displayName = this.getDisplayNameFromVersionInfo(info['sys.version'] || ''); // The root of the conda environment is itself a Python interpreter - // envs reported as e.g.: /Users/bob/miniconda3/envs/someEnv + // envs reported as e.g.: /Users/bob/miniconda3/envs/someEnv. const envs = Array.isArray(info.envs) ? info.envs : []; if (info.default_prefix && info.default_prefix.length > 0) { envs.push(info.default_prefix); @@ -81,7 +81,7 @@ export class CondaEnvService implements IInterpreterLocatorService { const promises = envs .map(env => { - // If it is an environment, hence suffix with env name + // If it is an environment, hence suffix with env name. const interpreterDisplayName = env === info.default_prefix ? displayName : `${displayName} (${path.basename(env)})`; const interpreter: PythonInterpreter = { path: path.join(env, ...CONDA_RELATIVE_PY_PATH), diff --git a/src/client/interpreter/locators/services/windowsRegistryService.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts index dc021c12f197..222c88e7139d 100644 --- a/src/client/interpreter/locators/services/windowsRegistryService.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -74,13 +74,13 @@ export class WindowsRegistryService implements IInterpreterLocatorService { }; return this.registry.getValue(key, hive, arch) .then(installPath => { - // Install path is mandatory + // Install path is mandatory. if (!installPath) { return Promise.resolve(null); } - // Check if 'ExecutablePath' exists - // Remember Python 2.7 doesn't have 'ExecutablePath' (there could be others) - // Treat all other values as optional + // Check if 'ExecutablePath' exists. + // Remember Python 2.7 doesn't have 'ExecutablePath' (there could be others). + // Treat all other values as optional. return Promise.all([ Promise.resolve(installPath), this.registry.getValue(key, hive, arch, 'ExecutablePath'), diff --git a/src/test/interpreters/condaEnvFileService.test.ts b/src/test/interpreters/condaEnvFileService.test.ts index bc25e0ae2b07..a75dca995190 100644 --- a/src/test/interpreters/condaEnvFileService.test.ts +++ b/src/test/interpreters/condaEnvFileService.test.ts @@ -6,7 +6,12 @@ import { initialize } from '../initialize'; import { IS_WINDOWS } from '../../client/common/utils'; import { MockInterpreterVersionProvider } from './mocks'; import { CondaEnvFileService } from '../../client/interpreter/locators/services/condaEnvFileService'; -import { AnacondaDisplayName, AnacondaCompanyName, CONDA_RELATIVE_PY_PATH, } from '../../client/interpreter/locators/services/conda'; +import { + AnacondaCompanyName, + AnacondaCompanyNames, + AnacondaDisplayName, + CONDA_RELATIVE_PY_PATH, +} from '../../client/interpreter/locators/services/conda'; const environmentsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'environments'); const environmentsFilePath = path.join(environmentsPath, 'environments.txt'); @@ -51,4 +56,20 @@ suite('Interpreters from Conda Environments Text File', () => { assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect display name'); assert.equal(interpreters[0].path, path.join(interpreterPaths[0], ...CONDA_RELATIVE_PY_PATH), 'Incorrect company display name'); }); + test('Must strip company name from version info', async () => { + const interpreterPaths = [ + path.join(environmentsPath, 'conda', 'envs', 'numpy') + ]; + await updateEnvWithInterpreters(interpreterPaths); + + AnacondaCompanyNames.forEach(async companyDisplayName => { + const displayNameProvider = new MockInterpreterVersionProvider(`Mock Version :: ${companyDisplayName}`); + const condaFileProvider = new CondaEnvFileService(environmentsFilePath, displayNameProvider); + const interpreters = await condaFileProvider.getInterpreters(); + // This is because conda environments will be under 'bin/python' however the folders path1 and path2 do not have such files + const numberOfEnvs = IS_WINDOWS ? 3 : 1; + assert.equal(interpreters.length, numberOfEnvs, 'Incorrect number of entries'); + assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version (numpy)`, 'Incorrect display name'); + }); + }); }); diff --git a/src/test/interpreters/condaEnvService.test.ts b/src/test/interpreters/condaEnvService.test.ts index adebde109f41..e363ab94dc75 100644 --- a/src/test/interpreters/condaEnvService.test.ts +++ b/src/test/interpreters/condaEnvService.test.ts @@ -4,6 +4,7 @@ import * as settings from '../../client/common/configSettings'; import { initialize, setPythonExecutable } from '../initialize'; import { IS_WINDOWS } from '../../client/common/utils'; import { CondaEnvService } from '../../client/interpreter/locators/services/condaEnvService'; +import { AnacondaCompanyName } from '../../client/interpreter/locators/services/conda'; import { MockProvider } from './mocks'; import { PythonInterpreter } from '../../client/interpreter/contracts'; @@ -44,12 +45,12 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], IS_WINDOWS ? 'python.exe' : 'bin/python'); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); assert.equal(interpreters[0].displayName, 'Anaconda 4.4.0 (64-bit) (numpy)', 'Incorrect display name for first env'); - assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); + assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); const path2 = path.join(info.envs[1], IS_WINDOWS ? 'python.exe' : 'bin/python'); assert.equal(interpreters[1].path, path2, 'Incorrect path for first env'); assert.equal(interpreters[1].displayName, 'Anaconda 4.4.0 (64-bit) (scipy)', 'Incorrect display name for first env'); - assert.equal(interpreters[1].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); + assert.equal(interpreters[1].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); }); test('Must use the default display name if sys.version is invalid', async () => { const condaProvider = new CondaEnvService(); @@ -64,7 +65,7 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], IS_WINDOWS ? 'python.exe' : 'bin/python'); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); assert.equal(interpreters[0].displayName, 'Anaconda (numpy)', 'Incorrect display name for first env'); - assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); + assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); }); test('Must use the default display name if sys.version is empty', async () => { const condaProvider = new CondaEnvService(); @@ -77,7 +78,7 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.envs[0], IS_WINDOWS ? 'python.exe' : 'bin/python'); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); assert.equal(interpreters[0].displayName, 'Anaconda (numpy)', 'Incorrect display name for first env'); - assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); + assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); }); test('Must include the default_prefix into the list of interpreters', async () => { const condaProvider = new CondaEnvService(); @@ -90,7 +91,7 @@ suite('Interpreters from Conda Environments', () => { const path1 = path.join(info.default_prefix, IS_WINDOWS ? 'python.exe' : 'bin/python'); assert.equal(interpreters[0].path, path1, 'Incorrect path for first env'); assert.equal(interpreters[0].displayName, 'Anaconda', 'Incorrect display name for first env'); - assert.equal(interpreters[0].companyDisplayName, 'Continuum Analytics, Inc.', 'Incorrect company display name for first env'); + assert.equal(interpreters[0].companyDisplayName, AnacondaCompanyName, 'Incorrect company display name for first env'); }); test('Must exclude interpreters that do not exist on disc', async () => { const condaProvider = new CondaEnvService(); From 070637f3d4da18a8c108a5f2e4d56909f5cfdb36 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 26 Sep 2017 13:54:20 -0700 Subject: [PATCH 9/9] force a new line --- src/client/interpreter/interpreterVersion.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/interpreter/interpreterVersion.ts b/src/client/interpreter/interpreterVersion.ts index 808072270bf4..de9fddc57575 100644 --- a/src/client/interpreter/interpreterVersion.ts +++ b/src/client/interpreter/interpreterVersion.ts @@ -10,3 +10,4 @@ export class InterpreterVersionService implements IInterpreterVersionService { .catch(() => defaultValue); } } +