diff --git a/src/common/utils/pathUtils.ts b/src/common/utils/pathUtils.ts index d398828a1..d38efda7d 100644 --- a/src/common/utils/pathUtils.ts +++ b/src/common/utils/pathUtils.ts @@ -65,6 +65,20 @@ export function normalizePath(fsPath: string): string { return path1; } +/** + * Returns `true` when `candidateFsPath` is the same path as, or nested inside, `scopeFsPath` + * (inclusive of the scope itself). Both operands are resolved to absolute paths and compared with + * `path.relative`, so sibling directories sharing a name prefix (e.g. `.../app` vs `.../app-2`) are + * correctly treated as outside the scope. + */ +export function isPathInside(scopeFsPath: string, candidateFsPath: string): boolean { + const relative = path.relative(path.resolve(scopeFsPath), path.resolve(candidateFsPath)); + return ( + relative === '' || + (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)) + ); +} + export function getResourceUri(resourcePath: string, root?: string): Uri | undefined { try { if (!resourcePath) { diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 6dcda4df8..b912afb8c 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -25,7 +25,7 @@ import { PYTHON_EXTENSION_ID } from '../../common/constants'; import { VenvManagerStrings } from '../../common/localize'; import { traceError, traceWarn } from '../../common/logging'; import { createDeferred, Deferred } from '../../common/utils/deferred'; -import { normalizePath } from '../../common/utils/pathUtils'; +import { normalizePath, isPathInside } from '../../common/utils/pathUtils'; import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis'; import { findParentIfFile } from '../../features/envCommands'; import { getProjectFsPathForScope, tryFastPathGet } from '../common/fastPath'; @@ -324,12 +324,7 @@ export class VenvManager implements EnvironmentManager { title, }, async () => { - const discard = this.collection.map((env) => ({ - kind: EnvironmentChangeKind.remove, - environment: env, - })); - - this.collection = + const discovered = (await findVirtualEnvironments( hardRefresh, this.nativeFinder, @@ -338,14 +333,66 @@ export class VenvManager implements EnvironmentManager { this, scope ? [scope] : undefined, )) ?? []; - await this.loadEnvMap(); - const added = this.collection.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })); - this._onDidChangeEnvironments.fire([...discard, ...added]); + let changes: DidChangeEnvironmentsEventArgs; + if (scope) { + changes = await this.mergeScopedEnvironments(scope, discovered); + } else { + const discard = this.collection.map((env) => ({ + kind: EnvironmentChangeKind.remove, + environment: env, + })); + this.collection = discovered; + await this.loadEnvMap(); + const added = this.collection.map((env) => ({ + environment: env, + kind: EnvironmentChangeKind.add, + })); + changes = [...discard, ...added]; + } + + this._onDidChangeEnvironments.fire(changes); }, ); } + // A scoped discovery is authoritative only within `scope`: environments in other workspace + // folders (and globals outside it) are retained untouched, so they neither disappear nor emit + // events; only in-scope environments are replaced by the freshly discovered ones. + private async mergeScopedEnvironments( + scope: Uri, + discovered: PythonEnvironment[], + ): Promise { + let scopeDir: string; + try { + scopeDir = await findParentIfFile(scope.fsPath); + } catch { + scopeDir = scope.fsPath; + } + const inScope = (env: PythonEnvironment): boolean => isPathInside(scopeDir, env.environmentPath.fsPath); + + const retained: PythonEnvironment[] = []; + const removed: PythonEnvironment[] = []; + for (const env of this.collection) { + (inScope(env) ? removed : retained).push(env); + } + const retainedIds = new Set(retained.map((env) => env.envId.id)); + const retainedPaths = new Set(retained.map((env) => normalizePath(env.environmentPath.fsPath))); + + this.collection = [ + ...retained, + ...discovered.filter((env) => !retainedPaths.has(normalizePath(env.environmentPath.fsPath))), + ]; + await this.loadEnvMap(); + + return [ + ...removed.map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })), + ...this.collection + .filter((env) => !retainedIds.has(env.envId.id)) + .map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })), + ]; + } + async getEnvironments(scope: GetEnvironmentsScope): Promise { await this.initialize(); diff --git a/src/test/common/pathUtils.unit.test.ts b/src/test/common/pathUtils.unit.test.ts index 1733e789f..bbb720ed3 100644 --- a/src/test/common/pathUtils.unit.test.ts +++ b/src/test/common/pathUtils.unit.test.ts @@ -1,7 +1,8 @@ import assert from 'node:assert'; +import * as path from 'node:path'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; -import { getResourceUri, normalizePath } from '../../common/utils/pathUtils'; +import { getResourceUri, isPathInside, normalizePath } from '../../common/utils/pathUtils'; import * as utils from '../../common/utils/platformUtils'; suite('Path Utilities', () => { @@ -128,4 +129,52 @@ suite('Path Utilities', () => { assert.strictEqual(result, 'C:/Path/To/File.txt'); }); }); + + suite('isPathInside', () => { + const root = path.join(path.parse(process.cwd()).root, 'workspaces', 'app'); + + test('returns true when the candidate equals the scope (inclusive of scope.fsPath)', () => { + assert.strictEqual(isPathInside(root, root), true); + }); + + test('returns true for a direct child path', () => { + assert.strictEqual(isPathInside(root, path.join(root, '.venv')), true); + }); + + test('returns true for a deeply nested child path', () => { + assert.strictEqual(isPathInside(root, path.join(root, '.venv', 'bin', 'python')), true); + }); + + test('returns false for the parent directory', () => { + assert.strictEqual(isPathInside(root, path.dirname(root)), false); + }); + + test('returns false for a sibling directory that shares a name prefix (app vs app-2)', () => { + const sibling = path.join(path.dirname(root), 'app-2', '.venv', 'bin', 'python'); + assert.strictEqual(isPathInside(root, sibling), false); + }); + + test('returns false for an unrelated directory', () => { + const unrelated = path.join(path.dirname(root), 'other', '.venv'); + assert.strictEqual(isPathInside(root, unrelated), false); + }); + + test('resolves relative segments in the candidate before comparing', () => { + assert.strictEqual(isPathInside(root, path.join(root, 'pkg', '..', '.venv')), true); + }); + + test('returns false for a path on a different Windows drive', function () { + if (process.platform !== 'win32') { + this.skip(); + } + assert.strictEqual(isPathInside('C:\\workspaces\\app', 'D:\\workspaces\\app\\.venv'), false); + }); + + test('is case-insensitive on Windows (drive letter and folder casing)', function () { + if (process.platform !== 'win32') { + this.skip(); + } + assert.strictEqual(isPathInside('C:\\Workspaces\\App', 'c:\\workspaces\\app\\.venv\\Scripts\\python.exe'), true); + }); + }); }); diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts new file mode 100644 index 000000000..f7ea09ce4 --- /dev/null +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -0,0 +1,260 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import assert from 'assert'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { Uri } from 'vscode'; +import { + DidChangeEnvironmentsEventArgs, + EnvironmentChangeKind, + EnvironmentManager, + PythonEnvironment, + PythonEnvironmentApi, +} from '../../../api'; +import { VENV_MANAGER_ID } from '../../../common/constants'; +import * as windowApis from '../../../common/window.apis'; +import * as envCommands from '../../../features/envCommands'; +import { VenvManager } from '../../../managers/builtin/venvManager'; +import * as venvUtils from '../../../managers/builtin/venvUtils'; +import { NativePythonFinder } from '../../../managers/common/nativePythonFinder'; +import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment'; + +const ROOT = Uri.file(path.join(os.tmpdir(), 'vscode-python-envs-tests', 'venv-scoped-refresh')).fsPath; +const GLOBAL_ROOT = Uri.file(path.join(os.tmpdir(), 'vscode-python-envs-tests', 'venv-scoped-global')).fsPath; + +function venvPython(venvRoot: string): string { + return path.join(venvRoot, process.platform === 'win32' ? 'Scripts' : 'bin', 'python'); +} + +function makeEnv(id: string, venvRoot: string, version?: string): PythonEnvironment { + return createMockPythonEnvironment({ + name: path.basename(venvRoot), + envPath: venvPython(venvRoot), + sysPrefix: venvRoot, + managerId: VENV_MANAGER_ID, + id, + version, + }); +} + +function ids(collection: PythonEnvironment[]): string[] { + return collection.map((e) => e.envId.id).sort(); +} + +suite('VenvManager - scoped refresh preservation', () => { + let findVirtualEnvironmentsStub: sinon.SinonStub; + let findParentIfFileStub: sinon.SinonStub; + + const folderA = path.join(ROOT, 'app'); + const folderB = path.join(ROOT, 'app-2'); + const venvARoot = path.join(folderA, '.venv'); + const venvBRoot = path.join(folderB, '.venv'); + const globalVenvRoot = path.join(GLOBAL_ROOT, 'shared-env'); + + function createManager(): VenvManager { + const api = { + getEnvironments: sinon.stub().resolves([]), + getPythonProject: sinon.stub().returns(undefined), + getPythonProjects: sinon.stub().returns([]), + refreshEnvironments: sinon.stub().resolves(undefined), + } as any as PythonEnvironmentApi; + const baseManager = { + getEnvironments: sinon.stub().resolves([]), + } as any as EnvironmentManager; + const manager = new VenvManager({} as NativePythonFinder, api, baseManager, { + info: sinon.stub(), + error: sinon.stub(), + warn: sinon.stub(), + } as any); + (manager as any)._initialized = { promise: Promise.resolve() }; + (manager as any).collection = []; + return manager; + } + + function seed(manager: VenvManager, envs: PythonEnvironment[]): void { + (manager as any).collection = envs; + } + + function captureEvents(manager: VenvManager): DidChangeEnvironmentsEventArgs[] { + const events: DidChangeEnvironmentsEventArgs[] = []; + manager.onDidChangeEnvironments((e) => events.push(e)); + return events; + } + + function flatChanges(events: DidChangeEnvironmentsEventArgs[]): DidChangeEnvironmentsEventArgs { + return events.flat(); + } + + setup(() => { + findVirtualEnvironmentsStub = sinon.stub(venvUtils, 'findVirtualEnvironments'); + sinon.stub(venvUtils, 'getVenvForGlobal').resolves(undefined); + sinon.stub(venvUtils, 'getVenvForWorkspace').resolves(undefined); + sinon.stub(venvUtils, 'resolveVenvPythonEnvironmentPath').resolves(undefined); + findParentIfFileStub = sinon.stub(envCommands, 'findParentIfFile').callsFake(async (p: string) => p); + sinon + .stub(windowApis, 'withProgress') + .callsFake((_options: any, task: any) => + task( + { report: () => {} }, + { isCancellationRequested: false, onCancellationRequested: () => ({ dispose() {} }) }, + ), + ); + }); + + teardown(() => { + sinon.restore(); + }); + + test('retains siblings and globals while rediscovering the in-scope env', async () => { + const manager = createManager(); + const envAOld = makeEnv('A-old', venvARoot, '3.11.0'); + const envB = makeEnv('B', venvBRoot); + const envGlobal = makeEnv('G', globalVenvRoot); + seed(manager, [envAOld, envB, envGlobal]); + + const envANew = makeEnv('A-new', venvARoot, '3.12.5'); + findVirtualEnvironmentsStub.resolves([envANew]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.ok(collection.includes(envB) && collection.includes(envGlobal)); + assert.ok(collection.includes(envANew) && !collection.includes(envAOld)); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [ + { id: 'A-old', kind: EnvironmentChangeKind.remove }, + { id: 'A-new', kind: EnvironmentChangeKind.add }, + ], + ); + }); + + test('removes only stale environments inside the target scope', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot), makeEnv('G', globalVenvRoot)]); + findVirtualEnvironmentsStub.resolves([]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['B', 'G']); + const changes = flatChanges(events); + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(changes[0].environment.envId.id, 'A'); + }); + + test('adds newly discovered environments in the target scope', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + const envANew = makeEnv('A-new', venvARoot); + findVirtualEnvironmentsStub.resolves([envANew]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + const changes = flatChanges(events); + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, EnvironmentChangeKind.add); + assert.strictEqual(changes[0].environment.envId.id, 'A-new'); + }); + + test('ignores out-of-scope discovery results (configured global venvFolders) already retained', async () => { + const manager = createManager(); + const envGlobal = makeEnv('G', globalVenvRoot); + seed(manager, [envGlobal]); + + const envANew = makeEnv('A-new', venvARoot); + const envGlobalFresh = makeEnv('G-fresh', globalVenvRoot); + findVirtualEnvironmentsStub.resolves([envANew, envGlobalFresh]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.deepStrictEqual(ids(collection), ['A-new', 'G']); + assert.ok(collection.includes(envGlobal)); + assert.ok(!collection.some((e) => e.envId.id === 'G-fresh')); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [{ id: 'A-new', kind: EnvironmentChangeKind.add }], + ); + }); + + test('treats sibling directories sharing a name prefix as outside the scope (app vs app-2)', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + findVirtualEnvironmentsStub.resolves([]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['B']); + assert.strictEqual(flatChanges(events).length, 0); + }); + + test('full (unscoped) refresh still replaces the entire collection', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); + + const envX = makeEnv('X', path.join(ROOT, 'x', '.venv')); + const envY = makeEnv('Y', path.join(ROOT, 'y', '.venv')); + findVirtualEnvironmentsStub.resolves([envX, envY]); + + const events = captureEvents(manager); + await manager.refresh(undefined); + + assert.deepStrictEqual(ids((manager as any).collection), ['X', 'Y'].sort()); + + const changes = flatChanges(events); + const removed = changes.filter((c) => c.kind === EnvironmentChangeKind.remove).map((c) => c.environment.envId.id); + const added = changes.filter((c) => c.kind === EnvironmentChangeKind.add).map((c) => c.environment.envId.id); + assert.deepStrictEqual(removed.sort(), ['A', 'B']); + assert.deepStrictEqual(added.sort(), ['X', 'Y']); + }); + + test('passes the scope through to discovery as a single-element uri array', async () => { + const manager = createManager(); + seed(manager, []); + findVirtualEnvironmentsStub.resolves([]); + + const scope = Uri.file(folderA); + await manager.refresh(scope); + + const uris = findVirtualEnvironmentsStub.firstCall.args[5] as Uri[] | undefined; + assert.ok(Array.isArray(uris) && uris.length === 1); + assert.strictEqual(uris[0].fsPath, scope.fsPath); + }); + + test('resolves a file scope to its containing directory for the merge', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + const fileScope = Uri.file(path.join(folderA, 'main.py')); + findParentIfFileStub.callsFake(async () => folderA); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + await manager.refresh(fileScope); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + }); + + test('falls back to the raw scope when the scope path cannot be inspected', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + findParentIfFileStub.rejects(new Error('ENOENT: no such file or directory')); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + }); +});