Skip to content

Commit 265d130

Browse files
authored
Normalize file paths across environment managers (#1222)
Ensure consistent handling of file paths by normalizing them across different environment managers. This change improves the reliability of path comparisons and operations within the codebase.
1 parent e4b1332 commit 265d130

File tree

7 files changed

+167
-111
lines changed

7 files changed

+167
-111
lines changed

src/managers/builtin/sysPythonManager.ts

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from '../../api';
2020
import { SysManagerStrings } from '../../common/localize';
2121
import { createDeferred, Deferred } from '../../common/utils/deferred';
22+
import { normalizePath } from '../../common/utils/pathUtils';
2223
import { NativePythonFinder } from '../common/nativePythonFinder';
2324
import { getLatest } from '../common/utils';
2425
import {
@@ -134,7 +135,7 @@ export class SysPythonManager implements EnvironmentManager {
134135
}
135136

136137
if (scope instanceof Uri) {
137-
const env = this.fsPathToEnv.get(scope.fsPath);
138+
const env = this.fsPathToEnv.get(normalizePath(scope.fsPath));
138139
if (env) {
139140
return [env];
140141
}
@@ -171,10 +172,11 @@ export class SysPythonManager implements EnvironmentManager {
171172
return;
172173
}
173174

175+
const normalizedPwPath = normalizePath(pw.uri.fsPath);
174176
if (environment) {
175-
this.fsPathToEnv.set(pw.uri.fsPath, environment);
177+
this.fsPathToEnv.set(normalizedPwPath, environment);
176178
} else {
177-
this.fsPathToEnv.delete(pw.uri.fsPath);
179+
this.fsPathToEnv.delete(normalizedPwPath);
178180
}
179181
await setSystemEnvForWorkspace(pw.uri.fsPath, environment?.environmentPath.fsPath);
180182
}
@@ -191,11 +193,12 @@ export class SysPythonManager implements EnvironmentManager {
191193

192194
const before: Map<string, PythonEnvironment | undefined> = new Map();
193195
projects.forEach((p) => {
194-
before.set(p.uri.fsPath, this.fsPathToEnv.get(p.uri.fsPath));
196+
const normalizedPath = normalizePath(p.uri.fsPath);
197+
before.set(p.uri.fsPath, this.fsPathToEnv.get(normalizedPath));
195198
if (environment) {
196-
this.fsPathToEnv.set(p.uri.fsPath, environment);
199+
this.fsPathToEnv.set(normalizedPath, environment);
197200
} else {
198-
this.fsPathToEnv.delete(p.uri.fsPath);
201+
this.fsPathToEnv.delete(normalizedPath);
199202
}
200203
});
201204

@@ -282,24 +285,28 @@ export class SysPythonManager implements EnvironmentManager {
282285
}
283286

284287
private findEnvironmentByPath(fsPath: string): PythonEnvironment | undefined {
285-
const normalized = path.normalize(fsPath); // /opt/homebrew/bin/python3.12
288+
const normalized = normalizePath(fsPath);
286289
return this.collection.find((e) => {
287-
const n = path.normalize(e.environmentPath.fsPath);
288-
return n === normalized || path.dirname(n) === normalized || path.dirname(path.dirname(n)) === normalized;
290+
const n = normalizePath(e.environmentPath.fsPath);
291+
return (
292+
n === normalized ||
293+
normalizePath(path.dirname(e.environmentPath.fsPath)) === normalized ||
294+
normalizePath(path.dirname(path.dirname(e.environmentPath.fsPath))) === normalized
295+
);
289296
});
290297
}
291298

292299
private fromEnvMap(uri: Uri): PythonEnvironment | undefined {
293300
// Find environment directly using the URI mapping
294-
const env = this.fsPathToEnv.get(uri.fsPath);
301+
const env = this.fsPathToEnv.get(normalizePath(uri.fsPath));
295302
if (env) {
296303
return env;
297304
}
298305

299306
// Find environment using the Python project for the Uri
300307
const project = this.api.getPythonProject(uri);
301308
if (project) {
302-
return this.fsPathToEnv.get(project.uri.fsPath);
309+
return this.fsPathToEnv.get(normalizePath(project.uri.fsPath));
303310
}
304311

305312
return this.globalEnv;
@@ -332,24 +339,26 @@ export class SysPythonManager implements EnvironmentManager {
332339
}
333340

334341
// Try to find workspace environments
335-
const paths = this.api.getPythonProjects().map((p) => p.uri.fsPath);
342+
const projects = this.api.getPythonProjects();
336343

337-
// Iterate over each path
338-
for (const p of paths) {
339-
const env = await getSystemEnvForWorkspace(p);
344+
// Iterate over each project
345+
for (const project of projects) {
346+
const originalPath = project.uri.fsPath;
347+
const normalizedPath = normalizePath(originalPath);
348+
const env = await getSystemEnvForWorkspace(originalPath);
340349

341350
if (env) {
342351
const found = this.findEnvironmentByPath(env);
343352

344353
if (found) {
345-
this.fsPathToEnv.set(p, found);
354+
this.fsPathToEnv.set(normalizedPath, found);
346355
} else {
347356
// If not found, resolve the path.
348357
const resolved = await resolveSystemPythonEnvironmentPath(env, this.nativeFinder, this.api, this);
349358

350359
if (resolved) {
351360
// If resolved add it to the collection.
352-
this.fsPathToEnv.set(p, resolved);
361+
this.fsPathToEnv.set(normalizedPath, resolved);
353362
this.collection.push(resolved);
354363
} else {
355364
this.log.error(`Failed to resolve python environment: ${env}`);

src/managers/builtin/venvManager.ts

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -291,15 +291,15 @@ export class VenvManager implements EnvironmentManager {
291291
}
292292

293293
private updateCollection(environment: PythonEnvironment): void {
294-
this.collection = this.collection.filter(
295-
(e) => e.environmentPath.fsPath !== environment.environmentPath.fsPath,
296-
);
294+
const envPath = normalizePath(environment.environmentPath.fsPath);
295+
this.collection = this.collection.filter((e) => normalizePath(e.environmentPath.fsPath) !== envPath);
297296
}
298297

299298
private updateFsPathToEnv(environment: PythonEnvironment): Uri[] {
299+
const envPath = normalizePath(environment.environmentPath.fsPath);
300300
const changed: Uri[] = [];
301301
this.fsPathToEnv.forEach((env, uri) => {
302-
if (env.environmentPath.fsPath === environment.environmentPath.fsPath) {
302+
if (normalizePath(env.environmentPath.fsPath) === envPath) {
303303
this.fsPathToEnv.delete(uri);
304304
changed.push(Uri.file(uri));
305305
}
@@ -361,7 +361,7 @@ export class VenvManager implements EnvironmentManager {
361361
return [];
362362
}
363363

364-
const env = this.fsPathToEnv.get(scope.fsPath);
364+
const env = this.fsPathToEnv.get(normalizePath(scope.fsPath));
365365
return env ? [env] : [];
366366
}
367367

@@ -378,7 +378,7 @@ export class VenvManager implements EnvironmentManager {
378378
return this.globalEnv;
379379
}
380380

381-
let env = this.fsPathToEnv.get(project.uri.fsPath);
381+
let env = this.fsPathToEnv.get(normalizePath(project.uri.fsPath));
382382
if (!env) {
383383
env = this.findEnvironmentByPath(project.uri.fsPath);
384384
}
@@ -414,11 +414,12 @@ export class VenvManager implements EnvironmentManager {
414414
}
415415
}
416416

417-
const before = this.fsPathToEnv.get(pw.uri.fsPath);
417+
const normalizedPwPath = normalizePath(pw.uri.fsPath);
418+
const before = this.fsPathToEnv.get(normalizedPwPath);
418419
if (environment) {
419-
this.fsPathToEnv.set(pw.uri.fsPath, environment);
420+
this.fsPathToEnv.set(normalizedPwPath, environment);
420421
} else {
421-
this.fsPathToEnv.delete(pw.uri.fsPath);
422+
this.fsPathToEnv.delete(normalizedPwPath);
422423
}
423424
await setVenvForWorkspace(pw.uri.fsPath, environment?.environmentPath.fsPath);
424425

@@ -439,11 +440,12 @@ export class VenvManager implements EnvironmentManager {
439440

440441
const before: Map<string, PythonEnvironment | undefined> = new Map();
441442
projects.forEach((p) => {
442-
before.set(p.uri.fsPath, this.fsPathToEnv.get(p.uri.fsPath));
443+
const normalizedPath = normalizePath(p.uri.fsPath);
444+
before.set(p.uri.fsPath, this.fsPathToEnv.get(normalizedPath));
443445
if (environment) {
444-
this.fsPathToEnv.set(p.uri.fsPath, environment);
446+
this.fsPathToEnv.set(normalizedPath, environment);
445447
} else {
446-
this.fsPathToEnv.delete(p.uri.fsPath);
448+
this.fsPathToEnv.delete(normalizedPath);
447449
}
448450
});
449451

@@ -572,16 +574,17 @@ export class VenvManager implements EnvironmentManager {
572574
this.fsPathToEnv.clear();
573575

574576
const sorted = sortEnvironments(this.collection);
575-
const projectPaths = this.api.getPythonProjects().map((p) => normalizePath(p.uri.fsPath));
577+
const projects = this.api.getPythonProjects();
576578
const events: (() => void)[] = [];
577579
// Iterates through all workspace projects
578-
for (const p of projectPaths) {
579-
const env = await getVenvForWorkspace(p);
580+
for (const project of projects) {
581+
const originalPath = project.uri.fsPath;
582+
const normalizedPath = normalizePath(originalPath);
583+
const env = await getVenvForWorkspace(originalPath);
580584
if (env) {
581585
// from env path find PythonEnvironment object in the collection.
582586
let foundEnv = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
583-
const previousEnv = this.fsPathToEnv.get(p);
584-
const pw = this.api.getPythonProject(Uri.file(p));
587+
const previousEnv = this.fsPathToEnv.get(normalizedPath);
585588
if (!foundEnv) {
586589
// attempt to resolve
587590
const resolved = await resolveVenvPythonEnvironmentPath(
@@ -601,20 +604,20 @@ export class VenvManager implements EnvironmentManager {
601604
}
602605
}
603606
// Given found env, add it to the map and fire the event if needed.
604-
this.fsPathToEnv.set(p, foundEnv);
605-
if (pw && previousEnv?.envId.id !== foundEnv.envId.id) {
607+
this.fsPathToEnv.set(normalizedPath, foundEnv);
608+
if (previousEnv?.envId.id !== foundEnv.envId.id) {
606609
events.push(() =>
607-
this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: foundEnv }),
610+
this._onDidChangeEnvironment.fire({ uri: project.uri, old: undefined, new: foundEnv }),
608611
);
609612
}
610613
} else {
611614
// 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.
612615
const found = sorted.find((e) => {
613616
const t = this.api.getPythonProject(e.environmentPath)?.uri.fsPath;
614-
return t && normalizePath(t) === p;
617+
return t && normalizePath(t) === normalizedPath;
615618
});
616619
if (found) {
617-
this.fsPathToEnv.set(p, found);
620+
this.fsPathToEnv.set(normalizedPath, found);
618621
}
619622
}
620623
}

src/managers/common/utils.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ export function isGreater(a: string | undefined, b: string | undefined): boolean
6868

6969
export function sortEnvironments(collection: PythonEnvironment[]): PythonEnvironment[] {
7070
return collection.sort((a, b) => {
71+
// Environments with errors should be sorted to the end
72+
if (a.error && !b.error) {
73+
return 1;
74+
}
75+
if (!a.error && b.error) {
76+
return -1;
77+
}
7178
if (a.version !== b.version) {
7279
return isGreater(a.version, b.version) ? -1 : 1;
7380
}
@@ -83,8 +90,12 @@ export function getLatest(collection: PythonEnvironment[]): PythonEnvironment |
8390
if (collection.length === 0) {
8491
return undefined;
8592
}
86-
let latest = collection[0];
87-
for (const env of collection) {
93+
// Filter out environments with errors first, then find latest
94+
const nonErroredEnvs = collection.filter((e) => !e.error);
95+
const candidates = nonErroredEnvs.length > 0 ? nonErroredEnvs : collection;
96+
97+
let latest = candidates[0];
98+
for (const env of candidates) {
8899
if (isGreater(env.version, latest.version)) {
89100
latest = env;
90101
}

0 commit comments

Comments
 (0)