Point release for 2024.2 (#23005)

anthonykim1 · Kartik Raj · web-flow · commit ea8bba850b5c · 2024-02-29T10:52:23.000-08:00
Kartik's fixes

---------

Co-authored-by: Kartik Raj &lt;karraj@microsoft.com&gt;
diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts
@@ -20,7 +20,7 @@ export type PythonEnvUpdatedEvent<I = PythonEnvInfo> = {
     /**
      * The iteration index of The env info that was previously provided.
      */
-    index?: number;
+    index: number;
     /**
      * The env info that was previously provided.
      */
@@ -243,7 +243,7 @@ export interface IDiscoveryAPI {
     resolveEnv(path: string): Promise<PythonEnvInfo | undefined>;
 }
 
-interface IEmitter<E> {
+export interface IEmitter<E> {
     fire(e: E): void;
 }
 
diff --git a/src/client/pythonEnvironments/base/locatorUtils.ts b/src/client/pythonEnvironments/base/locatorUtils.ts
@@ -95,8 +95,6 @@ export async function getEnvs<I = PythonEnvInfo>(iterator: IPythonEnvsIterator<I
                 }
                 // We don't worry about if envs[index] is set already.
                 envs[index] = update;
-            } else if (event.update) {
-                envs.push(event.update);
             }
         });
     }
diff --git a/src/client/pythonEnvironments/base/locators/common/resourceBasedLocator.ts b/src/client/pythonEnvironments/base/locators/common/resourceBasedLocator.ts
@@ -43,18 +43,9 @@ export abstract class LazyResourceBasedLocator extends Locator<BasicEnvInfo> imp
         await this.disposables.dispose();
     }
 
