From c1fbf2eaba7e4bf90b2d84cae9c2d9f532e5a8e0 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 19 Jun 2025 09:32:30 -0700 Subject: [PATCH] fixes recursive venv assignment and incorrect multiroot result --- src/managers/builtin/venvManager.ts | 78 ++++++++++++++++------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 122d5e8c..3d6ad7b1 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -91,11 +91,13 @@ export class VenvManager implements EnvironmentManager { } } + /** + * Returns configuration for quick create in the workspace root, undefined if no suitable Python 3 version is found. + */ quickCreateConfig(): QuickCreateConfig | undefined { if (!this.globalEnv || !this.globalEnv.version.startsWith('3.')) { return undefined; } - return { description: l10n.t('Create a virtual environment in workspace root'), detail: l10n.t( @@ -169,6 +171,9 @@ export class VenvManager implements EnvironmentManager { } } + /** + * Removes the specified Python environment, updates internal collections, and fires change events as needed. + */ async remove(environment: PythonEnvironment): Promise { try { this.skipWatcherRefresh = true; @@ -422,6 +427,9 @@ export class VenvManager implements EnvironmentManager { await this.loadGlobalEnv(globals); } + /** + * Loads and sets the global Python environment from the provided list, resolving if necessary. O(g) where g = globals.length + */ private async loadGlobalEnv(globals: PythonEnvironment[]) { this.globalEnv = undefined; @@ -454,6 +462,9 @@ 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 + */ private async loadEnvMap() { const globals = await this.baseManager.getEnvironments('global'); await this.loadGlobalEnv(globals); @@ -461,23 +472,18 @@ export class VenvManager implements EnvironmentManager { this.fsPathToEnv.clear(); const sorted = sortEnvironments(this.collection); - const paths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath)); + const projectPaths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath)); const events: (() => void)[] = []; - for (const p of paths) { + // Iterates through all workspace projects + for (const p of projectPaths) { const env = await getVenvForWorkspace(p); - if (env) { - const found = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals); - const previous = this.fsPathToEnv.get(p); + // from env path find PythonEnvironment object in the collection. + let foundEnv = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals); + const previousEnv = this.fsPathToEnv.get(p); const pw = this.api.getPythonProject(Uri.file(p)); - if (found) { - this.fsPathToEnv.set(p, found); - if (pw && previous?.envId.id !== found.envId.id) { - events.push(() => - this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: found }), - ); - } - } else { + if (!foundEnv) { + // attempt to resolve const resolved = await resolveVenvPythonEnvironmentPath( env, this.nativeFinder, @@ -486,32 +492,29 @@ export class VenvManager implements EnvironmentManager { this.baseManager, ); if (resolved) { - // If resolved add it to the collection - this.fsPathToEnv.set(p, resolved); + // If resolved; add it to the venvManager collection this.addEnvironment(resolved, false); - if (pw && previous?.envId.id !== resolved.envId.id) { - events.push(() => - this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: resolved }), - ); - } + foundEnv = resolved; } else { this.log.error(`Failed to resolve python environment: ${env}`); + return; } } + // Given found env, add it to the map and fire the event if needed. + this.fsPathToEnv.set(p, foundEnv); + if (pw && previousEnv?.envId.id !== foundEnv.envId.id) { + events.push(() => + this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: foundEnv }), + ); + } } else { - // There is NO selected venv, then try and choose the venv that is in the workspace. - if (sorted.length === 1) { - this.fsPathToEnv.set(p, sorted[0]); - } else { - // These are sorted by version and by path length. The assumption is that the user would want to pick - // latest version and the one that is closest to the workspace. - const found = sorted.find((e) => { - const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath; - return t && path.normalize(t) === p; - }); - if (found) { - this.fsPathToEnv.set(p, found); - } + // Search through all known environments (e) and check if any are associated with the current project path. If so, add that environment and path in the map. + const found = sorted.find((e) => { + const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath; + return t && path.normalize(t) === p; + }); + if (found) { + this.fsPathToEnv.set(p, found); } } } @@ -519,6 +522,9 @@ export class VenvManager implements EnvironmentManager { events.forEach((e) => e()); } + /** + * Finds a PythonEnvironment in the given collection (or all environments) that matches the provided file system path. O(e) where e = environments.len + */ private findEnvironmentByPath(fsPath: string, collection?: PythonEnvironment[]): PythonEnvironment | undefined { const normalized = path.normalize(fsPath); const envs = collection ?? this.collection; @@ -528,6 +534,10 @@ export class VenvManager implements EnvironmentManager { }); } + /** + * Returns all Python projects associated with the given environment. + * O(p), where p is project.len + */ public getProjectsByEnvironment(environment: PythonEnvironment): PythonProject[] { const projects: PythonProject[] = []; this.fsPathToEnv.forEach((env, fsPath) => {