Do not resolve inside iterEnvs in low-level locators (#16642)

* Log more details

* Fix tests

* Pluck resolving out of locators

* Allow ILocator to accept additional return types for iterEnvs

* Modify iterEnvs of Windows store locator to return only basic envs

* Rename executable to executablePath

* Factor out resolver for kind

* Only do basic env collision resolving in reducer

* Fix tests for resolver

* Fix tests with reducer

* Pluck out resolving in windows registry locator

* Pluck out resolving in workspace locator

* Pluck out resolving in global locator

* Pluck out resolving in poetry locator

* Pluck out resolving in custom virtualenv locator

* Pluck out resolving in pyenv locator

* Pluck out resolving in conda locator

* Pluck out resolving in posix known paths locator

* Pluck out resolving in windows path locator

* Fix watcher tests

* Change pythonEnvs index module to accept BasicEnvInfo

* Remove duplicate assignment

* Oops

* Remove unnecessary API

* Fix bug

* Added resolver to resolve globally installed envs

* Rename

* Fix tests

src/client/common/utils/async.ts

@@ -99,7 +99,7 @@ export function createDeferredFromPromise<T>(promise: Promise<T>): Deferred<T> {
 
 // iterators
 
-interface IAsyncIterator<T> extends AsyncIterator<T, void>, Partial<AsyncIterable<T>> {}
+interface IAsyncIterator<T> extends AsyncIterator<T, void> {}
 
 export interface IAsyncIterableIterator<T> extends IAsyncIterator<T>, AsyncIterable<T> {}
 

src/client/pythonEnvironments/base/info/index.ts

@@ -28,6 +28,14 @@ export enum PythonEnvKind {
     OtherVirtual = 'virt-other',
 }
 
+export const virtualEnvKinds = [
+    PythonEnvKind.Poetry,
+    PythonEnvKind.Pipenv,
+    PythonEnvKind.Venv,
+    PythonEnvKind.VirtualEnvWrapper,
+    PythonEnvKind.Conda,
+    PythonEnvKind.VirtualEnv,
+];
 /**
  * Information about a file.
  */

src/client/pythonEnvironments/base/locator.ts

@@ -11,20 +11,20 @@ import { BasicPythonEnvsChangedEvent, IPythonEnvsWatcher, PythonEnvsChangedEvent
 /**
  * A single update to a previously provided Python env object.
  */
-export type PythonEnvUpdatedEvent = {
+export type PythonEnvUpdatedEvent<I = PythonEnvInfo> = {
     /**
      * The iteration index of The env info that was previously provided.
      */
     index: number;
     /**
      * The env info that was previously provided.
      */
-    old?: PythonEnvInfo;
+    old?: I;
     /**
      * The env info that replaces the old info.
      * Update is sent as `undefined` if we find out that the environment is no longer valid.
      */
-    update: PythonEnvInfo | undefined;
+    update: I | undefined;
 };
 
 /**
@@ -49,7 +49,7 @@ export type PythonEnvUpdatedEvent = {
  * Callers can usually ignore the update event entirely and rely on
  * the locator to provide sufficiently complete information.
  */
-export interface IPythonEnvsIterator extends IAsyncIterableIterator<PythonEnvInfo> {
+export interface IPythonEnvsIterator<I = PythonEnvInfo> extends IAsyncIterableIterator<I> {
     /**
      * Provides possible updates for already-iterated envs.
      *
@@ -58,7 +58,7 @@ export interface IPythonEnvsIterator extends IAsyncIterableIterator<PythonEnvInf
      * If this property is not provided then it means the iterator does
      * not support updates.
      */
-    onUpdated?: Event<PythonEnvUpdatedEvent | null>;
+    onUpdated?: Event<PythonEnvUpdatedEvent<I> | null>;
 }
 
 /**
@@ -114,6 +114,8 @@ export type PythonLocatorQuery = BasicPythonLocatorQuery & {
 
 type QueryForEvent<E> = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery;
 
+export type BasicEnvInfo = { kind: PythonEnvKind; executablePath: string };
+
 /**
  * A single Python environment locator.
  *
@@ -128,7 +130,7 @@ type QueryForEvent<E> = E extends PythonEnvsChangedEvent ? PythonLocatorQuery :
  * events emitted via `onChanged` do not need to provide information
  * for the specific environments that changed.
  */
-export interface ILocator<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+export interface ILocator<I = PythonEnvInfo, E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
     extends IPythonEnvsWatcher<E> {
     /**
      * Iterate over the enviroments known tos this locator.
@@ -146,7 +148,7 @@ export interface ILocator<E extends BasicPythonEnvsChangedEvent = PythonEnvsChan
      * @param query - if provided, the locator will limit results to match
      * @returns - the fast async iterator of Python envs, which may have incomplete info
      */
-    iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator;
+    iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator<I>;
 }
 
 interface IResolver {
@@ -159,7 +161,7 @@ interface IResolver {
     resolveEnv(env: string): Promise<PythonEnvInfo | undefined>;
 }
 
-export interface IResolvingLocator extends IResolver, ILocator {}
+export interface IResolvingLocator<I = PythonEnvInfo> extends IResolver, ILocator<I> {}
 
 interface IEmitter<E extends PythonEnvsChangedEvent> {
     fire(e: E): void;
@@ -177,7 +179,8 @@ interface IEmitter<E extends PythonEnvsChangedEvent> {
  * should be used.  Only in low-level cases should you consider using
  * `BasicPythonEnvsChangedEvent`.
  */
-abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent> implements ILocator<E> {
+abstract class LocatorBase<I = PythonEnvInfo, E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+    implements ILocator<I, E> {
     public readonly onChanged: Event<E>;
 
     protected readonly emitter: IEmitter<E>;
@@ -188,7 +191,7 @@ abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsCha
     }
 
     // eslint-disable-next-line class-methods-use-this
-    public abstract iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator;
+    public abstract iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator<I>;
 }
 
 /**
@@ -203,7 +206,7 @@ abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsCha
  * Only in low-level cases should you consider subclassing `LocatorBase`
  * using `BasicPythonEnvsChangedEvent.
  */
-export abstract class Locator extends LocatorBase {
+export abstract class Locator<I = PythonEnvInfo> extends LocatorBase<I> {
     constructor() {
         super(new PythonEnvsWatcher());
     }

src/client/pythonEnvironments/base/locatorUtils.ts

@@ -74,14 +74,14 @@ function getSearchLocationFilters(query: PythonLocatorQuery): ((u: Uri) => boole
  *
  * This includes applying any received updates.
  */
-export async function getEnvs(iterator: IPythonEnvsIterator): Promise<PythonEnvInfo[]> {
-    const envs: (PythonEnvInfo | undefined)[] = [];
+export async function getEnvs<I = PythonEnvInfo>(iterator: IPythonEnvsIterator<I>): Promise<I[]> {
+    const envs: (I | undefined)[] = [];
 
     const updatesDone = createDeferred<void>();
     if (iterator.onUpdated === undefined) {
         updatesDone.resolve();
     } else {
-        const listener = iterator.onUpdated((event: PythonEnvUpdatedEvent | null) => {
+        const listener = iterator.onUpdated((event: PythonEnvUpdatedEvent<I> | null) => {
             if (event === null) {
                 updatesDone.resolve();
                 listener.dispose();

src/client/pythonEnvironments/base/locators.ts

@@ -3,26 +3,27 @@
 
 import { chain } from '../../common/utils/async';
 import { Disposables } from '../../common/utils/resourceLifecycle';
+import { PythonEnvInfo } from './info';
 import { ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery } from './locator';
 import { PythonEnvsWatchers } from './watchers';
 
 /**
  * Combine the `onUpdated` event of the given iterators into a single event.
  */
-export function combineIterators(iterators: IPythonEnvsIterator[]): IPythonEnvsIterator {
-    const result: IPythonEnvsIterator = chain(iterators);
+export function combineIterators<I>(iterators: IPythonEnvsIterator<I>[]): IPythonEnvsIterator<I> {
+    const result: IPythonEnvsIterator<I> = chain(iterators);
     const events = iterators.map((it) => it.onUpdated).filter((v) => v);
     if (!events || events.length === 0) {
         // There are no sub-events, so we leave `onUpdated` undefined.
         return result;
     }
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    result.onUpdated = (handleEvent: (e: PythonEnvUpdatedEvent | null) => any) => {
+    result.onUpdated = (handleEvent: (e: PythonEnvUpdatedEvent<I> | null) => any) => {
         const disposables = new Disposables();
         let numActive = events.length;
         events.forEach((event) => {
-            const disposable = event!((e: PythonEnvUpdatedEvent | null) => {
+            const disposable = event!((e: PythonEnvUpdatedEvent<I> | null) => {
                 // NOSONAR
                 if (e === null) {
                     numActive -= 1;
@@ -46,15 +47,15 @@ export function combineIterators(iterators: IPythonEnvsIterator[]): IPythonEnvsI
  *
  * Events and iterator results are combined.
  */
-export class Locators extends PythonEnvsWatchers implements ILocator {
+export class Locators<I = PythonEnvInfo> extends PythonEnvsWatchers implements ILocator<I> {
     constructor(
         // The locators will be watched as well as iterated.
-        private readonly locators: ReadonlyArray<ILocator>,
+        private readonly locators: ReadonlyArray<ILocator<I>>,
     ) {
         super(locators);
     }
 
-    public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+    public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<I> {
         const iterators = this.locators.map((loc) => loc.iterEnvs(query));
         return combineIterators(iterators);
     }

src/client/pythonEnvironments/base/locators/common/resourceBasedLocator.ts

@@ -3,6 +3,7 @@
 
 import { createDeferred, Deferred } from '../../../../common/utils/async';
 import { Disposables, IDisposable } from '../../../../common/utils/resourceLifecycle';
+import { PythonEnvInfo } from '../../info';
 import { IPythonEnvsIterator, Locator, PythonLocatorQuery } from '../../locator';
 
 /**
@@ -17,7 +18,7 @@ import { IPythonEnvsIterator, Locator, PythonLocatorQuery } from '../../locator'
  *
  * Otherwise it will leak (and we have no leak detection).
  */
-export abstract class LazyResourceBasedLocator extends Locator implements IDisposable {
+export abstract class LazyResourceBasedLocator<I = PythonEnvInfo> extends Locator<I> implements IDisposable {
     protected readonly disposables = new Disposables();
 
     // This will be set only once we have to create necessary resources
@@ -30,7 +31,7 @@ export abstract class LazyResourceBasedLocator extends Locator implements IDispo
         await this.disposables.dispose();
     }
 
-    public async *iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+    public async *iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<I> {
         await this.ensureResourcesReady();
         yield* this.doIterEnvs(query);
         // There is not need to wait for the watchers to get started.
@@ -40,7 +41,7 @@ export abstract class LazyResourceBasedLocator extends Locator implements IDispo
     /**
      * The subclass implementation of iterEnvs().
      */
-    protected abstract doIterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator;
+    protected abstract doIterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<I>;
 
     /**
      * This is where subclasses get their resources ready.

src/client/pythonEnvironments/base/locators/common/wrappingLocator.ts

@@ -8,30 +8,31 @@ import { IPythonEnvsIterator, IResolvingLocator, PythonLocatorQuery } from '../.
 import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
 import { LazyResourceBasedLocator } from './resourceBasedLocator';
 
-export type GetLocatorFunc = () => Promise<IResolvingLocator & Partial<IDisposable>>;
+export type GetLocatorFunc<I = PythonEnvInfo> = () => Promise<IResolvingLocator<I> & Partial<IDisposable>>;
 
 /**
  * A locator that wraps another.
  *
  * This facilitates isolating the wrapped locator.
  */
-export class LazyWrappingLocator extends LazyResourceBasedLocator {
+export class LazyWrappingLocator<I = PythonEnvInfo> extends LazyResourceBasedLocator<I> {
     public readonly onChanged: Event<PythonEnvsChangedEvent>;
 
     private readonly watcher = new PythonEnvsWatcher();
 
-    private wrapped?: IResolvingLocator;
+    private wrapped?: IResolvingLocator<I>;
 
-    constructor(private readonly getLocator: GetLocatorFunc) {
+    constructor(private readonly getLocator: GetLocatorFunc<I>) {
         super();
         this.onChanged = this.watcher.onChanged;
     }
 
-    protected async *doIterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+    protected async *doIterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<I> {
         yield* this.wrapped!.iterEnvs(query);
     }
 
     public async resolveEnv(env: string): Promise<PythonEnvInfo | undefined> {
+        await this.ensureResourcesReady();
         return this.wrapped!.resolveEnv(env);
     }