From 14396372e7bc593b1094f4258cc43086bcdaf16f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 14 Jun 2024 16:46:15 +1000 Subject: [PATCH] Adopt new Python locator (#23617) --- src/client/common/utils/iterable.ts | 11 ++ src/client/common/utils/resourceLifecycle.ts | 153 ++++++++++++++++++ src/client/pythonEnvironments/base/locator.ts | 5 - .../locators/common/nativePythonFinder.ts | 127 ++++++++------- .../base/locators/composite/envsResolver.ts | 13 +- .../base/locators/composite/resolverUtils.ts | 6 - .../base/locators/lowLevel/nativeLocator.ts | 144 +++++------------ 7 files changed, 285 insertions(+), 174 deletions(-) create mode 100644 src/client/common/utils/iterable.ts diff --git a/src/client/common/utils/iterable.ts b/src/client/common/utils/iterable.ts new file mode 100644 index 000000000000..5e04aaa430ea --- /dev/null +++ b/src/client/common/utils/iterable.ts @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/* eslint-disable @typescript-eslint/no-explicit-any */ +// eslint-disable-next-line @typescript-eslint/no-namespace +export namespace Iterable { + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + export function is(thing: any): thing is Iterable { + return thing && typeof thing === 'object' && typeof thing[Symbol.iterator] === 'function'; + } +} diff --git a/src/client/common/utils/resourceLifecycle.ts b/src/client/common/utils/resourceLifecycle.ts index 485294392ea1..f41efebc12cb 100644 --- a/src/client/common/utils/resourceLifecycle.ts +++ b/src/client/common/utils/resourceLifecycle.ts @@ -1,12 +1,50 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// eslint-disable-next-line max-classes-per-file import { IDisposable } from '../types'; +import { Iterable } from './iterable'; interface IDisposables extends IDisposable { push(...disposable: IDisposable[]): void; } +export const EmptyDisposable = { + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + dispose: () => { + /** */ + }, +}; + +/** + * Disposes of the value(s) passed in. + */ +export function dispose(disposable: T): T; +export function dispose(disposable: T | undefined): T | undefined; +export function dispose = Iterable>(disposables: A): A; +export function dispose(disposables: Array): Array; +export function dispose(disposables: ReadonlyArray): ReadonlyArray; +// eslint-disable-next-line @typescript-eslint/no-explicit-any, consistent-return +export function dispose(arg: T | Iterable | undefined): any { + if (Iterable.is(arg)) { + for (const d of arg) { + if (d) { + try { + d.dispose(); + } catch (e) { + console.warn(`dispose() failed for ${d}`, e); + } + } + } + + return Array.isArray(arg) ? [] : arg; + } + if (arg) { + arg.dispose(); + return arg; + } +} + /** * Safely dispose each of the disposables. */ @@ -43,3 +81,118 @@ export class Disposables implements IDisposables { await disposeAll(disposables); } } + +/** + * Manages a collection of disposable values. + * + * This is the preferred way to manage multiple disposables. A `DisposableStore` is safer to work with than an + * `IDisposable[]` as it considers edge cases, such as registering the same value multiple times or adding an item to a + * store that has already been disposed of. + */ +export class DisposableStore implements IDisposable { + static DISABLE_DISPOSED_WARNING = false; + + private readonly _toDispose = new Set(); + + private _isDisposed = false; + + constructor(...disposables: IDisposable[]) { + disposables.forEach((disposable) => this.add(disposable)); + } + + /** + * Dispose of all registered disposables and mark this object as disposed. + * + * Any future disposables added to this object will be disposed of on `add`. + */ + public dispose(): void { + if (this._isDisposed) { + return; + } + + this._isDisposed = true; + this.clear(); + } + + /** + * @return `true` if this object has been disposed of. + */ + public get isDisposed(): boolean { + return this._isDisposed; + } + + /** + * Dispose of all registered disposables but do not mark this object as disposed. + */ + public clear(): void { + if (this._toDispose.size === 0) { + return; + } + + try { + dispose(this._toDispose); + } finally { + this._toDispose.clear(); + } + } + + /** + * Add a new {@link IDisposable disposable} to the collection. + */ + public add(o: T): T { + if (!o) { + return o; + } + if (((o as unknown) as DisposableStore) === this) { + throw new Error('Cannot register a disposable on itself!'); + } + + if (this._isDisposed) { + if (!DisposableStore.DISABLE_DISPOSED_WARNING) { + console.warn( + new Error( + 'Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!', + ).stack, + ); + } + } else { + this._toDispose.add(o); + } + + return o; + } +} + +/** + * Abstract class for a {@link IDisposable disposable} object. + * + * Subclasses can {@linkcode _register} disposables that will be automatically cleaned up when this object is disposed of. + */ +export abstract class DisposableBase implements IDisposable { + protected readonly _store = new DisposableStore(); + + private _isDisposed = false; + + public get isDisposed(): boolean { + return this._isDisposed; + } + + constructor(...disposables: IDisposable[]) { + disposables.forEach((disposable) => this._store.add(disposable)); + } + + public dispose(): void { + this._store.dispose(); + this._isDisposed = true; + } + + /** + * Adds `o` to the collection of disposables managed by this object. + */ + public _register(o: T): T { + if (((o as unknown) as DisposableBase) === this) { + throw new Error('Cannot register a disposable on itself!'); + } + return this._store.add(o); + } +} diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index d61f530f46ab..da73735cb323 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -157,11 +157,6 @@ export type BasicEnvInfo = { * E.g. display name as provided by Windows Registry or Windows Store, etc */ displayName?: string; - /** - * Command used to run Python in this environment. - * E.g. `conda run -n envName python` or `python.exe` - */ - pythonRunCommand?: string[]; identifiedUsingNativeLocator?: boolean; arch?: Architecture; ctime?: number; diff --git a/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts b/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts index ac89d9e3aaf8..e8ed33802a11 100644 --- a/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts +++ b/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { CancellationToken, Disposable, Event, EventEmitter } from 'vscode'; +import { Disposable, EventEmitter, Event, Uri } from 'vscode'; import * as ch from 'child_process'; import * as path from 'path'; import * as rpc from 'vscode-jsonrpc/node'; @@ -10,40 +10,36 @@ import { isWindows } from '../../../../common/platform/platformService'; import { EXTENSION_ROOT_DIR } from '../../../../constants'; import { traceError, traceInfo, traceLog, traceVerbose, traceWarn } from '../../../../logging'; import { createDeferred } from '../../../../common/utils/async'; +import { DisposableBase } from '../../../../common/utils/resourceLifecycle'; const NATIVE_LOCATOR = isWindows() - ? path.join(EXTENSION_ROOT_DIR, 'native_locator', 'bin', 'python-finder.exe') - : path.join(EXTENSION_ROOT_DIR, 'native_locator', 'bin', 'python-finder'); + ? path.join(EXTENSION_ROOT_DIR, 'native_locator', 'bin', 'pet.exe') + : path.join(EXTENSION_ROOT_DIR, 'native_locator', 'bin', 'pet'); export interface NativeEnvInfo { displayName?: string; - name: string; - pythonExecutablePath?: string; + name?: string; + executable?: string; category: string; version?: string; - pythonRunCommand?: string[]; - envPath?: string; - envManager?: NativeEnvManagerInfo; + prefix?: string; + manager?: NativeEnvManagerInfo; /** * Path to the project directory when dealing with pipenv virtual environments. */ - projectPath?: string; - arch?: 'X64' | 'X86'; + project?: string; + arch?: 'x64' | 'x86'; symlinks?: string[]; - creationTime?: number; - modifiedTime?: number; } export interface NativeEnvManagerInfo { tool: string; - executablePath: string; + executable: string; version?: string; } export interface NativeGlobalPythonFinder extends Disposable { - startSearch(token?: CancellationToken): Promise; - onDidFindPythonEnvironment: Event; - onDidFindEnvironmentManager: Event; + refresh(paths: Uri[]): AsyncIterable; } interface NativeLog { @@ -51,24 +47,51 @@ interface NativeLog { message: string; } -class NativeGlobalPythonFinderImpl implements NativeGlobalPythonFinder { - private readonly _onDidFindPythonEnvironment = new EventEmitter(); - - private readonly _onDidFindEnvironmentManager = new EventEmitter(); - - public readonly onDidFindPythonEnvironment = this._onDidFindPythonEnvironment.event; - - public readonly onDidFindEnvironmentManager = this._onDidFindEnvironmentManager.event; +class NativeGlobalPythonFinderImpl extends DisposableBase implements NativeGlobalPythonFinder { + async *refresh(_paths: Uri[]): AsyncIterable { + const result = this.start(); + let completed = false; + void result.completed.finally(() => { + completed = true; + }); + const envs: NativeEnvInfo[] = []; + let discovered = createDeferred(); + const disposable = result.discovered((data) => envs.push(data)); + + do { + await Promise.race([result.completed, discovered.promise]); + if (envs.length) { + const dataToSend = [...envs]; + envs.length = 0; + for (const data of dataToSend) { + yield data; + } + } + if (!completed) { + discovered = createDeferred(); + envs.length = 0; + } + } while (!completed); + + disposable.dispose(); + } - public startSearch(token?: CancellationToken): Promise { - const deferred = createDeferred(); - const proc = ch.spawn(NATIVE_LOCATOR, [], { env: process.env }); + // eslint-disable-next-line class-methods-use-this + private start(): { completed: Promise; discovered: Event } { + const discovered = new EventEmitter(); + const completed = createDeferred(); + const proc = ch.spawn(NATIVE_LOCATOR, ['server'], { env: process.env }); const disposables: Disposable[] = []; // jsonrpc package cannot handle messages coming through too quicly. // Lets handle the messages and close the stream only when // we have got the exit event. const readable = new PassThrough(); proc.stdout.pipe(readable, { end: false }); + let err = ''; + proc.stderr.on('data', (data) => { + err += data.toString(); + traceError('Native Python Finder', err); + }); const writable = new PassThrough(); writable.pipe(proc.stdin, { end: false }); const disposeStreams = new Disposable(() => { @@ -79,23 +102,30 @@ class NativeGlobalPythonFinderImpl implements NativeGlobalPythonFinder { new rpc.StreamMessageReader(readable), new rpc.StreamMessageWriter(writable), ); - disposables.push( connection, disposeStreams, + discovered, connection.onError((ex) => { disposeStreams.dispose(); traceError('Error in Native Python Finder', ex); }), - connection.onNotification('pythonEnvironment', (data: NativeEnvInfo) => { - this._onDidFindPythonEnvironment.fire(data); + connection.onNotification('environment', (data: NativeEnvInfo) => { + discovered.fire(data); }), - connection.onNotification('envManager', (data: NativeEnvManagerInfo) => { - this._onDidFindEnvironmentManager.fire(data); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + connection.onNotification((method: string, data: any) => { + console.log(method, data); }), - connection.onNotification('exit', () => { - traceInfo('Native Python Finder exited'); + connection.onNotification('exit', (time: number) => { + traceInfo(`Native Python Finder completed after ${time}ms`); disposeStreams.dispose(); + completed.resolve(); + }), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + connection.onRequest((method: string, args: any) => { + console.error(method, args); + return 'HELLO THERE'; }), connection.onNotification('log', (data: NativeLog) => { switch (data.level) { @@ -116,7 +146,7 @@ class NativeGlobalPythonFinderImpl implements NativeGlobalPythonFinder { } }), connection.onClose(() => { - deferred.resolve(); + completed.resolve(); disposables.forEach((d) => d.dispose()); }), { @@ -125,33 +155,20 @@ class NativeGlobalPythonFinderImpl implements NativeGlobalPythonFinder { if (proc.exitCode === null) { proc.kill(); } - } catch (err) { - traceVerbose('Error while disposing Native Python Finder', err); + } catch (ex) { + traceVerbose('Error while disposing Native Python Finder', ex); } }, }, ); - if (token) { - disposables.push( - token.onCancellationRequested(() => { - deferred.resolve(); - try { - proc.kill(); - } catch (err) { - traceVerbose('Error while handling cancellation request for Native Python Finder', err); - } - }), - ); - } - connection.listen(); - return deferred.promise; - } + connection.sendRequest('initialize', { body: ['This is id', 'Another'], supported: true }).then((r) => { + console.error(r); + void connection.sendNotification('initialized'); + }); - public dispose() { - this._onDidFindPythonEnvironment.dispose(); - this._onDidFindEnvironmentManager.dispose(); + return { completed: completed.promise, discovered: discovered.event }; } } diff --git a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts index 8c02eab33359..a823e6f77ec5 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts @@ -140,7 +140,7 @@ export class PythonEnvsResolver implements IResolvingLocator { const info = await this.environmentInfoService.getMandatoryEnvironmentInfo(seen[envIndex]); const old = seen[envIndex]; if (info) { - const resolvedEnv = getResolvedEnv(info, seen[envIndex]); + const resolvedEnv = getResolvedEnv(info, seen[envIndex], old.identifiedUsingNativeLocator); seen[envIndex] = resolvedEnv; didUpdate.fire({ old, index: envIndex, update: resolvedEnv }); } else { @@ -188,7 +188,11 @@ function checkIfFinishedAndNotify( } } -function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: PythonEnvInfo) { +function getResolvedEnv( + interpreterInfo: InterpreterInformation, + environment: PythonEnvInfo, + identifiedUsingNativeLocator = false, +) { // Deep copy into a new object const resolvedEnv = cloneDeep(environment); resolvedEnv.executable.sysPrefix = interpreterInfo.executable.sysPrefix; @@ -196,7 +200,10 @@ function getResolvedEnv(interpreterInfo: InterpreterInformation, environment: Py getEnvPath(resolvedEnv.executable.filename, resolvedEnv.location).pathType === 'envFolderPath'; // TODO: Shouldn't this only apply to conda, how else can we have an environment and not have Python in it? // If thats the case, then this should be gated on environment.kind === PythonEnvKind.Conda - if (isEnvLackingPython && environment.kind !== PythonEnvKind.MicrosoftStore) { + // For non-native do not blow away the versions returned by native locator. + // Windows Store and Home brew have exe and sysprefix in different locations, + // Thus above check is not valid for these envs. + if (isEnvLackingPython && environment.kind !== PythonEnvKind.MicrosoftStore && !identifiedUsingNativeLocator) { // Install python later into these envs might change the version, which can be confusing for users. // So avoid displaying any version until it is installed. resolvedEnv.version = getEmptyVersion(); diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index 079ed108ee54..1b7f23d5651a 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -160,7 +160,6 @@ async function resolveGloballyInstalledEnv(env: BasicEnvInfo): Promise { location: env.envPath, display: env.displayName, searchLocation: env.searchLocation, - pythonRunCommand: env.pythonRunCommand, identifiedUsingNativeLocator: env.identifiedUsingNativeLocator, name: env.name, type: PythonEnvType.Virtual, @@ -202,7 +200,6 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise { location: envPath, sysPrefix: envPath, display: env.displayName, - pythonRunCommand: env.pythonRunCommand, identifiedUsingNativeLocator: env.identifiedUsingNativeLocator, searchLocation: env.searchLocation, source: [], @@ -285,7 +282,6 @@ async function resolvePyenvEnv(env: BasicEnvInfo): Promise { sysPrefix: env.envPath, display: env.displayName, name: env.name, - pythonRunCommand: env.pythonRunCommand, identifiedUsingNativeLocator: env.identifiedUsingNativeLocator, // Pyenv environments can fall in to these three categories: // 1. Global Installs : These are environments that are created when you install @@ -329,7 +325,6 @@ async function resolveActiveStateEnv(env: BasicEnvInfo): Promise identifiedUsingNativeLocator: env.identifiedUsingNativeLocator, location: env.envPath, name: env.name, - pythonRunCommand: env.pythonRunCommand, searchLocation: env.searchLocation, sysPrefix: env.envPath, }); @@ -368,7 +363,6 @@ async function resolveMicrosoftStoreEnv(env: BasicEnvInfo): Promise, IDisposable { return Promise.resolve(); } - public iterEnvs(): IPythonEnvsIterator { - const stopWatch = new StopWatch(); - traceInfo('Searching for Python environments using Native Locator'); - const promise = this.finder.startSearch(); - const envs: BasicEnvInfo[] = []; + public async *iterEnvs(): IPythonEnvsIterator { const disposables: IDisposable[] = []; const disposable = new Disposable(() => disposeAll(disposables)); this.disposables.push(disposable); - promise.finally(() => disposable.dispose()); - let environmentsWithoutPython = 0; - disposables.push( - this.finder.onDidFindPythonEnvironment((data: NativeEnvInfo) => { - // TODO: What if executable is undefined? - if (data.pythonExecutablePath) { - const arch = (data.arch || '').toLowerCase(); - envs.push({ - kind: categoryToKind(data.category), - executablePath: data.pythonExecutablePath, - envPath: data.envPath, - version: parseVersion(data.version), - name: data.name === '' ? undefined : data.name, - displayName: data.displayName, - pythonRunCommand: data.pythonRunCommand, - searchLocation: data.projectPath ? Uri.file(data.projectPath) : undefined, - identifiedUsingNativeLocator: true, - arch: - // eslint-disable-next-line no-nested-ternary - arch === 'x64' ? Architecture.x64 : arch === 'x86' ? Architecture.x86 : undefined, - ctime: data.creationTime, - mtime: data.modifiedTime, - }); - } else { - environmentsWithoutPython += 1; - } - }), - this.finder.onDidFindEnvironmentManager((data: NativeEnvManagerInfo) => { - switch (toolToKnownEnvironmentTool(data.tool)) { + for await (const data of this.finder.refresh([])) { + if (data.manager) { + switch (toolToKnownEnvironmentTool(data.manager.tool)) { case 'Conda': { - Conda.setConda(data.executablePath); + Conda.setConda(data.manager.executable); break; } case 'Pyenv': { - setPyEnvBinary(data.executablePath); + setPyEnvBinary(data.manager.executable); break; } default: { break; } } - }), - ); - - const iterator = async function* (): IPythonEnvsIterator { - // When this promise is complete, we know that the search is complete. - await promise; - traceInfo( - `Finished searching for Python environments using Native Locator: ${stopWatch.elapsedTime} milliseconds`, - ); - yield* envs; - sendTelemetry(envs, environmentsWithoutPython, stopWatch); - traceInfo( - `Finished yielding Python environments using Native Locator: ${stopWatch.elapsedTime} milliseconds`, - ); - }; - - return iterator(); + } + if (data.executable) { + const arch = (data.arch || '').toLowerCase(); + const env: BasicEnvInfo = { + kind: categoryToKind(data.category), + executablePath: data.executable, + envPath: data.prefix, + version: parseVersion(data.version), + name: data.name === '' ? undefined : data.name, + displayName: data.displayName, + searchLocation: data.project ? Uri.file(data.project) : undefined, + identifiedUsingNativeLocator: true, + arch: + // eslint-disable-next-line no-nested-ternary + arch === 'x64' ? Architecture.x64 : arch === 'x86' ? Architecture.x86 : undefined, + }; + yield env; + } + } } } - -function sendTelemetry(envs: BasicEnvInfo[], environmentsWithoutPython: number, stopWatch: StopWatch) { - const activeStateEnvs = envs.filter((e) => e.kind === PythonEnvKind.ActiveState).length; - const condaEnvs = envs.filter((e) => e.kind === PythonEnvKind.Conda).length; - const customEnvs = envs.filter((e) => e.kind === PythonEnvKind.Custom).length; - const hatchEnvs = envs.filter((e) => e.kind === PythonEnvKind.Hatch).length; - const microsoftStoreEnvs = envs.filter((e) => e.kind === PythonEnvKind.MicrosoftStore).length; - const otherGlobalEnvs = envs.filter((e) => e.kind === PythonEnvKind.OtherGlobal).length; - const otherVirtualEnvs = envs.filter((e) => e.kind === PythonEnvKind.OtherVirtual).length; - const pipEnvEnvs = envs.filter((e) => e.kind === PythonEnvKind.Pipenv).length; - const poetryEnvs = envs.filter((e) => e.kind === PythonEnvKind.Poetry).length; - const pyenvEnvs = envs.filter((e) => e.kind === PythonEnvKind.Pyenv).length; - const systemEnvs = envs.filter((e) => e.kind === PythonEnvKind.System).length; - const unknownEnvs = envs.filter((e) => e.kind === PythonEnvKind.Unknown).length; - const venvEnvs = envs.filter((e) => e.kind === PythonEnvKind.Venv).length; - const virtualEnvEnvs = envs.filter((e) => e.kind === PythonEnvKind.VirtualEnv).length; - const virtualEnvWrapperEnvs = envs.filter((e) => e.kind === PythonEnvKind.VirtualEnvWrapper).length; - - // Intent is to capture time taken for discovery of all envs to complete the first time. - sendTelemetryEvent(EventName.PYTHON_INTERPRETER_DISCOVERY, stopWatch.elapsedTime, { - interpreters: envs.length, - environmentsWithoutPython, - activeStateEnvs, - condaEnvs, - customEnvs, - hatchEnvs, - microsoftStoreEnvs, - otherGlobalEnvs, - otherVirtualEnvs, - pipEnvEnvs, - poetryEnvs, - pyenvEnvs, - systemEnvs, - unknownEnvs, - venvEnvs, - virtualEnvEnvs, - virtualEnvWrapperEnvs, - }); -}