Merge branch 'main' into fix/issue-1877-uv-managed-python-global

Author: eleanorjboyd

src/managers/builtin/main.ts

@@ -28,6 +28,9 @@ export async function registerSystemPythonFeatures(
         venvManager.watcherRefresh();
     });
     const activationWatcher = createFileSystemWatcher('{**/activate}', false, false, false);
+    // Deleting a folder is reported for the folder only, not for the files in it, so deleting a
+    // virtual environment folder never matches the activate script pattern above.
+    const deletionWatcher = createFileSystemWatcher('**/*', true, true, false);
     disposables.push(
         activationWatcher,
         activationWatcher.onDidCreate(() => {
@@ -39,6 +42,12 @@ export async function registerSystemPythonFeatures(
         activationWatcher.onDidDelete(() => {
             venvDebouncedRefresh.trigger();
         }),
+        deletionWatcher,
+        deletionWatcher.onDidDelete((uri) => {
+            if (venvManager.hasEnvironmentAt(uri.fsPath)) {
+                venvDebouncedRefresh.trigger();
+            }
+        }),
         onDidDeleteFiles(() => {
             venvDebouncedRefresh.trigger();
         }),

src/managers/builtin/venvManager.ts

@@ -53,6 +53,7 @@ import {
 
 export class VenvManager implements EnvironmentManager {
     private collection: PythonEnvironment[] = [];
+    private environmentFolders: { collection: PythonEnvironment[]; length: number; folders: Set<string> } | undefined;
     private readonly fsPathToEnv: Map<string, PythonEnvironment> = new Map();
     private globalEnv: PythonEnvironment | undefined;
     private skipWatcherRefresh = false;
@@ -314,6 +315,31 @@ export class VenvManager implements EnvironmentManager {
         return this.internalRefresh(undefined, true, VenvManagerStrings.venvRefreshing);
     }
 
+    /**
+     * Returns true when a known virtual environment is at, or inside, the given path.
+     *
+     * Called for every deletion in the workspace, so it is a single set lookup: the set holds every
+     * environment prefix and its parent folders, and is rebuilt when the collection changes.
+     *
+     * @param fsPath A file system path, for example of a deleted folder.
+     */
+    hasEnvironmentAt(fsPath: string): boolean {
+        const cache = this.environmentFolders;
+        if (cache?.collection !== this.collection || cache.length !== this.collection.length) {
+            const folders = new Set<string>();
+            for (const env of this.collection) {
+                let folder = env.sysPrefix ? path.resolve(env.sysPrefix) : undefined;
+                while (folder && !folders.has(normalizePath(folder))) {
+                    folders.add(normalizePath(folder));
+                    const parent = path.dirname(folder);
+                    folder = parent !== folder ? parent : undefined;
+                }
+            }
+            this.environmentFolders = { collection: this.collection, length: this.collection.length, folders };
+        }
+        return this.environmentFolders!.folders.has(normalizePath(path.resolve(fsPath)));
+    }
+
     private async internalRefresh(
         scope: RefreshEnvironmentsScope,
         hardRefresh: boolean,
@@ -575,6 +601,7 @@ export class VenvManager implements EnvironmentManager {
 
     /**
      * Loads and maps Python environments to their corresponding project paths in the workspace. about  O(p × e) where p = projects.len and e = environments.len
+     * Skips unresolvable selections without preventing other projects from restoring their environments.
      */
     private async loadEnvMap() {
         const globals = await this.baseManager.getEnvironments('global');
@@ -609,7 +636,7 @@ export class VenvManager implements EnvironmentManager {
                         foundEnv = resolved;
                     } else {
                         this.log.error(`Failed to resolve python environment: ${env}`);
-                        return;
+                        continue;
                     }
                 }
                 // Given found env, add it to the map and fire the event if needed.

src/test/integration/environmentFixture.ts

@@ -18,6 +18,7 @@ const OWNERSHIP_FILE_NAME = '.python-envs-test-owner.json';
 const COMMAND_TIMEOUT_MS = 180_000;
 const DISCOVERY_TIMEOUT_MS = 60_000;
 const API_REMOVAL_SETTLE_TIMEOUT_MS = 10_000;
+const CONDA_FIXTURE_PYTHON_VERSION = '3.12';
 
 export interface EnvironmentFixtureProvider {
     readonly environmentDirectory: string;
@@ -312,16 +313,12 @@ export function createCondaFixtureProvider(): EnvironmentFixtureProvider {
         environmentDirectory: '.conda',
         managerId: CONDA_MANAGER_ID,
         create: async (_api, prefix) => {
-            const version = await runFixtureCommand('python', [
-                '-c',
-                'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")',
-            ]);
             await runFixtureCommand(await getCondaExecutable(), [
                 'create',
                 '--yes',
                 '--prefix',
                 prefix.fsPath,
-                `python=${version.stdout.trim()}`,
+                `python=${CONDA_FIXTURE_PYTHON_VERSION}`,
             ]);
             return undefined;
         },

src/test/managers/builtin/main.unit.test.ts

@@ -2,9 +2,10 @@
 // Licensed under the MIT License.
 
 import * as assert from 'assert';
+import * as path from 'path';
 import * as sinon from 'sinon';
 import { Disposable, EventEmitter, LogOutputChannel, Uri } from 'vscode';
-import { PythonEnvironmentApi } from '../../../api';
+import { PythonEnvironment, PythonEnvironmentApi } from '../../../api';
 import * as workspaceApis from '../../../common/workspace.apis';
 import * as extensionApi from '../../../extensionApi';
 import { registerSystemPythonFeatures } from '../../../managers/builtin/main';
@@ -61,7 +62,9 @@ suite('registerSystemPythonFeatures - activation watcher', () => {
 
     test('activation watcher is created with change events enabled and triggers refresh on change', async () => {
         const mockWatcher = createMockWatcher();
-        createFileSystemWatcherStub.returns(mockWatcher);
+        const deletionWatcher = createMockWatcher();
+        createFileSystemWatcherStub.withArgs('{**/activate}').returns(mockWatcher);
+        createFileSystemWatcherStub.withArgs('**/*').returns(deletionWatcher);
 
         const disposables: Disposable[] = [];
         const nativeFinder = {} as NativePythonFinder;
@@ -76,7 +79,7 @@ suite('registerSystemPythonFeatures - activation watcher', () => {
         await registerSystemPythonFeatures(nativeFinder, disposables, log, envManager);
 
         // Assertion A: Watcher is created with [false, false, false]
-        assert.strictEqual(createFileSystemWatcherStub.callCount, 1);
+        assert.strictEqual(createFileSystemWatcherStub.callCount, 2);
         assert.deepStrictEqual(createFileSystemWatcherStub.firstCall.args, ['{**/activate}', false, false, false]);
 
         // Assertion B & C: CHANGE event triggers debounced watcherRefresh
@@ -112,4 +115,83 @@ suite('registerSystemPythonFeatures - activation watcher', () => {
         mockWatcher._deleteEmitter.dispose();
         disposables.forEach((d) => d.dispose());
     });
+
+    test('deleting a folder that holds a known venv triggers refresh', async () => {
+        const deletionWatcher = createMockWatcher();
+        createFileSystemWatcherStub.withArgs('{**/activate}').returns(createMockWatcher());
+        createFileSystemWatcherStub.withArgs('**/*').returns(deletionWatcher);
+        const venvFolder = Uri.file(path.resolve('workspace', '.venv'));
+        sandbox
+            .stub(VenvManager.prototype, 'hasEnvironmentAt')
+            .callsFake((fsPath: string) => fsPath === venvFolder.fsPath);
+
+        const disposables: Disposable[] = [];
+        await registerSystemPythonFeatures(
+            {} as NativePythonFinder,
+            disposables,
+            {} as LogOutputChannel,
+            {} as SysPythonManager,
+        );
+
+        // Only deletions are watched: the folder is reported, not the files in it.
+        assert.ok(createFileSystemWatcherStub.calledWithExactly('**/*', true, true, false));
+
+        deletionWatcher._deleteEmitter.fire(Uri.file(path.resolve('workspace', 'notes.txt')));
+        await clock.tickAsync(600);
+        assert.strictEqual(watcherRefreshStub.callCount, 0);
+
+        deletionWatcher._deleteEmitter.fire(venvFolder);
+        await clock.tickAsync(600);
+        assert.strictEqual(watcherRefreshStub.callCount, 1);
+
+        disposables.forEach((d) => d.dispose());
+    });
+});
+
+suite('VenvManager.hasEnvironmentAt', () => {
+    const project = path.resolve('venv-delete-project');
+    const venvPrefix = path.join(project, '.venv');
+
+    function createManager(): VenvManager {
+        const manager = new VenvManager(
+            {} as NativePythonFinder,
+            {} as PythonEnvironmentApi,
+            {} as SysPythonManager,
+            {} as LogOutputChannel,
+        );
+        (manager as unknown as { collection: PythonEnvironment[] }).collection = [
+            { sysPrefix: venvPrefix } as PythonEnvironment,
+        ];
+        return manager;
+    }
+
+    test('is true for the venv folder and its parent folders', () => {
+        const manager = createManager();
+
+        assert.strictEqual(manager.hasEnvironmentAt(venvPrefix), true);
+        assert.strictEqual(manager.hasEnvironmentAt(project), true);
+    });
+
+    test('is false for unrelated paths and paths inside the venv', () => {
+        const manager = createManager();
+
+        assert.strictEqual(manager.hasEnvironmentAt(path.join(project, 'src')), false);
+        assert.strictEqual(manager.hasEnvironmentAt(path.join(venvPrefix, 'Lib', 'site-packages', 'pkg')), false);
+        assert.strictEqual(manager.hasEnvironmentAt(path.resolve('venv-delete-project-other')), false);
+        assert.strictEqual(manager.hasEnvironmentAt(`${venvPrefix}2`), false);
+    });
+
+    test('follows changes to the known environments', () => {
+        const manager = createManager();
+        const otherPrefix = path.resolve('venv-delete-other', '.venv');
+        assert.strictEqual(manager.hasEnvironmentAt(otherPrefix), false);
+
+        const state = manager as unknown as { collection: PythonEnvironment[] };
+        state.collection.push({ sysPrefix: otherPrefix } as PythonEnvironment);
+        assert.strictEqual(manager.hasEnvironmentAt(otherPrefix), true);
+
+        state.collection = [];
+        assert.strictEqual(manager.hasEnvironmentAt(venvPrefix), false);
+        assert.strictEqual(manager.hasEnvironmentAt(otherPrefix), false);
+    });
 });

src/test/managers/builtin/venvManager.selection.unit.test.ts

@@ -0,0 +1,128 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import * as assert from 'assert';
+import * as path from 'path';
+import * as sinon from 'sinon';
+import * as typemoq from 'typemoq';
+import { CancellationTokenSource, LogOutputChannel, Uri } from 'vscode';
+import {
+    DidChangeEnvironmentEventArgs,
+    EnvironmentManager,
+    PythonEnvironmentApi,
+    PythonProject,
+} from '../../../api';
+import { VENV_MANAGER_ID } from '../../../common/constants';
+import * as windowApis from '../../../common/window.apis';
+import { VenvManager } from '../../../managers/builtin/venvManager';
+import * as venvUtils from '../../../managers/builtin/venvUtils';
+import { NativePythonFinder } from '../../../managers/common/nativePythonFinder';
+import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment';
+
+suite('VenvManager - project selection recovery', () => {
+    let sandbox: sinon.SinonSandbox;
+    let cancellation: CancellationTokenSource;
+
+    setup(() => {
+        sandbox = sinon.createSandbox();
+        cancellation = new CancellationTokenSource();
+        sandbox.stub(windowApis, 'withProgress').callsFake(async (_options, task) =>
+            task({ report: () => {} }, cancellation.token),
+        );
+        sandbox.stub(venvUtils, 'getVenvForGlobal').resolves(undefined);
+    });
+
+    teardown(() => {
+        sandbox.restore();
+        cancellation.dispose();
+    });
+
+    for (const operation of ['initialize', 'refresh'] as const) {
+        for (const brokenIndex of [0, 1]) {
+            test(`${operation} restores other projects when saved selection ${brokenIndex + 1} cannot resolve`, async () => {
+                const projects: PythonProject[] = ['a', 'b', 'c'].map((name) => ({
+                    name,
+                    uri: Uri.file(path.resolve('selection-recovery', `project-${name}`)),
+                }));
+                const environments = projects.map((project) =>
+                    createMockPythonEnvironment({
+                        name: project.name,
+                        envPath: path.resolve(
+                            'selection-recovery',
+                            `external-${project.name}`,
+                            process.platform === 'win32' ? 'Scripts' : 'bin',
+                            'python',
+                        ),
+                        managerId: VENV_MANAGER_ID,
+                    }),
+                );
+                const fallback = createMockPythonEnvironment({
+                    name: 'global',
+                    envPath: path.resolve('selection-recovery', 'global', 'python'),
+                });
+                const api = typemoq.Mock.ofType<PythonEnvironmentApi>();
+                api.setup((a) => a.getPythonProjects()).returns(() => projects);
+                api.setup((a) => a.getPythonProject(typemoq.It.isAny())).returns((uri: Uri) =>
+                    projects.find((project) => project.uri.fsPath === uri.fsPath),
+                );
+                const baseManager = typemoq.Mock.ofType<EnvironmentManager>();
+                baseManager.setup((m) => m.getEnvironments('global')).returns(async () => [fallback]);
+                const log = typemoq.Mock.ofType<LogOutputChannel>();
+                const manager = new VenvManager(
+                    typemoq.Mock.ofType<NativePythonFinder>().object,
+                    api.object,
+                    baseManager.object,
+                    log.object,
+                );
+                sandbox.stub(venvUtils, 'findVirtualEnvironments').callsFake(async () => []);
+                sandbox.stub(venvUtils, 'getVenvForWorkspace').callsFake(async (fsPath) => {
+                    const index = projects.findIndex((project) => project.uri.fsPath === fsPath);
+                    return environments[index]?.environmentPath.fsPath;
+                });
+                const resolve = sandbox
+                    .stub(venvUtils, 'resolveVenvPythonEnvironmentPath')
+                    .callsFake(async (fsPath) => environments.find((env) => env.environmentPath.fsPath === fsPath));
+                const events: DidChangeEnvironmentEventArgs[] = [];
+                const subscription = manager.onDidChangeEnvironment((event) => events.push(event));
+                try {
+                    if (operation === 'refresh') {
+                        await manager.initialize();
+                        for (const [index, project] of projects.entries()) {
+                            assert.strictEqual(await manager.get(project.uri), environments[index]);
+                        }
+                        events.length = 0;
+                    }
+
+                    const brokenPath = environments[brokenIndex].environmentPath.fsPath;
+                    resolve.withArgs(brokenPath).resolves(undefined);
+                    if (operation === 'initialize') {
+                        await manager.initialize();
+                    } else {
+                        await manager.refresh(undefined);
+                    }
+
+                    for (const [index, project] of projects.entries()) {
+                        if (index !== brokenIndex) {
+                            assert.strictEqual(
+                                (await manager.get(project.uri))?.environmentPath.fsPath,
+                                environments[index].environmentPath.fsPath,
+                                `Project ${project.name} must retain its selected interpreter`,
+                            );
+                        }
+                    }
+                    assert.deepStrictEqual(
+                        events.map((event) => event.uri?.fsPath),
+                        projects.filter((_, index) => index !== brokenIndex).map((project) => project.uri.fsPath),
+                        'Healthy projects must still publish their restored selections',
+                    );
+                    log.verify(
+                        (l) => l.error(`Failed to resolve python environment: ${brokenPath}`),
+                        typemoq.Times.once(),
+                    );
+                } finally {
+                    subscription.dispose();
+                }
+            });
+        }
+    }
+});