Skip to content

fixes recursive venv assignment and incorrect multiroot result #558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2025
Merged

fixes recursive venv assignment and incorrect multiroot result #558

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 44 additions & 34 deletions src/managers/builtin/venvManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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<void> {
try {
this.skipWatcherRefresh = true;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -454,30 +462,28 @@ 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);

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,
Expand All @@ -486,39 +492,39 @@ 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);
}
}
}

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;
Expand All @@ -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) => {
Expand Down