Changes to logic for selection of best (default) version of Python interpreter

build/ci/mocha-vsts-reporter.js

@@ -15,11 +15,9 @@ function MochaVstsReporter(runner, options) {
 
   runner.on('suite', function(suite){
     if (suite.root === true){
-      console.log('Begin test run.............');
       indentLevel++;
       indenter = INDENT_BASE.repeat(indentLevel);
     } else {
-      console.log('%sStart "%s"', indenter, suite.title);
       indentLevel++;
       indenter = INDENT_BASE.repeat(indentLevel);
     }
@@ -29,9 +27,7 @@ function MochaVstsReporter(runner, options) {
     if (suite.root === true) {
       indentLevel=0;
       indenter = '';
-      console.log('.............End test run.');
     } else {
-      console.log('%sEnd "%s"', indenter, suite.title);
       indentLevel--;
       indenter = INDENT_BASE.repeat(indentLevel);
       // ##vso[task.setprogress]current operation

news/1 Enhancements/3369.md

@@ -0,0 +1 @@
+Improvements to automatic selection of the python interpreter.

src/client/common/configSettings.ts

@@ -5,10 +5,13 @@ import { EventEmitter } from 'events';
 import * as path from 'path';
 import {
     ConfigurationTarget, DiagnosticSeverity, Disposable, Uri,
-    workspace, WorkspaceConfiguration
+    WorkspaceConfiguration
 } from 'vscode';
+import { IInterpreterAutoSeletionProxyService } from '../interpreter/autoSelection/types';
 import { sendTelemetryEvent } from '../telemetry';
 import { COMPLETION_ADD_BRACKETS, FORMAT_ON_TYPE } from '../telemetry/constants';
+import { IWorkspaceService } from './application/types';
+import { WorkspaceService } from './application/workspace';
 import { isTestExecution } from './constants';
 import { IS_WINDOWS } from './platform/constants';
 import {
@@ -57,21 +60,28 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
     private disposables: Disposable[] = [];
     // tslint:disable-next-line:variable-name
     private _pythonPath = '';
+    private readonly workspace: IWorkspaceService;
 
-    constructor(workspaceFolder?: Uri) {
+    constructor(workspaceFolder: Uri | undefined, private readonly interpreterAutoSeletionService: IInterpreterAutoSeletionProxyService,
+        workspace?: IWorkspaceService) {
         super();
+        this.workspace = workspace || new WorkspaceService();
         this.workspaceRoot = workspaceFolder ? workspaceFolder : Uri.file(__dirname);
         this.initialize();
     }
     // tslint:disable-next-line:function-name
-    public static getInstance(resource?: Uri): PythonSettings {
-        const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource).uri;
+    public static getInstance(resource: Uri | undefined, interpreterAutoSeletionService: IInterpreterAutoSeletionProxyService,
+        workspace?: IWorkspaceService): PythonSettings {
+        workspace = workspace || new WorkspaceService();
+        const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri;
         const workspaceFolderKey = workspaceFolderUri ? workspaceFolderUri.fsPath : '';
 
         if (!PythonSettings.pythonSettings.has(workspaceFolderKey)) {
-            const settings = new PythonSettings(workspaceFolderUri);
+            const settings = new PythonSettings(workspaceFolderUri, interpreterAutoSeletionService, workspace);
             PythonSettings.pythonSettings.set(workspaceFolderKey, settings);
-            const config = workspace.getConfiguration('editor', resource ? resource : null);
+            // Pass null to avoid VSC from complaining about not passing in a value.
+            // tslint:disable-next-line:no-any
+            const config = workspace.getConfiguration('editor', resource ? resource : null as any);
             const formatOnType = config ? config.get('formatOnType', false) : false;
             sendTelemetryEvent(COMPLETION_ADD_BRACKETS, undefined, { enabled: settings.autoComplete ? settings.autoComplete.addBrackets : false });
             sendTelemetryEvent(FORMAT_ON_TYPE, undefined, { enabled: formatOnType });
@@ -81,7 +91,8 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
     }
 
     // tslint:disable-next-line:type-literal-delimiter
-    public static getSettingsUriAndTarget(resource?: Uri): { uri: Uri | undefined, target: ConfigurationTarget } {
+    public static getSettingsUriAndTarget(resource: Uri | undefined, workspace?: IWorkspaceService): { uri: Uri | undefined, target: ConfigurationTarget } {
+        workspace = workspace || new WorkspaceService();
         const workspaceFolder = resource ? workspace.getWorkspaceFolder(resource) : undefined;
         let workspaceFolderUri: Uri | undefined = workspaceFolder ? workspaceFolder.uri : undefined;
 
@@ -99,21 +110,26 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
             throw new Error('Dispose can only be called from unit tests');
         }
         // tslint:disable-next-line:no-void-expression
-        PythonSettings.pythonSettings.forEach(item => item.dispose());
+        PythonSettings.pythonSettings.forEach(item => item && item.dispose());
         PythonSettings.pythonSettings.clear();
     }
     public dispose() {
         // tslint:disable-next-line:no-unsafe-any
-        this.disposables.forEach(disposable => disposable.dispose());
+        this.disposables.forEach(disposable => disposable && disposable.dispose());
         this.disposables = [];
     }
     // tslint:disable-next-line:cyclomatic-complexity max-func-body-length
-    public update(pythonSettings: WorkspaceConfiguration) {
+    protected update(pythonSettings: WorkspaceConfiguration) {
         const workspaceRoot = this.workspaceRoot.fsPath;
         const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
 
         // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
         this.pythonPath = systemVariables.resolveAny(pythonSettings.get<string>('pythonPath'))!;
+        // If user has defined a custom value, use it else try to get the best interpreter ourselves.
+        if (this.pythonPath.length === 0 || this.pythonPath === 'python') {
+            const autoSelectedPythonPath = this.interpreterAutoSeletionService.getAutoSelectedInterpreter(this.workspaceRoot);
+            this.pythonPath = autoSelectedPythonPath ? autoSelectedPythonPath.path : this.pythonPath;
+        }
         this.pythonPath = getAbsolutePath(this.pythonPath, workspaceRoot);
         // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
         this.venvPath = systemVariables.resolveAny(pythonSettings.get<string>('venvPath'))!;
@@ -342,22 +358,27 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
         // Add support for specifying just the directory where the python executable will be located.
         // E.g. virtual directory name.
         try {
-            this._pythonPath = getPythonExecutable(value);
+            this._pythonPath = this.getPythonExecutable(value);
         } catch (ex) {
             this._pythonPath = value;
         }
     }
+    protected getPythonExecutable(pythonPath: string) {
+        return getPythonExecutable(pythonPath);
+    }
     protected initialize(): void {
-        this.disposables.push(workspace.onDidChangeConfiguration(() => {
-            const currentConfig = workspace.getConfiguration('python', this.workspaceRoot);
+        const onDidChange = () => {
+            const currentConfig = this.workspace.getConfiguration('python', this.workspaceRoot);
             this.update(currentConfig);
 
             // If workspace config changes, then we could have a cascading effect of on change events.
             // Let's defer the change notification.
             setTimeout(() => this.emit('change'), 1);
-        }));
+        };
+        this.disposables.push(this.interpreterAutoSeletionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this)));
+        this.disposables.push(this.workspace.onDidChangeConfiguration(onDidChange.bind(this)));
 
-        const initialConfig = workspace.getConfiguration('python', this.workspaceRoot);
+        const initialConfig = this.workspace.getConfiguration('python', this.workspaceRoot);
         if (initialConfig) {
             this.update(initialConfig);
         }

src/client/common/configuration/service.ts

@@ -1,23 +1,32 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { injectable } from 'inversify';
+import { inject, injectable } from 'inversify';
 import { ConfigurationTarget, Uri, workspace, WorkspaceConfiguration } from 'vscode';
+import { IInterpreterAutoSeletionProxyService } from '../../interpreter/autoSelection/types';
+import { IServiceContainer } from '../../ioc/types';
+import { IWorkspaceService } from '../application/types';
 import { PythonSettings } from '../configSettings';
 import { IConfigurationService, IPythonSettings } from '../types';
 
 @injectable()
 export class ConfigurationService implements IConfigurationService {
+    private readonly workspaceService: IWorkspaceService;
+    constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) {
+        this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
+    }
     public getSettings(resource?: Uri): IPythonSettings {
-        return PythonSettings.getInstance(resource);
+        const interpreterAutoSeletionService = this.serviceContainer.get<IInterpreterAutoSeletionProxyService>(IInterpreterAutoSeletionProxyService);
+        return PythonSettings.getInstance(resource, interpreterAutoSeletionService, this.workspaceService);
+        // return PythonSettings.getInstance(resource);
     }
 
     public async updateSectionSetting(section: string, setting: string, value?: {}, resource?: Uri, configTarget?: ConfigurationTarget): Promise<void> {
         const defaultSetting = {
             uri: resource,
             target: configTarget || ConfigurationTarget.WorkspaceFolder
         };
-        const settingsInfo = section === 'python' && configTarget !== ConfigurationTarget.Global ? PythonSettings.getSettingsUriAndTarget(resource) : defaultSetting;
+        const settingsInfo = section === 'python' && configTarget !== ConfigurationTarget.Global ? PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService) : defaultSetting;
 
         const configSection = workspace.getConfiguration(section, settingsInfo.uri);
         const currentValue = configSection.inspect(setting);

src/client/common/installer/productInstaller.ts

@@ -197,16 +197,16 @@ export class LinterInstaller extends BaseInstaller {
         }
         const response = await this.appShell.showErrorMessage(message, ...options);
         if (response === install) {
-            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { tool: productName as LinterId, action: 'install'});
+            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { tool: productName as LinterId, action: 'install' });
             return this.install(product, resource);
         } else if (response === disableInstallPrompt) {
             await this.setStoredResponse(disableLinterInstallPromptKey, true);
-            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { tool: productName as LinterId, action: 'disablePrompt'});
+            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { tool: productName as LinterId, action: 'disablePrompt' });
             return InstallerResponse.Ignore;
         }
 
-        if (response === selectLinter){
-            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { action: 'select'});
+        if (response === selectLinter) {
+            sendTelemetryEvent(LINTER_NOT_INSTALLED_PROMPT, undefined, { action: 'select' });
             const commandManager = this.serviceContainer.get<ICommandManager>(ICommandManager);
             await commandManager.executeCommand(Commands.Set_Linter);
         }
@@ -224,7 +224,7 @@ export class LinterInstaller extends BaseInstaller {
     protected getStoredResponse(key: string): boolean {
         const factory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
         const state = factory.createGlobalPersistentState<boolean | undefined>(key, undefined);
-        return state.value;
+        return state.value === true;
     }
 
     /**

src/client/common/persistentState.ts

@@ -7,7 +7,7 @@ import { inject, injectable, named } from 'inversify';
 import { Memento } from 'vscode';
 import { GLOBAL_MEMENTO, IMemento, IPersistentState, IPersistentStateFactory, WORKSPACE_MEMENTO } from './types';
 
-class PersistentState<T> implements IPersistentState<T>{
+export class PersistentState<T> implements IPersistentState<T>{
     constructor(private storage: Memento, private key: string, private defaultValue?: T, private expiryDurationMs?: number) { }
 
     public get value(): T {

src/client/common/platform/registry.ts

@@ -1,5 +1,5 @@
 import { injectable } from 'inversify';
-import * as Registry from 'winreg';
+import { Options } from 'winreg';
 import { Architecture } from '../utils/platform';
 import { IRegistry, RegistryHive } from './types';
 
@@ -29,7 +29,9 @@ export function getArchitectureDisplayName(arch?: Architecture) {
     }
 }
 
-async function getRegistryValue(options: Registry.Options, name: string = '') {
+async function getRegistryValue(options: Options, name: string = '') {
+    // tslint:disable-next-line:no-require-imports
+    const Registry = require('winreg') as typeof import('winreg');
     return new Promise<string | undefined | null>((resolve, reject) => {
         new Registry(options).get(name, (error, result) => {
             if (error || !result || typeof result.value !== 'string') {
@@ -39,7 +41,10 @@ async function getRegistryValue(options: Registry.Options, name: string = '') {
         });
     });
 }
-async function getRegistryKeys(options: Registry.Options): Promise<string[]> {
+
+async function getRegistryKeys(options: Options): Promise<string[]> {
+    // tslint:disable-next-line:no-require-imports
+    const Registry = require('winreg') as typeof import('winreg');
     // https://github.com/python/peps/blob/master/pep-0514.txt#L85
     return new Promise<string[]>((resolve, reject) => {
         new Registry(options).keys((error, result) => {
@@ -61,6 +66,8 @@ function translateArchitecture(arch?: Architecture): RegistryArchitectures | und
     }
 }
 function translateHive(hive: RegistryHive): string | undefined {
+    // tslint:disable-next-line:no-require-imports
+    const Registry = require('winreg') as typeof import('winreg');
     switch (hive) {
         case RegistryHive.HKCU:
             return Registry.HKCU;

src/client/common/platform/serviceRegistry.ts

@@ -11,7 +11,5 @@ import { IFileSystem, IPlatformService, IRegistry } from './types';
 export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingleton<IPlatformService>(IPlatformService, PlatformService);
     serviceManager.addSingleton<IFileSystem>(IFileSystem, FileSystem);
-    if (serviceManager.get<IPlatformService>(IPlatformService).isWindows) {
-        serviceManager.addSingleton<IRegistry>(IRegistry, RegistryImplementation);
-    }
+    serviceManager.addSingleton<IRegistry>(IRegistry, RegistryImplementation);
 }

src/client/common/process/pythonProcess.ts

@@ -10,7 +10,7 @@ import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
 import { traceError } from '../logger';
 import { IFileSystem } from '../platform/types';
 import { Architecture } from '../utils/platform';
-import { convertPythonVersionToSemver } from '../utils/version';
+import { parsePythonVersion } from '../utils/version';
 import { ExecutionResult, InterpreterInfomation, IProcessService, IPythonExecutionService, ObservableExecutionResult, PythonVersionInfo, SpawnOptions } from './types';
 
 @injectable()
@@ -42,7 +42,7 @@ export class PythonExecutionService implements IPythonExecutionService {
             return {
                 architecture: json.is64Bit ? Architecture.x64 : Architecture.x86,
                 path: this.pythonPath,
-                version: convertPythonVersionToSemver(versionValue),
+                version: parsePythonVersion(versionValue),
                 sysVersion: json.sysVersion,
                 sysPrefix: json.sysPrefix
             };

src/client/common/process/types.ts

@@ -3,9 +3,7 @@
 import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process';
 import { Observable } from 'rxjs/Observable';
 import { CancellationToken, Uri } from 'vscode';
-
-import { SemVer } from 'semver';
-import { ExecutionInfo } from '../types';
+import { ExecutionInfo, Version } from '../types';
 import { Architecture } from '../utils/platform';
 import { EnvironmentVariables } from '../variables/types';
 
@@ -64,7 +62,7 @@ export type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown';
 export type PythonVersionInfo = [number, number, number, ReleaseLevel];
 export type InterpreterInfomation = {
     path: string;
-    version?: SemVer;
+    version?: Version;
     sysVersion: string;
     architecture: Architecture;
     sysPrefix: string;

src/client/common/types.ts

@@ -15,10 +15,19 @@ export const IMemento = Symbol('IGlobalMemento');
 export const GLOBAL_MEMENTO = Symbol('IGlobalMemento');
 export const WORKSPACE_MEMENTO = Symbol('IWorkspaceMemento');
 
+export type Resource = Uri | undefined;
 export interface IPersistentState<T> {
     readonly value: T;
     updateValue(value: T): Promise<void>;
 }
+export type Version = {
+    raw: string;
+    major: number;
+    minor: number;
+    patch: number;
+    build: string[];
+    prerelease: string[];
+};
 
 export const IPersistentStateFactory = Symbol('IPersistentStateFactory');
 
@@ -365,7 +374,7 @@ export interface IEditorUtils {
 }
 
 export interface IDisposable {
-    dispose(): Promise<void> | undefined;
+    dispose(): Promise<void> | undefined | void;
 }
 
 export const IAsyncDisposableRegistry = Symbol('IAsyncDisposableRegistry');

src/client/common/utils/async.ts

@@ -39,12 +39,14 @@ class DeferredImpl<T> implements Deferred<T> {
         });
     }
     public resolve(value?: T | PromiseLike<T>) {
-        this._resolve.apply(this.scope ? this.scope : this, arguments);
+        // tslint:disable-next-line:no-any
+        this._resolve.apply(this.scope ? this.scope : this, arguments as any);
         this._resolved = true;
     }
     // tslint:disable-next-line:no-any
     public reject(reason?: any) {
-        this._reject.apply(this.scope ? this.scope : this, arguments);
+        // tslint:disable-next-line:no-any
+        this._reject.apply(this.scope ? this.scope : this, arguments as any);
         this._rejected = true;
     }
     get promise(): Promise<T> {
@@ -67,9 +69,18 @@ export function createDeferred<T>(scope: any = null): Deferred<T> {
 
 export function createDeferredFrom<T>(...promises: Promise<T>[]): Deferred<T> {
     const deferred = createDeferred<T>();
-    Promise.all(promises)
+    Promise.all<T>(promises)
+        // tslint:disable-next-line:no-any
+        .then(deferred.resolve.bind(deferred) as any)
+        // tslint:disable-next-line:no-any
+        .catch(deferred.reject.bind(deferred) as any);
+
+    return deferred;
+}
+export function createDeferredFromPromise<T>(promise: Promise<T>): Deferred<T> {
+    const deferred = createDeferred<T>();
+    promise
         .then(deferred.resolve.bind(deferred))
         .catch(deferred.reject.bind(deferred));
-
     return deferred;
 }