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

* Refactor version in interpreter information * News entry * Add telemetry * Changed version to a simple struct * Tests for config settings * Fixed linter command tests * Create linter manage unit tests

Author: DonJayamanne

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

@@ -3,12 +3,13 @@
 import * as child_process from 'child_process';
 import { EventEmitter } from 'events';
 import * as path from 'path';
-import {
- ConfigurationChangeEvent, ConfigurationTarget, DiagnosticSeverity, Disposable, Uri,
- workspace, WorkspaceConfiguration
-} from 'vscode';
+import { ConfigurationChangeEvent, ConfigurationTarget, DiagnosticSeverity, Disposable, Uri, WorkspaceConfiguration } from 'vscode';
+import '../common/extensions';
+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 +58,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 InterpreterAutoSelectionService: 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, interpreterAutoSelectionService: 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, interpreterAutoSelectionService, 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 +89,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 +108,29 @@ 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 autoSelectedPythonInterpreter = this.InterpreterAutoSelectionService.getAutoSelectedInterpreter(this.workspaceRoot);
+ if (autoSelectedPythonInterpreter) {
+ this.InterpreterAutoSelectionService.setWorkspaceInterpreter(this.workspaceRoot, autoSelectedPythonInterpreter).ignoreErrors();
+ }
+ this.pythonPath = autoSelectedPythonInterpreter ? autoSelectedPythonInterpreter.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,25 +359,31 @@ 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((event: ConfigurationChangeEvent) => {
- if (event.affectsConfiguration('python'))
- {
- const currentConfig = 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);
+ 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.InterpreterAutoSelectionService.onDidChangeAutoSelectedInterpreter(onDidChange.bind(this)));
+ this.disposables.push(this.workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
+ if (event.affectsConfiguration('python')) {
+ onDidChange();
  }
  }));
 
- 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,34 @@
 // 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 InterpreterAutoSelectionService = this.serviceContainer.get<IInterpreterAutoSeletionProxyService>(IInterpreterAutoSeletionProxyService);
+ return PythonSettings.getInstance(resource, InterpreterAutoSelectionService, this.workspaceService);
  }
 
  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;
+ let settingsInfo = defaultSetting;
+ if (section === 'python' && configTarget !== ConfigurationTarget.Global) {
+ settingsInfo = PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService);
+ }
 
  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;