-    public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<BasicEnvInfo> {
-        const iterator = this.doIterEnvs(query);
-        const it = this._iterEnvs(iterator, query);
-        it.onUpdated = iterator.onUpdated;
-        return it;
-    }
-
-    private async *_iterEnvs(
-        iterator: IPythonEnvsIterator<BasicEnvInfo>,
-        query?: PythonLocatorQuery,
-    ): IPythonEnvsIterator<BasicEnvInfo> {
+    public async *iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator<BasicEnvInfo> {
         await this.activate();
+        const iterator = this.doIterEnvs(query);
         if (query?.envPath) {
             let result = await iterator.next();
             while (!result.done) {
diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionCache.ts
@@ -245,6 +245,10 @@ export class PythonEnvInfoCache extends PythonEnvsWatcher<PythonEnvCollectionCha
             // Any environment with complete information is flushed, so this env does not contain complete info.
             return false;
         }
+        if (env.version.micro === -1 || env.version.major === -1 || env.version.minor === -1) {
+            // Env should not contain incomplete versions.
+            return false;
+        }
         const { ctime, mtime } = await getFileInfo(env.executable.filename);
         if (ctime !== -1 && mtime !== -1 && ctime === env.executable.ctime && mtime === env.executable.mtime) {
             return true;
diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts
@@ -141,12 +141,12 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
         const updatesDone = createDeferred<void>();
 
         if (iterator.onUpdated !== undefined) {
-            iterator.onUpdated(async (event) => {
+            const listener = iterator.onUpdated(async (event) => {
                 if (isProgressEvent(event)) {
                     switch (event.stage) {
                         case ProgressReportStage.discoveryFinished:
                             state.done = true;
-                            // listener.dispose();
+                            listener.dispose();
                             break;
                         case ProgressReportStage.allPathsDiscovered:
                             if (!query) {
@@ -164,10 +164,6 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
                         seen[event.index] = event.update;
                     }
                     state.pending -= 1;
-                } else if (event.update) {
-                    // New env, add it to cache.
-                    seen.push(event.update);
-                    this.cache.addEnv(event.update);
                 }
                 if (state.done && state.pending === 0) {
                     updatesDone.resolve();
diff --git a/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts b/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts
@@ -49,15 +49,12 @@ async function* iterEnvsIterator(
     };
     const seen: BasicEnvInfo[] = [];
 
-    didUpdate.fire({ stage: ProgressReportStage.discoveryStarted });
     if (iterator.onUpdated !== undefined) {
-        iterator.onUpdated((event) => {
+        const listener = iterator.onUpdated((event) => {
             if (isProgressEvent(event)) {
                 if (event.stage === ProgressReportStage.discoveryFinished) {
                     state.done = true;
-                    // For super slow locators such as Windows registry, we expect updates even after discovery
-                    // is "officially" finished, hence do not dispose listeners.
-                    // listener.dispose();
+                    listener.dispose();
                 } else {
                     didUpdate.fire(event);
                 }
@@ -69,11 +66,15 @@ async function* iterEnvsIterator(
                 const oldEnv = seen[event.index];
                 seen[event.index] = event.update;
                 didUpdate.fire({ index: event.index, old: oldEnv, update: event.update });
-            } else if (event.update) {
-                didUpdate.fire({ update: event.update });
+            } else {
+                // This implies a problem in a downstream locator
+                traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`);
             }
+            state.pending -= 1;
             checkIfFinishedAndNotify(state, didUpdate);
         });
+    } else {
+        didUpdate.fire({ stage: ProgressReportStage.discoveryStarted });
     }
 
     let result = await iterator.next();
@@ -89,8 +90,10 @@ async function* iterEnvsIterator(
         }
         result = await iterator.next();
     }
-    state.done = true;
-    checkIfFinishedAndNotify(state, didUpdate);
+    if (iterator.onUpdated === undefined) {
+        state.done = true;
+        checkIfFinishedAndNotify(state, didUpdate);
+    }
 }
 
 async function resolveDifferencesInBackground(
@@ -124,8 +127,8 @@ function checkIfFinishedAndNotify(
 ) {
     if (state.done && state.pending === 0) {
         didUpdate.fire({ stage: ProgressReportStage.discoveryFinished });
+        didUpdate.dispose();
         traceVerbose(`Finished with environment reducer`);
-        state.done = false; // No need to notify again.
     }
 }
 
diff --git a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts
@@ -81,30 +81,29 @@ export class PythonEnvsResolver implements IResolvingLocator {
         const seen: PythonEnvInfo[] = [];
 
         if (iterator.onUpdated !== undefined) {
-            iterator.onUpdated(async (event) => {
+            const listener = iterator.onUpdated(async (event) => {
                 state.pending += 1;
                 if (isProgressEvent(event)) {
                     if (event.stage === ProgressReportStage.discoveryFinished) {
                         didUpdate.fire({ stage: ProgressReportStage.allPathsDiscovered });
                         state.done = true;
-                        // For super slow locators such as Windows registry, we expect updates even after discovery
-                        // is "officially" finished, hence do not dispose listeners.
-                        // listener.dispose();
+                        listener.dispose();
                     } else {
                         didUpdate.fire(event);
                     }
                 } else if (event.update === undefined) {
                     throw new Error(
                         'Unsupported behavior: `undefined` environment updates are not supported from downstream locators in resolver',
                     );
-                } else if (event.index && seen[event.index] !== undefined) {
+                } else if (event.index !== undefined && seen[event.index] !== undefined) {
                     const old = seen[event.index];
                     await setKind(event.update, environmentKinds);
                     seen[event.index] = await resolveBasicEnv(event.update);
                     didUpdate.fire({ old, index: event.index, update: seen[event.index] });
                     this.resolveInBackground(event.index, state, didUpdate, seen).ignoreErrors();
                 } else {
-                    didUpdate.fire({ update: await this.resolveEnv(event.update.executablePath) });
+                    // This implies a problem in a downstream locator
+                    traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`);
                 }
                 state.pending -= 1;
                 checkIfFinishedAndNotify(state, didUpdate);
@@ -174,6 +173,7 @@ function checkIfFinishedAndNotify(
 ) {
     if (state.done && state.pending === 0) {
         didUpdate.fire({ stage: ProgressReportStage.discoveryFinished });
+        didUpdate.dispose();
         traceVerbose(`Finished with environment resolver`);
     }
 }
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts
@@ -3,76 +3,53 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { EventEmitter } from 'vscode';
 import { PythonEnvKind, PythonEnvSource } from '../../info';
-import {
-    BasicEnvInfo,
-    IPythonEnvsIterator,
-    Locator,
-    ProgressNotificationEvent,
-    ProgressReportStage,
-    PythonEnvUpdatedEvent,
-} from '../../locator';
+import { BasicEnvInfo, IPythonEnvsIterator, Locator, PythonLocatorQuery, IEmitter } from '../../locator';
 import { getRegistryInterpreters } from '../../../common/windowsUtils';
 import { traceError, traceVerbose } from '../../../../logging';
 import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv';
 import { inExperiment } from '../../../common/externalDependencies';
 import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups';
+import { PythonEnvsChangedEvent } from '../../watcher';
+
+export const WINDOWS_REG_PROVIDER_ID = 'windows-registry';
 
 export class WindowsRegistryLocator extends Locator<BasicEnvInfo> {
-    public readonly providerId: string = 'windows-registry';
+    public readonly providerId: string = WINDOWS_REG_PROVIDER_ID;
 
     // eslint-disable-next-line class-methods-use-this
     public iterEnvs(
-        _?: unknown,
+        query?: PythonLocatorQuery,
         useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment),
     ): IPythonEnvsIterator<BasicEnvInfo> {
-        const didUpdate = new EventEmitter<PythonEnvUpdatedEvent<BasicEnvInfo> | ProgressNotificationEvent>();
-        const iterator = useWorkerThreads ? iterEnvsIterator(didUpdate) : oldIterEnvsIterator();
         if (useWorkerThreads) {
-            iterator.onUpdated = didUpdate.event;
+            /**
+             * Windows registry is slow and often not necessary, so notify completion immediately, but use watcher
+             * change events to signal for any new envs which are found.
+             */
+            if (query?.providerId === this.providerId) {
+                // Query via change event, so iterate all envs.
+                return iterateEnvs(true);
+            }
+            return iterateEnvsLazily(this.emitter);
         }
-        return iterator;
+        return iterateEnvs(false);
     }
 }
 
-/**
- * Windows registry is slow and often not necessary, so notify completion immediately, while still updating lazily as we find stuff.
- * To accomplish this, use an empty iterator while lazily firing environments using updates.
- */
-async function* iterEnvsIterator(
-    didUpdate: EventEmitter<PythonEnvUpdatedEvent<BasicEnvInfo> | ProgressNotificationEvent>,
-): IPythonEnvsIterator<BasicEnvInfo> {
-    updateLazily(didUpdate).ignoreErrors();
+async function* iterateEnvsLazily(changed: IEmitter<PythonEnvsChangedEvent>): IPythonEnvsIterator<BasicEnvInfo> {
+    loadAllEnvs(changed).ignoreErrors();
 }
 
-async function updateLazily(didUpdate: EventEmitter<PythonEnvUpdatedEvent<BasicEnvInfo> | ProgressNotificationEvent>) {
+async function loadAllEnvs(changed: IEmitter<PythonEnvsChangedEvent>) {
     traceVerbose('Searching for windows registry interpreters');
-    const interpreters = await getRegistryInterpreters(true);
-    for (const interpreter of interpreters) {
-        try {
-            // Filter out Microsoft Store app directories. We have a store app locator that handles this.
-            // The python.exe available in these directories might not be python. It can be a store install
-            // shortcut that takes you to microsoft store.
-            if (isMicrosoftStoreDir(interpreter.interpreterPath)) {
-                continue;
-            }
-            const env: BasicEnvInfo = {
-                kind: PythonEnvKind.OtherGlobal,
-                executablePath: interpreter.interpreterPath,
-                source: [PythonEnvSource.WindowsRegistry],
-            };
-            didUpdate.fire({ update: env });
-        } catch (ex) {
-            traceError(`Failed to process environment: ${interpreter}`, ex);
-        }
-    }
-    didUpdate.fire({ stage: ProgressReportStage.discoveryFinished });
+    await getRegistryInterpreters(true);
+    changed.fire({ providerId: WINDOWS_REG_PROVIDER_ID });
     traceVerbose('Finished searching for windows registry interpreters');
 }
 
-async function* oldIterEnvsIterator(): IPythonEnvsIterator<BasicEnvInfo> {
-    const interpreters = await getRegistryInterpreters(false);
+async function* iterateEnvs(useWorkerThreads: boolean): IPythonEnvsIterator<BasicEnvInfo> {
+    const interpreters = await getRegistryInterpreters(useWorkerThreads);
     for (const interpreter of interpreters) {
         try {
             // Filter out Microsoft Store app directories. We have a store app locator that handles this.
diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts
@@ -32,7 +32,7 @@ import { defaultShells } from '../../interpreter/activation/service';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
 import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
 import { getSearchPathEnvVarNames } from '../../common/utils/exec';
-import { EnvironmentVariables } from '../../common/variables/types';
+import { EnvironmentVariables, IEnvironmentVariablesProvider } from '../../common/variables/types';
 import { TerminalShellType } from '../../common/terminal/types';
 import { OSType } from '../../common/utils/platform';
 import { normCase } from '../../common/platform/fs-paths';
@@ -81,6 +81,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
         @inject(ITerminalDeactivateService) private readonly terminalDeactivateService: ITerminalDeactivateService,
         @inject(IPathUtils) private readonly pathUtils: IPathUtils,
         @inject(IShellIntegrationService) private readonly shellIntegrationService: IShellIntegrationService,
+        @inject(IEnvironmentVariablesProvider)
+        private readonly environmentVariablesProvider: IEnvironmentVariablesProvider,
     ) {
         this.separator = platform.osType === OSType.Windows ? ';' : ':';
         this.progressService = new ProgressService(this.shell);
@@ -119,6 +121,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                     this,
                     this.disposables,
                 );
+                this.environmentVariablesProvider.onDidEnvironmentVariablesChange(
+                    async (r: Resource) => {
+                        await this._applyCollection(r).ignoreErrors();
+                    },
+                    this,
+                    this.disposables,
+                );
                 this.applicationEnvironment.onDidChangeShell(
                     async (shell: string) => {
                         this.processEnvVars = undefined;
diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
@@ -38,6 +38,7 @@ import { PathUtils } from '../../../client/common/platform/pathUtils';
 import { PythonEnvType } from '../../../client/pythonEnvironments/base/info';
 import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
 import { IShellIntegrationService, ITerminalDeactivateService } from '../../../client/terminals/types';
+import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types';
 
 suite('Terminal Environment Variable Collection Service', () => {
     let platform: IPlatformService;
@@ -74,6 +75,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         interpreterService = mock<IInterpreterService>();
         context = mock<IExtensionContext>();
         shell = mock<IApplicationShell>();
+        const envVarProvider = mock<IEnvironmentVariablesProvider>();
         shellIntegrationService = mock<IShellIntegrationService>();
         when(shellIntegrationService.isWorking()).thenResolve(true);
         globalCollection = mock<GlobalEnvironmentVariableCollection>();
@@ -113,6 +115,7 @@ suite('Terminal Environment Variable Collection Service', () => {
             instance(terminalDeactivateService),
             new PathUtils(getOSType() === OSType.Windows),
             instance(shellIntegrationService),
+            instance(envVarProvider),
         );
     });
 
diff --git a/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsCollectionService.unit.test.ts
@@ -78,6 +78,9 @@ suite('Python envs locator - Environments Collection', async () => {
     ) {
         const env = buildEnvInfo({ executable, searchLocation, name, location, kind });
         env.id = id ?? env.id;
+        env.version.major = 3;
+        env.version.minor = 10;
+        env.version.micro = 10;
         return env;
     }
 
diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts
@@ -2,7 +2,10 @@
 // Licensed under the MIT License.
 
 import { getEnvs } from '../../../../../client/pythonEnvironments/base/locatorUtils';
-import { WindowsRegistryLocator } from '../../../../../client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator';
+import {
+    WindowsRegistryLocator,
+    WINDOWS_REG_PROVIDER_ID,
+} from '../../../../../client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator';
 import { assertBasicEnvsEqual } from '../envTestUtils';
 import { TEST_TIMEOUT } from '../../../../constants';
 import { getOSType, OSType } from '../../../../../client/common/utils/platform';
@@ -19,8 +22,8 @@ suite('Windows Registry Locator', async () => {
     });
 
     test('Worker thread to fetch registry interpreters is working', async () => {
-        const items = await getEnvs(locator.iterEnvs(undefined, false));
-        const workerItems = await getEnvs(locator.iterEnvs(undefined, true));
+        const items = await getEnvs(locator.iterEnvs({ providerId: WINDOWS_REG_PROVIDER_ID }, false));
+        const workerItems = await getEnvs(locator.iterEnvs({ providerId: WINDOWS_REG_PROVIDER_ID }, true));
         console.log('Number of items Windows registry locator returned:', items.length);
         // Make sure items returned when using worker threads v/s not are the same.
         assertBasicEnvsEqual(items, workerItems);
diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -95,8 +95,6 @@ export async function getEnvs<I = PythonEnvInfo>(iterator: IPythonEnvsIterator<I`
`95`	`95`	`}`
`96`	`96`	`// We don't worry about if envs[index] is set already.`
`97`	`97`	`envs[index] = update;`
`98`		`- } else if (event.update) {`
`99`		`- envs.push(event.update);`
`100`	`98`	`}`
`101`	`99`	`});`
`102`	`100`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,9 @@ suite('Python envs locator - Environments Collection', async () => {`
`78`	`78`	`) {`
`79`	`79`	`const env = buildEnvInfo({ executable, searchLocation, name, location, kind });`
`80`	`80`	`env.id = id ?? env.id;`
	`81`	`+ env.version.major = 3;`
	`82`	`+ env.version.minor = 10;`
	`83`	`+ env.version.micro = 10;`
`81`	`84`	`return env;`
`82`	`85`	`}`
`83`	`86`