Skip to content

Commit c1fbf2e

Browse files
committed
fixes recursive venv assignment and incorrect multiroot result
1 parent 0157b2a commit c1fbf2e

1 file changed

Lines changed: 44 additions & 34 deletions

File tree

src/managers/builtin/venvManager.ts

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,13 @@ export class VenvManager implements EnvironmentManager {
9191
}
9292
}
9393

94+
/**
95+
* Returns configuration for quick create in the workspace root, undefined if no suitable Python 3 version is found.
96+
*/
9497
quickCreateConfig(): QuickCreateConfig | undefined {
9598
if (!this.globalEnv || !this.globalEnv.version.startsWith('3.')) {
9699
return undefined;
97100
}
98-
99101
return {
100102
description: l10n.t('Create a virtual environment in workspace root'),
101103
detail: l10n.t(
@@ -169,6 +171,9 @@ export class VenvManager implements EnvironmentManager {
169171
}
170172
}
171173

174+
/**
175+
* Removes the specified Python environment, updates internal collections, and fires change events as needed.
176+
*/
172177
async remove(environment: PythonEnvironment): Promise<void> {
173178
try {
174179
this.skipWatcherRefresh = true;
@@ -422,6 +427,9 @@ export class VenvManager implements EnvironmentManager {
422427
await this.loadGlobalEnv(globals);
423428
}
424429

430+
/**
431+
* Loads and sets the global Python environment from the provided list, resolving if necessary. O(g) where g = globals.length
432+
*/
425433
private async loadGlobalEnv(globals: PythonEnvironment[]) {
426434
this.globalEnv = undefined;
427435

@@ -454,30 +462,28 @@ export class VenvManager implements EnvironmentManager {
454462
}
455463
}
456464

465+
/**
466+
* 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
467+
*/
457468
private async loadEnvMap() {
458469
const globals = await this.baseManager.getEnvironments('global');
459470
await this.loadGlobalEnv(globals);
460471

461472
this.fsPathToEnv.clear();
462473

463474
const sorted = sortEnvironments(this.collection);
464-
const paths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath));
475+
const projectPaths = this.api.getPythonProjects().map((p) => path.normalize(p.uri.fsPath));
465476
const events: (() => void)[] = [];
466-
for (const p of paths) {
477+
// Iterates through all workspace projects
478+
for (const p of projectPaths) {
467479
const env = await getVenvForWorkspace(p);
468-
469480
if (env) {
470-
const found = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
471-
const previous = this.fsPathToEnv.get(p);
481+
// from env path find PythonEnvironment object in the collection.
482+
let foundEnv = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
483+
const previousEnv = this.fsPathToEnv.get(p);
472484
const pw = this.api.getPythonProject(Uri.file(p));
473-
if (found) {
474-
this.fsPathToEnv.set(p, found);
475-
if (pw && previous?.envId.id !== found.envId.id) {
476-
events.push(() =>
477-
this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: found }),
478-
);
479-
}
480-
} else {
485+
if (!foundEnv) {
486+
// attempt to resolve
481487
const resolved = await resolveVenvPythonEnvironmentPath(
482488
env,
483489
this.nativeFinder,
@@ -486,39 +492,39 @@ export class VenvManager implements EnvironmentManager {
486492
this.baseManager,
487493
);
488494
if (resolved) {
489-
// If resolved add it to the collection
490-
this.fsPathToEnv.set(p, resolved);
495+
// If resolved; add it to the venvManager collection
491496
this.addEnvironment(resolved, false);
492-
if (pw && previous?.envId.id !== resolved.envId.id) {
493-
events.push(() =>
494-
this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: resolved }),
495-
);
496-
}
497+
foundEnv = resolved;
497498
} else {
498499
this.log.error(`Failed to resolve python environment: ${env}`);
500+
return;
499501
}
500502
}
503+
// Given found env, add it to the map and fire the event if needed.
504+
this.fsPathToEnv.set(p, foundEnv);
505+
if (pw && previousEnv?.envId.id !== foundEnv.envId.id) {
506+
events.push(() =>
507+
this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: foundEnv }),
508+
);
509+
}
501510
} else {
502-
// There is NO selected venv, then try and choose the venv that is in the workspace.
503-
if (sorted.length === 1) {
504-
this.fsPathToEnv.set(p, sorted[0]);
505-
} else {
506-
// These are sorted by version and by path length. The assumption is that the user would want to pick
507-
// latest version and the one that is closest to the workspace.
508-
const found = sorted.find((e) => {
509-
const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath;
510-
return t && path.normalize(t) === p;
511-
});
512-
if (found) {
513-
this.fsPathToEnv.set(p, found);
514-
}
511+
// 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.
512+
const found = sorted.find((e) => {
513+
const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath;
514+
return t && path.normalize(t) === p;
515+
});
516+
if (found) {
517+
this.fsPathToEnv.set(p, found);
515518
}
516519
}
517520
}
518521

519522
events.forEach((e) => e());
520523
}
521524

525+
/**
526+
* Finds a PythonEnvironment in the given collection (or all environments) that matches the provided file system path. O(e) where e = environments.len
527+
*/
522528
private findEnvironmentByPath(fsPath: string, collection?: PythonEnvironment[]): PythonEnvironment | undefined {
523529
const normalized = path.normalize(fsPath);
524530
const envs = collection ?? this.collection;
@@ -528,6 +534,10 @@ export class VenvManager implements EnvironmentManager {
528534
});
529535
}
530536

537+
/**
538+
* Returns all Python projects associated with the given environment.
539+
* O(p), where p is project.len
540+
*/
531541
public getProjectsByEnvironment(environment: PythonEnvironment): PythonProject[] {
532542
const projects: PythonProject[] = [];
533543
this.fsPathToEnv.forEach((env, fsPath) => {

0 commit comments

Comments
 (0)