Skip to content

Commit d9fd3ec

Browse files
CopilotCopilot
authored
Use normalizePath for cross-platform path comparisons on Windows (#1604)
Several path-equality checks used `path.normalize()` — or worse, compared `path.resolve()` output against `path.normalize()` output. On Windows these differ in drive-letter case and slash direction, causing settings/override and project matching to silently fail. All affected comparisons now route through the existing `normalizePath()` helper (slash-normalized + Windows case-folded) on both sides. ### Changes - **`settingHelpers.ts` / `interpreterSelection.ts`** — fixed the core bug: comparisons of the form `path.resolve(ws, s.path) === path.normalize(project)`. Both sides now normalized, fixing pythonProjects override lookups (env/package manager resolution, add/remove/rename) on Windows. - **`projectManager.ts`, `creators/existingProjects.ts`, `creators/autoFindProjects.ts`, `terminal/terminalManager.ts`, `views/utils.ts`** — migrated remaining equality checks and map keys to `normalizePath()` for case-insensitive matching. - **Left untouched** — non-comparison uses of `path.normalize()` (drive-root/depth validation, `dirname` manipulation, `fileNameUtils` which depends on `path.sep`). - Added unit tests for `views/utils.removable` covering Windows drive-case/slash equivalence and POSIX case-sensitivity. ### Example ```ts // before — resolve() adds drive letter, normalize() does not; mismatch on Windows const pwPath = path.normalize(e.project.uri.fsPath); overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath); // after — consistent normalization on both sides const pwPath = normalizePath(e.project.uri.fsPath); overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath); ``` The `findEnvironmentByPath` methods named in the issue already use `normalizePath()`; this change extends that convention to the remaining comparison sites. Fixes #1181 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 036e883 commit d9fd3ec

8 files changed

Lines changed: 114 additions & 32 deletions

File tree

‎src/features/creators/autoFindProjects.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { traceInfo } from '../../common/logging';
66
import { showErrorMessage, showQuickPickWithButtons, showWarningMessage } from '../../common/window.apis';
77
import { findFiles } from '../../common/workspace.apis';
88
import { PythonProjectManager, PythonProjectsImpl } from '../../internal.api';
9+
import { normalizePath } from '../../common/utils/pathUtils';
910

1011
function getUniqueUri(uris: Uri[]): {
1112
label: string;
@@ -73,8 +74,8 @@ export class AutoFindProjects implements PythonProjectCreator {
7374
// Skip this project if:
7475
// 1. There's already a project registered with exactly the same path
7576
// 2. There's already a project registered with this project's parent directory path
76-
const np = path.normalize(p.uri.fsPath);
77-
const nf = path.normalize(uri.fsPath);
77+
const np = normalizePath(p.uri.fsPath);
78+
const nf = normalizePath(uri.fsPath);
7879
const nfp = path.dirname(nf);
7980
return np !== nf && np !== nfp;
8081
}

‎src/features/creators/existingProjects.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { ProjectCreatorString } from '../../common/localize';
55
import { traceInfo, traceLog } from '../../common/logging';
66
import { showOpenDialog, showWarningMessage } from '../../common/window.apis';
77
import { PythonProjectManager, PythonProjectsImpl } from '../../internal.api';
8+
import { normalizePath } from '../../common/utils/pathUtils';
89

910
export class ExistingProjects implements PythonProjectCreator {
1011
public readonly name = 'existingProjects';
@@ -47,8 +48,8 @@ export class ExistingProjects implements PythonProjectCreator {
4748
const p = this.pm.get(uri);
4849
if (p) {
4950
// Skip this project if there's already a project registered with exactly the same path
50-
const np = path.normalize(p.uri.fsPath);
51-
const nf = path.normalize(uri.fsPath);
51+
const np = normalizePath(p.uri.fsPath);
52+
const nf = normalizePath(uri.fsPath);
5253
return np !== nf;
5354
}
5455
return true;

‎src/features/interpreterSelection.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { StopWatch } from '../common/stopWatch';
1010
import { EventNames } from '../common/telemetry/constants';
1111
import { sendTelemetryEvent } from '../common/telemetry/sender';
1212
import { resolveVariables } from '../common/utils/internalVariables';
13+
import { normalizePath } from '../common/utils/pathUtils';
1314
import { showWarningMessage } from '../common/window.apis';
1415
import {
1516
getConfiguration,
@@ -534,8 +535,8 @@ function getProjectSpecificEnvManager(projectManager: PythonProjectManager, scop
534535
const pw = projectManager.get(scope);
535536
const w = getWorkspaceFolder(scope);
536537
if (pw && w) {
537-
const pwPath = path.resolve(pw.uri.fsPath);
538-
const matching = overrides.find((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
538+
const pwPath = normalizePath(path.resolve(pw.uri.fsPath));
539+
const matching = overrides.find((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
539540
if (matching && matching.envManager && matching.envManager.length > 0) {
540541
return matching.envManager;
541542
}

‎src/features/projectManager.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
onDidRenameFiles,
1313
} from '../common/workspace.apis';
1414
import { PythonProjectManager, PythonProjectSettings, PythonProjectsImpl } from '../internal.api';
15+
import { normalizePath } from '../common/utils/pathUtils';
1516
import {
1617
addPythonProjectSetting,
1718
EditProjectSettings,
@@ -276,9 +277,9 @@ export class PythonProjectManagerImpl implements PythonProjectManager {
276277
private findProjectByUri(uri: Uri): PythonProject | undefined {
277278
const _projects = Array.from(this._projects.values()).sort((a, b) => b.uri.fsPath.length - a.uri.fsPath.length);
278279

279-
const normalizedUriPath = path.normalize(uri.fsPath);
280+
const normalizedUriPath = normalizePath(uri.fsPath);
280281
for (const p of _projects) {
281-
const normalizedProjectPath = path.normalize(p.uri.fsPath);
282+
const normalizedProjectPath = normalizePath(p.uri.fsPath);
282283
if (this.isUriMatching(normalizedUriPath, normalizedProjectPath)) {
283284
return p;
284285
}

‎src/features/settings/settingHelpers.ts‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { PythonProject } from '../../api';
44
import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID, SYSTEM_MANAGER_ID } from '../../common/constants';
55
import { traceError, traceInfo, traceVerbose, traceWarn } from '../../common/logging';
66
import { getGlobalPersistentState } from '../../common/persistentState';
7+
import { normalizePath } from '../../common/utils/pathUtils';
78
import { EventNames } from '../../common/telemetry/constants';
89
import { sendTelemetryEvent } from '../../common/telemetry/sender';
910
import * as workspaceApis from '../../common/workspace.apis';
@@ -20,8 +21,8 @@ function getSettings(
2021
const pw = wm.get(scope);
2122
const w = workspaceApis.getWorkspaceFolder(scope);
2223
if (pw && w) {
23-
const pwPath = path.normalize(pw.uri.fsPath);
24-
return overrides.find((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
24+
const pwPath = normalizePath(pw.uri.fsPath);
25+
return overrides.find((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
2526
}
2627
}
2728
return undefined;
@@ -140,9 +141,9 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr
140141
const originalOverridesLength = overrides.length;
141142

142143
es.forEach((e) => {
143-
const pwPath = path.normalize(e.project.uri.fsPath);
144-
const isRoot = path.normalize(w.uri.fsPath) === pwPath;
145-
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
144+
const pwPath = normalizePath(e.project.uri.fsPath);
145+
const isRoot = normalizePath(w.uri.fsPath) === pwPath;
146+
const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
146147

147148
// For workspace root in single-folder workspaces (no workspaceFile),
148149
// use default settings instead of pythonProjects entries
@@ -250,8 +251,8 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr
250251
let projectsModified = false;
251252

252253
es.forEach((e) => {
253-
const pwPath = path.normalize(e.project.uri.fsPath);
254-
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
254+
const pwPath = normalizePath(e.project.uri.fsPath);
255+
const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
255256
if (index >= 0) {
256257
overrides[index].envManager = e.envManager;
257258
projectsModified = true;
@@ -311,8 +312,8 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr
311312
let projectsModified = false;
312313

313314
es.forEach((e) => {
314-
const pwPath = path.normalize(e.project.uri.fsPath);
315-
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
315+
const pwPath = normalizePath(e.project.uri.fsPath);
316+
const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
316317
if (index >= 0) {
317318
overrides[index].packageManager = e.packageManager;
318319
projectsModified = true;
@@ -367,14 +368,16 @@ export async function addPythonProjectSetting(edits: EditProjectSettings[]): Pro
367368
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
368369
let overridesModified = false;
369370
es.forEach((e) => {
370-
const pwPath = path.normalize(e.project.uri.fsPath);
371-
const isRoot = path.normalize(w.uri.fsPath) === pwPath;
371+
const pwPath = normalizePath(e.project.uri.fsPath);
372+
const isRoot = normalizePath(w.uri.fsPath) === pwPath;
372373

373374
// For workspace root projects in single-folder workspaces, use default settings
374375
// instead of adding to pythonProjects with empty path
375376
if (isRoot && !isMultiroot) {
376377
// Remove existing entry if present (migration from buggy empty path)
377-
const existingIndex = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
378+
const existingIndex = overrides.findIndex(
379+
(s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath,
380+
);
378381
if (existingIndex >= 0) {
379382
overrides.splice(existingIndex, 1);
380383
overridesModified = true;
@@ -398,9 +401,9 @@ export async function addPythonProjectSetting(edits: EditProjectSettings[]): Pro
398401
const index = overrides.findIndex((s) => {
399402
if (s.workspace) {
400403
// If the workspace is set, check workspace and path in existing overrides
401-
return s.workspace === w.name && path.resolve(w.uri.fsPath, s.path) === pwPath;
404+
return s.workspace === w.name && normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath;
402405
}
403-
return path.resolve(w.uri.fsPath, s.path) === pwPath;
406+
return normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath;
404407
});
405408
if (index >= 0) {
406409
// Preserve existing manager settings if not explicitly provided
@@ -454,8 +457,8 @@ export async function removePythonProjectSetting(edits: EditProjectSettings[]):
454457
const config = workspaceApis.getConfiguration('python-envs', w.uri);
455458
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
456459
es.forEach((e) => {
457-
const pwPath = path.normalize(e.project.uri.fsPath);
458-
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
460+
const pwPath = normalizePath(e.project.uri.fsPath);
461+
const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath);
459462
if (index >= 0) {
460463
overrides.splice(index, 1);
461464
}
@@ -480,8 +483,8 @@ export async function updatePythonProjectSettingPath(oldUri: Uri, newUri: Uri):
480483
// Find the workspace folder that contains the old path
481484
let targetWorkspace: WorkspaceFolder | undefined;
482485
for (const w of workspaceFolders) {
483-
const oldPath = path.normalize(oldUri.fsPath);
484-
if (oldPath.startsWith(path.normalize(w.uri.fsPath))) {
486+
const oldPath = normalizePath(oldUri.fsPath);
487+
if (oldPath.startsWith(normalizePath(w.uri.fsPath))) {
485488
targetWorkspace = w;
486489
break;
487490
}
@@ -494,9 +497,11 @@ export async function updatePythonProjectSettingPath(oldUri: Uri, newUri: Uri):
494497

495498
const config = workspaceApis.getConfiguration('python-envs', targetWorkspace.uri);
496499
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
497-
const oldNormalizedPath = path.normalize(oldUri.fsPath);
500+
const oldNormalizedPath = normalizePath(oldUri.fsPath);
498501

499-
const index = overrides.findIndex((s) => path.resolve(targetWorkspace!.uri.fsPath, s.path) === oldNormalizedPath);
502+
const index = overrides.findIndex(
503+
(s) => normalizePath(path.resolve(targetWorkspace!.uri.fsPath, s.path)) === oldNormalizedPath,
504+
);
500505
if (index >= 0) {
501506
// Update the path to the new location
502507
const newRelativePath = path.relative(targetWorkspace.uri.fsPath, newUri.fsPath).replace(/\\/g, '/');

‎src/features/terminal/terminalManager.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
withProgress,
1515
} from '../../common/window.apis';
1616
import { getConfiguration, onDidChangeConfiguration } from '../../common/workspace.apis';
17+
import { normalizePath } from '../../common/utils/pathUtils';
1718
import { isActivatableEnvironment } from '../common/activation';
1819
import { identifyTerminalShell } from '../common/shellDetector';
1920
import { getPythonApi } from '../pythonApi';
@@ -323,7 +324,7 @@ export class TerminalManagerImpl implements TerminalManager {
323324
environment: PythonEnvironment,
324325
createNew: boolean = false,
325326
): Promise<Terminal> {
326-
const part = terminalKey instanceof Uri ? path.normalize(terminalKey.fsPath) : terminalKey;
327+
const part = terminalKey instanceof Uri ? normalizePath(terminalKey.fsPath) : terminalKey;
327328
const key = `${environment.envId.id}:${part}`;
328329
if (!createNew) {
329330
const terminal = this.dedicatedTerminals.get(key);
@@ -373,7 +374,7 @@ export class TerminalManagerImpl implements TerminalManager {
373374
createNew: boolean = false,
374375
): Promise<Terminal> {
375376
const uri = project instanceof Uri ? project : project.uri;
376-
const key = `${environment.envId.id}:${path.normalize(uri.fsPath)}`;
377+
const key = `${environment.envId.id}:${normalizePath(uri.fsPath)}`;
377378
if (!createNew) {
378379
const terminal = this.projectTerminals.get(key);
379380
if (terminal) {

‎src/features/views/utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import * as path from 'path';
21
import { PythonProject } from '../../api';
32
import { getWorkspaceFolder } from '../../common/workspace.apis';
3+
import { normalizePath } from '../../common/utils/pathUtils';
44

55
export function removable(project: PythonProject): boolean {
66
const workspace = getWorkspaceFolder(project.uri);
77
if (workspace) {
88
// If the project path is same as the workspace path, then we cannot remove the project.
9-
return path.normalize(workspace?.uri.fsPath).toLowerCase() !== path.normalize(project.uri.fsPath).toLowerCase();
9+
return normalizePath(workspace?.uri.fsPath) !== normalizePath(project.uri.fsPath);
1010
}
1111
return true;
1212
}

‎src/test/features/views/utils.unit.test.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/* eslint-disable @typescript-eslint/no-explicit-any */
2+
import * as assert from 'assert';
3+
import * as sinon from 'sinon';
4+
import { PythonProject } from '../../../api';
5+
import * as platformUtils from '../../../common/utils/platformUtils';
6+
import * as workspaceApis from '../../../common/workspace.apis';
7+
import { removable } from '../../../features/views/utils';
8+
9+
/**
10+
* Builds a minimal PythonProject-like object for the given filesystem path.
11+
*/
12+
function makeProject(fsPath: string): PythonProject {
13+
return { name: fsPath, uri: { fsPath } } as any;
14+
}
15+
16+
/**
17+
* Stubs getWorkspaceFolder to return a workspace folder with the given path.
18+
*/
19+
function stubWorkspaceFolder(getWsStub: sinon.SinonStub, fsPath: string | undefined): void {
20+
getWsStub.returns(fsPath === undefined ? undefined : ({ uri: { fsPath } } as any));
21+
}
22+
23+
suite('Views utils - removable', () => {
24+
let getWorkspaceFolderStub: sinon.SinonStub;
25+
let isWindowsStub: sinon.SinonStub;
26+
27+
setup(() => {
28+
getWorkspaceFolderStub = sinon.stub(workspaceApis, 'getWorkspaceFolder');
29+
isWindowsStub = sinon.stub(platformUtils, 'isWindows');
30+
});
31+
32+
teardown(() => {
33+
sinon.restore();
34+
});
35+
36+
test('returns true when the project has no workspace folder', () => {
37+
stubWorkspaceFolder(getWorkspaceFolderStub, undefined);
38+
assert.strictEqual(removable(makeProject('/home/user/project')), true);
39+
});
40+
41+
test('returns false when project path equals workspace path', () => {
42+
isWindowsStub.returns(false);
43+
stubWorkspaceFolder(getWorkspaceFolderStub, '/home/user/project');
44+
assert.strictEqual(removable(makeProject('/home/user/project')), false);
45+
});
46+
47+
test('returns true when project is nested inside the workspace folder', () => {
48+
isWindowsStub.returns(false);
49+
stubWorkspaceFolder(getWorkspaceFolderStub, '/home/user/workspace');
50+
assert.strictEqual(removable(makeProject('/home/user/workspace/project')), true);
51+
});
52+
53+
test('Windows: matches paths that differ only by drive-letter case', () => {
54+
// Regression: path.normalize() does not lowercase, so 'C:\\ws' and 'c:\\ws'
55+
// would not match and the workspace root would be wrongly reported removable.
56+
isWindowsStub.returns(true);
57+
stubWorkspaceFolder(getWorkspaceFolderStub, 'C:\\Users\\test\\project');
58+
assert.strictEqual(removable(makeProject('c:\\users\\test\\project')), false);
59+
});
60+
61+
test('Windows: matches paths that differ only by slash direction', () => {
62+
isWindowsStub.returns(true);
63+
stubWorkspaceFolder(getWorkspaceFolderStub, 'C:\\Users\\test\\project');
64+
assert.strictEqual(removable(makeProject('C:/Users/test/project')), false);
65+
});
66+
67+
test('non-Windows comparison stays case-sensitive', () => {
68+
isWindowsStub.returns(false);
69+
stubWorkspaceFolder(getWorkspaceFolderStub, '/home/user/Project');
70+
assert.strictEqual(removable(makeProject('/home/user/project')), true);
71+
});
72+
});

0 commit comments

Comments
 (0)