diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index 1164921d3..6eeac8aa3 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -19,6 +19,7 @@ This guide covers the full testing lifecycle: ## Learnings - Pip commands that return JSON must pass `--disable-pip-version-check`; the process helper combines stderr with stdout, so update notices can otherwise make valid JSON unparseable (1). +- When a view subscribes to a newly added provider event, TypeMoq-based view tests must return a real `EventEmitter.event`; an unstubbed event yields an undefined disposable and fails during teardown (1). ### When to Use This Guide diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 35ad498f7..52f4112e3 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to the `@vscode/python-environments` API package are documen The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.5.0] + +### Added + +- Added the optional `PackageManager.createForProject` factory for package managers whose operations depend on the calling Python project. Explicit project contexts are used directly; environment-only operations use a scoped manager only when exactly one tracked project matches. +- Added optional `PackageManager.dispose` support for releasing resources owned by project-scoped package managers. +- Added `PackageManagerRequiresProjectError` and `isPackageManagerRequiresProjectError` for environment-only package mutations and refreshes that cannot identify a unique project. + +### Changed + +- Environment-only package operations no longer fall back to an unbound project-aware package manager. Package reads return `undefined`; mutations and refreshes reject with `PackageManagerRequiresProjectError`. + ## [1.4.0] ### Changed diff --git a/api/package-lock.json b/api/package-lock.json index 4c8908c6b..62fda10a8 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vscode/python-environments", - "version": "1.4.0", + "version": "1.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vscode/python-environments", - "version": "1.4.0", + "version": "1.5.0", "license": "MIT", "dependencies": { "@renovatebot/pep440": "^3.1.0" diff --git a/api/package.json b/api/package.json index 84bb1e735..ada2db299 100644 --- a/api/package.json +++ b/api/package.json @@ -1,7 +1,7 @@ { "name": "@vscode/python-environments", "description": "An API facade for the Python Environments extension in VS Code", - "version": "1.4.0", + "version": "1.5.0", "author": { "name": "Microsoft Corporation" }, diff --git a/docs/README.md b/docs/README.md index 0d4c1e862..1e528b271 100644 --- a/docs/README.md +++ b/docs/README.md @@ -793,8 +793,8 @@ getPackages( **Returns** `Promise`. `undefined` means the manager could not produce a list - for example no package manager is associated with -the environment - which is different from an empty array meaning "nothing -installed". +the environment or a project-aware manager cannot identify one unique project - +which is different from an empty array meaning "nothing installed". ```typescript const packages = await api.getPackages(env); @@ -818,7 +818,9 @@ refreshPackages(environment: PythonEnvironment): Promise; | `environment` | [`PythonEnvironment`](#pythonenvironment) | Yes | The environment whose package list should be refreshed. | **Returns** `Promise`. Changes surface through -[`onDidChangePackages`](#ondidchangepackages). +[`onDidChangePackages`](#ondidchangepackages). Rejects with +`PackageManagerRequiresProjectError` when a project-aware manager cannot identify +one unique project for the environment. ```typescript // Packages were installed outside the extension - re-read the list. @@ -842,8 +844,9 @@ managePackages( | `environment` | [`PythonEnvironment`](#pythonenvironment) | Yes | The environment to modify. | | `options` | [`PackageManagementOptions`](#packagemanagementoptions) | Yes | Must specify `install`, `uninstall`, or both. Also carries `upgrade`, `showSkipOption`, and `runHeadless`. | -**Returns** `Promise`, resolving when the operation finishes. Rejects if -the underlying tool fails. +**Returns** `Promise`, resolving when the operation finishes. Rejects with +`PackageManagerRequiresProjectError` when a project-aware manager cannot identify +one unique project for the environment, or if the underlying tool fails. ```typescript await api.managePackages(env, { @@ -973,6 +976,30 @@ context.subscriptions.push( ### Package errors +#### `PackageManagerRequiresProjectError` + +Thrown by environment-only package mutations and refreshes when the selected +package manager is project-aware but the environment does not identify exactly +one tracked project. Its `code` is the stable +`'PackageManagerRequiresProject'` discriminator. + +Use `isPackageManagerRequiresProjectError(error)` instead of `instanceof` when +the error may cross extension bundle boundaries: + +```typescript +import { isPackageManagerRequiresProjectError } from '@vscode/python-environments'; + +try { + await api.refreshPackages(env); +} catch (error) { + if (isPackageManagerRequiresProjectError(error)) { + // Ask the user to open or select the intended Python project. + } else { + throw error; + } +} +``` + #### `PackageVersionLookupNotSupportedError` Thrown when a package manager cannot list available versions at all. It @@ -1613,6 +1640,8 @@ Reports and changes the packages of an environment. | `refresh(environment)` | `(environment: PythonEnvironment) => Promise` | Yes | Re-reads the installed package list. | | `getPackages(environment, options?)` | `(environment: PythonEnvironment, options?: GetPackagesOptions) => Promise` | Yes | Returns installed packages, or `undefined` if they cannot be retrieved. | | `getPackageWatchTargets(environment)` | `(environment: PythonEnvironment) => RelativePattern[]` | No | Extra filesystem patterns to watch for install and uninstall changes, appended to the default site-packages locations. Implement for manager-specific locations such as `conda-meta`. | +| `createForProject(project)` | `(project: PythonProject) => PackageManager` | No | Creates a manager bound to a project for project-sensitive operations. | +| `dispose()` | `() => void` | No | Releases resources owned by the manager. The extension disposes project-scoped managers when their project is removed or replaced, their provider is unregistered, or the extension shuts down. | | `getDirectPackageNames(environment)` | `(environment: PythonEnvironment) => Promise \| undefined>` | No | Best-effort set of non-transitive package names. Most tools cannot record user intent - pip uses `pip list --not-required`, which reports leaf packages rather than explicitly installed ones. | | `clearCache()` | `() => Promise` | No | Drops cached package data. | | `getVersion(environment)` | `(environment: PythonEnvironment) => Promise` | No | Version of the underlying tool, such as pip, uv, or conda. | @@ -1620,6 +1649,52 @@ Reports and changes the packages of an environment. | `formatInstallSpec(packageName, version)` | `(packageName: string, version: string) => string` | No | Formats a pinned specifier for this tool, for example `requests==2.31.0` for pip or `requests=2.31.0` for conda. Callers default to `name==version` when absent. | | `onDidChangePackages` | `Event` | No | Fire when packages change. | +##### Project-scoped package managers + +Implement `createForProject` when package operations depend on project files or +the process working directory, as they do for tools such as Poetry. Callers that +already have a `PythonProject` use that project directly. For environment-only +operations, the extension selects a project-scoped manager only when exactly one +tracked project uses the environment; it does not choose arbitrarily when no +project or multiple projects match. + +Environment-only paths never use a project-aware provider as an unbound +fallback. When no unique project can be inferred, package reads return +`undefined`, package views suppress those operations, and package mutations or +refreshes reject with `PackageManagerRequiresProjectError`. Keep +project-specific caches and mutable state on the manager returned by +`createForProject`, and implement `dispose` when that manager owns resources. + +When a scoped manager fires `onDidChangePackages`, the event's `manager` must be +the exact scoped instance returned by `createForProject`. This requirement also +applies when root and scoped managers share an event emitter. + +```typescript +class ProjectPackageManager implements PackageManager { + readonly name = 'project-pm'; + + constructor(private readonly project?: PythonProject) {} + + createForProject(project: PythonProject): PackageManager { + return new ProjectPackageManager(project); + } + + dispose(): void { + // Release project-specific watchers or processes. + } + + async manage( + environment: PythonEnvironment, + options: PackageManagementOptions, + ): Promise { + if (!this.project) { + throw new Error('Package management requires a Python project.'); + } + await runPackageCommand(options, { cwd: this.project.uri.fsPath }); + } +} +``` + ```typescript class MyPackageManager implements PackageManager { readonly name = 'my-pm'; diff --git a/src/extension.ts b/src/extension.ts index 5dfa3db4c..d585e2b82 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -115,6 +115,7 @@ import { NativePythonFinder, } from './managers/common/nativePythonFinder'; import { registerPackageWatchers } from './managers/common/packageWatcher'; +import { PackageManagerRequiresProjectError } from './managers/common/errors'; import { IDisposable } from './managers/common/types'; import { registerCondaFeatures } from './managers/conda/main'; import { registerPipenvFeatures } from './managers/pipenv/main'; @@ -297,7 +298,7 @@ export async function activate(context: ExtensionContext): Promise { - const manager = envManagers.getPackageManager(environment); + const { manager } = envManagers.resolvePackageManagerForEnvironment(environment); const names = await manager?.getDirectPackageNames?.(environment); return names ? Array.from(names) : undefined; }, @@ -358,25 +359,32 @@ export async function activate(context: ExtensionContext): Promise { - await handlePackageUninstall(context, envManagers); + await handlePackageUninstall(context); }), commands.registerCommand('python-envs.managePackageVersion', async (context: unknown) => { - await managePackageVersion(context, envManagers); + await managePackageVersion(context); }), commands.registerCommand('python-envs.set', async (item) => { await setEnvironmentCommand(item, envManagers, projectManager); diff --git a/src/extensionApi.ts b/src/extensionApi.ts index 1643e40a6..abbc104ff 100644 --- a/src/extensionApi.ts +++ b/src/extensionApi.ts @@ -45,7 +45,8 @@ import { handlePythonPath } from './common/utils/pythonPath'; import type { EnvironmentManagers } from './features/envManagers'; import type { ProjectCreators } from './features/creators/projectCreators'; import type { PythonProjectManager } from './features/projectManager'; -import type { InternalEnvironmentManager } from './managers/common/registeredManagers'; +import { PackageManagerRequiresProjectError } from './managers/common/errors'; +import type { InternalEnvironmentManager, InternalPackageManager } from './managers/common/registeredManagers'; import { PythonEnvironmentImpl, PythonPackageImpl } from './managers/common/models'; import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './features/common/managerReady'; import { EnvVarManager } from './features/execution/envVariableManager'; @@ -86,6 +87,7 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { this._onDidChangePythonProjects, this._onDidChangePackages, this._onDidChangeEnvironmentVariables, + this.envManagers.onDidChangePackageProviderPackages((e) => this._onDidChangePackages.fire(e)), this.envManagers.onDidChangeActiveEnvironment((e) => { this._onDidChangeEnvironment.fire(e); const location = e.uri?.fsPath ?? 'global'; @@ -295,37 +297,36 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { } registerPackageManager(manager: PackageManager, options?: { extensionId?: string }): Disposable { - const disposables: Disposable[] = []; - disposables.push(this.envManagers.registerPackageManager(manager, options)); - if (manager.onDidChangePackages) { - disposables.push(manager.onDidChangePackages((e) => this._onDidChangePackages.fire(e))); - } - return new Disposable(() => disposables.forEach((d) => d.dispose())); + return this.envManagers.registerPackageManager(manager, options); } async managePackages(context: PythonEnvironment, options: PackageManagementOptions): Promise { await waitForEnvManagerId([context.envId.managerId]); - const manager = this.envManagers.getPackageManager(context); - if (!manager) { - return Promise.reject(new Error('No package manager found')); - } + const manager = this.requirePackageManagerForEnvironment(context); return manager.manage(context, options); } async refreshPackages(context: PythonEnvironment): Promise { await waitForEnvManagerId([context.envId.managerId]); - const manager = this.envManagers.getPackageManager(context); - if (!manager) { - return Promise.reject(new Error('No package manager found')); - } + const manager = this.requirePackageManagerForEnvironment(context); return manager.refresh(context); } async getPackages(context: PythonEnvironment, options?: GetPackagesOptions): Promise { await waitForEnvManagerId([context.envId.managerId]); - const manager = this.envManagers.getPackageManager(context); - if (!manager) { - return Promise.resolve(undefined); + const { manager } = this.envManagers.resolvePackageManagerForEnvironment(context); + return manager?.getPackages(context, options); + } + + private requirePackageManagerForEnvironment(context: PythonEnvironment): InternalPackageManager { + const resolution = this.envManagers.resolvePackageManagerForEnvironment(context); + switch (resolution.kind) { + case 'resolved': + return resolution.manager; + case 'projectRequired': + throw new PackageManagerRequiresProjectError(); + case 'notFound': + throw new Error('No package manager found'); } - return manager.getPackages(context, options); } + getPackageAvailableVersions( context: PythonEnvironment, packageName: string, diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index 7575ecf89..583770f90 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -31,6 +31,7 @@ import type { InternalEnvironmentManager, InternalPackageManager, } from '../managers/common/registeredManagers'; +import { PackageManagerRequiresProjectError } from '../managers/common/errors'; import { removePythonProjectSetting, setEnvironmentManager, @@ -137,16 +138,14 @@ export async function refreshPackagesCommand(context: unknown, managers?: Enviro if (context instanceof ProjectEnvironment) { const view = context as ProjectEnvironment; if (managers) { - const pkgManager = managers.getPackageManager(view.parent.project.uri); + const pkgManager = managers.getPackageManagerForProject(view.parent.project); if (pkgManager) { await pkgManager.refresh(view.environment); } } } else if (context instanceof PythonEnvTreeItem) { const view = context as PythonEnvTreeItem; - const envManager = - view.parent.kind === EnvTreeItemKind.environmentGroup ? view.parent.parent.manager : view.parent.manager; - const pkgManager = managers?.getPackageManager(envManager.preferredPackageManagerId); + const pkgManager = managers?.resolvePackageManagerForEnvironment(view.environment).manager; if (pkgManager) { await pkgManager.refresh(view.environment); } @@ -325,7 +324,7 @@ export async function removeEnvironmentCommand(context: unknown, managers: Envir } } -export async function handlePackageUninstall(context: unknown, em: EnvironmentManagers) { +export async function handlePackageUninstall(context: unknown) { if (context instanceof PackageTreeItem || context instanceof ProjectPackage) { if (context.pkg.isTransitive) { const confirm = await showInformationMessage( @@ -343,8 +342,15 @@ export async function handlePackageUninstall(context: unknown, em: EnvironmentMa } const moduleName = context.pkg.name; const environment = context.parent.environment; - const packageManager = em.getPackageManager(environment); - await packageManager?.manage(environment, { uninstall: [moduleName], install: [] }); + try { + await context.manager.manage(environment, { uninstall: [moduleName], install: [] }); + } catch (error) { + if (error instanceof PackageManagerRequiresProjectError) { + await showErrorMessage(error.message); + return; + } + throw error; + } return; } traceError(`Invalid context for uninstall command: ${typeof context}`); @@ -354,15 +360,11 @@ export async function handlePackageUninstall(context: unknown, em: EnvironmentMa * Manages package versions by allowing the user to select from available versions or enter a specific version. * If available versions can be fetched, a QuickPick is shown. Otherwise, an InputBox is used for free-text version entry. */ -export async function managePackageVersion(context: unknown, em: EnvironmentManagers) { +export async function managePackageVersion(context: unknown) { if (context instanceof PackageTreeItem || context instanceof ProjectPackage) { const pkg = context.pkg; const environment = context.parent.environment; - const packageManager = em.getPackageManager(environment); - - if (!packageManager) { - return; - } + const packageManager = context.manager; if (pkg.isTransitive) { const confirm = await showInformationMessage( @@ -432,10 +434,18 @@ export async function managePackageVersion(context: unknown, em: EnvironmentMana return; } - await packageManager.manage(environment, { - install: [packageManager.formatInstallSpec(pkg.name, version)], - uninstall: [], - }); + try { + await packageManager.manage(environment, { + install: [packageManager.formatInstallSpec(pkg.name, version)], + uninstall: [], + }); + } catch (error) { + if (error instanceof PackageManagerRequiresProjectError) { + await showErrorMessage(error.message); + return; + } + throw error; + } } else { traceError(`Invalid context for manage package version command: ${typeof context}`); } @@ -783,7 +793,7 @@ async function resolvePackageCommandOptions( if (e instanceof ProjectEnvironment) { const environment = e.environment; - const packageManager = em.getPackageManager(e.parent.project.uri); + const packageManager = em.getPackageManagerForProject(e.parent.project); if (packageManager) { return { environment, packageManager }; } @@ -791,9 +801,14 @@ async function resolvePackageCommandOptions( if (e instanceof PythonEnvTreeItem) { const environment = e.environment; - const packageManager = em.getPackageManager(environment); - if (packageManager) { - return { environment, packageManager }; + const resolution = em.resolvePackageManagerForEnvironment(environment); + switch (resolution.kind) { + case 'resolved': + return { environment, packageManager: resolution.manager }; + case 'projectRequired': + throw new PackageManagerRequiresProjectError(); + case 'notFound': + break; } } diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 4127aa54b..5cb9dd926 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -30,6 +30,7 @@ import { sendTelemetryEvent } from '../common/telemetry/sender'; import { getCallingExtension } from '../common/utils/frameUtils'; import { normalizePath } from '../common/utils/pathUtils'; import { InternalEnvironmentManager, InternalPackageManager } from '../managers/common/registeredManagers'; +import { ProjectScopedPackageManagerCache } from '../managers/common/projectScopedPackageManagerCache'; import type { PythonProjectManager, PythonProjectSettings } from './projectManager'; import { EditAllManagerSettings, @@ -86,6 +87,11 @@ export interface InternalDidChangeEnvironmentsEventArgs { changes: DidChangeEnvironmentsEventArgs; } +export type PackageManagerResolution = + | { kind: 'resolved'; manager: InternalPackageManager } + | { kind: 'projectRequired'; manager?: never } + | { kind: 'notFound'; manager?: never }; + export interface EnvironmentManagers extends Disposable { registerEnvironmentManager(manager: EnvironmentManager, options?: { extensionId?: string }): Disposable; registerPackageManager(manager: PackageManager, options?: { extensionId?: string }): Disposable; @@ -111,13 +117,32 @@ export interface EnvironmentManagers extends Disposable { */ onDidChangeActiveEnvironment: Event; onDidChangePackages: Event; + onDidChangePackageProviderPackages: Event; onDidChangeEnvironmentManager: Event; onDidChangePackageManager: Event; + /** Fires when cached project-scoped managers are replaced or removed. */ + onDidChangeProjectPackageManager: Event; getEnvironmentManager(scope: EnvironmentManagerScope): InternalEnvironmentManager | undefined; getPackageManager(scope: PackageManagerScope): InternalPackageManager | undefined; + /** + * Returns the configured package manager for an explicit tracked project. + * + * @param project The project whose package manager should be returned. + * @returns The shared or project-scoped package manager. + */ + getPackageManagerForProject(project: PythonProject): InternalPackageManager | undefined; + + /** + * Resolves a package manager for an environment using last-known project selections. + * + * @param environment The environment whose package manager should be resolved. + * @returns A resolved manager or the reason resolution was not possible. + */ + resolvePackageManagerForEnvironment(environment: PythonEnvironment): PackageManagerResolution; + managers: InternalEnvironmentManager[]; packageManagers: InternalPackageManager[]; @@ -171,6 +196,8 @@ function generateId(name: string, extensionId?: string): string { export class PythonEnvironmentManagers implements EnvironmentManagers { private _environmentManagers: Map = new Map(); private _packageManagers: Map = new Map(); + private readonly packageManagerEventSubscriptions = new Map(); + private readonly projectPackageManagers: ProjectScopedPackageManagerCache; private readonly subscriptions: Disposable[] = []; /** @@ -190,6 +217,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { private _onDidChangeEnvironmentManager = new EventEmitter(); private _onDidChangePackageManager = new EventEmitter(); + private _onDidChangeProjectPackageManager = new EventEmitter(); private _onDidChangeEnvironments = new EventEmitter(); /** Fires when ANY manager reports a selection change, regardless of whether that manager is selected. */ @@ -198,16 +226,20 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { /** Fires when the active (selected) environment for a scope actually changes. */ private _onDidChangeActiveEnvironment = new EventEmitter(); private _onDidChangePackages = new EventEmitter(); + private _onDidChangePackageProviderPackages = new EventEmitter(); public onDidChangeEnvironmentManager: Event = this._onDidChangeEnvironmentManager.event; public onDidChangePackageManager: Event = this._onDidChangePackageManager.event; + public onDidChangeProjectPackageManager: Event = this._onDidChangeProjectPackageManager.event; public onDidChangeEnvironments: Event = this._onDidChangeEnvironments.event; /** Fires when any registered manager reports a change — even if that manager is not the selected one. */ public onDidChangeManagerEnvironment: Event = this._onDidChangeManagerEnvironment.event; public onDidChangePackages: Event = this._onDidChangePackages.event; + public onDidChangePackageProviderPackages: Event = + this._onDidChangePackageProviderPackages.event; /** Fires only when the *selected* manager's environment for a scope actually changes. */ public onDidChangeActiveEnvironment: Event = @@ -226,6 +258,19 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { private readonly pm: PythonProjectManager, private readonly inlineScriptRouting?: InlineScriptRoutingRegistry, ) { + this.projectPackageManagers = new ProjectScopedPackageManagerCache( + (manager) => this.subscribeToPackageManagerEvents(manager), + () => this._onDidChangeProjectPackageManager.fire(), + ); + const projectChanges = this.pm.onDidChangeProjects; + if (projectChanges) { + const subscription = projectChanges((projects) => + this.projectPackageManagers.reconcileProjects(projects ?? this.pm.getProjects()), + ); + if (subscription) { + this.subscriptions.push(subscription); + } + } if (this.inlineScriptRouting) { this.subscriptions.push( this.inlineScriptRouting.onDidChangeRouteability((e) => { @@ -301,20 +346,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { traceError(ex); throw ex; } - const disposables: Disposable[] = []; const mgr = new InternalPackageManager(managerId, manager); - disposables.push( - mgr.onDidChangePackages((e: DidChangePackagesEventArgs) => { - setImmediate(() => - this._onDidChangePackages.fire({ - environment: e.environment, - manager: mgr, - changes: e.changes, - }), - ); - }), - ); + this.packageManagerEventSubscriptions.set(mgr, this.subscribeToPackageManagerEvents(mgr)); this._packageManagers.set(managerId, mgr); this._onDidChangePackageManager.fire({ kind: 'registered', manager: mgr }); @@ -327,7 +361,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { return new Disposable(() => { this._packageManagers.delete(managerId); - disposables.forEach((d) => d.dispose()); + this.projectPackageManagers.removeProvider(mgr); + this.packageManagerEventSubscriptions.get(mgr)?.dispose(); + this.packageManagerEventSubscriptions.delete(mgr); setImmediate(() => this._onDidChangePackageManager.fire({ kind: 'unregistered', manager: mgr })); }); } @@ -335,14 +371,21 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { public dispose() { this._environmentManagers.clear(); this._packageManagers.clear(); + this.projectPackageManagers.dispose(); + for (const subscription of this.packageManagerEventSubscriptions.values()) { + subscription.dispose(); + } + this.packageManagerEventSubscriptions.clear(); this._inlineRoutingOverrides.clear(); this.subscriptions.forEach((subscription) => subscription.dispose()); this._onDidChangeEnvironmentManager.dispose(); this._onDidChangePackageManager.dispose(); + this._onDidChangeProjectPackageManager.dispose(); this._onDidChangeEnvironments.dispose(); this._onDidChangeManagerEnvironment.dispose(); this._onDidChangeActiveEnvironment.dispose(); this._onDidChangePackages.dispose(); + this._onDidChangePackageProviderPackages.dispose(); } /** @@ -407,17 +450,21 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } if (context === undefined || context instanceof Uri) { + const project = context ? this.pm.get(context) : undefined; const defaultPkgManagerId = getDefaultPkgManagerSetting(this.pm, context); const defaultEnvManagerId = getDefaultEnvManagerSetting(this.pm, context); if (defaultPkgManagerId) { - return this._packageManagers.get(defaultPkgManagerId); + return project + ? this.projectPackageManagers.getOrCreate(this._packageManagers.get(defaultPkgManagerId), project) + : this._packageManagers.get(defaultPkgManagerId); } if (defaultEnvManagerId) { const preferredPkgManagerId = this._environmentManagers.get(defaultEnvManagerId)?.preferredPackageManagerId; if (preferredPkgManagerId) { - return this._packageManagers.get(preferredPkgManagerId); + const manager = this._packageManagers.get(preferredPkgManagerId); + return project ? this.projectPackageManagers.getOrCreate(manager, project) : manager; } } return undefined; @@ -439,6 +486,61 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { return undefined; } + public getPackageManagerForProject(project: PythonProject): InternalPackageManager | undefined { + const canonicalProject = this.pm.get(project.uri); + if (canonicalProject !== project) { + traceVerbose(`Unable to resolve package manager for untracked project ${project.uri.fsPath}`); + return undefined; + } + return this.getPackageManager(canonicalProject.uri); + } + + public resolvePackageManagerForEnvironment( + environment: PythonEnvironment, + ): PackageManagerResolution { + const manager = this.getPackageManager(environment); + if (!manager) { + return { kind: 'notFound' }; + } + if (!manager.createForProject) { + return { kind: 'resolved', manager }; + } + + const matchingProjects = this.pm.getProjects().filter((project) => + this.isSameEnvironment(environment, this.getLastKnownEnvironment(project.uri)), + ); + if (matchingProjects.length !== 1) { + traceVerbose( + `Unable to resolve project-scoped package manager for environment ${environment.envId.id}: ` + + `found ${matchingProjects.length} matching projects`, + ); + return { kind: 'projectRequired' }; + } + + const scopedManager = this.getPackageManagerForProject(matchingProjects[0]); + return scopedManager + ? { kind: 'resolved', manager: scopedManager } + : { kind: 'notFound' }; + } + + private subscribeToPackageManagerEvents(manager: InternalPackageManager): Disposable { + const event = manager.packageChangeEvent; + if (!event) { + return new Disposable(() => {}); + } + + return event((e) => { + this._onDidChangePackageProviderPackages.fire(e); + setImmediate(() => + this._onDidChangePackages.fire({ + environment: e.environment, + manager, + changes: e.changes, + }), + ); + }); + } + public get managers(): InternalEnvironmentManager[] { return Array.from(this._environmentManagers.values()); } diff --git a/src/features/views/envManagersView.ts b/src/features/views/envManagersView.ts index 6ce731013..3c3207c2d 100644 --- a/src/features/views/envManagersView.ts +++ b/src/features/views/envManagersView.ts @@ -10,10 +10,6 @@ import type { InternalDidChangeEnvironmentsEventArgs, InternalDidChangePackagesEventArgs, } from '../envManagers'; -import type { - InternalEnvironmentManager, - InternalPackageManager, -} from '../../managers/common/registeredManagers'; import { ITemporaryStateManager } from './temporaryStateManager'; import { EnvInfoTreeItem, @@ -119,6 +115,9 @@ export class EnvManagerView implements TreeDataProvider, Disposable this.providers.onDidChangePackageManager((p: DidChangePackageManagerEventArgs) => { this.onDidChangePackageManager(p); }), + this.providers.onDidChangeProjectPackageManager(() => { + this.fireDataChanged(undefined); + }), ); this.disposables.push( @@ -244,12 +243,7 @@ export class EnvManagerView implements TreeDataProvider, Disposable if (element.kind === EnvTreeItemKind.environment) { const pythonEnvItem = element as PythonEnvTreeItem; const environment = pythonEnvItem.environment; - const envManager = - pythonEnvItem.parent.kind === EnvTreeItemKind.environmentGroup - ? pythonEnvItem.parent.parent.manager - : pythonEnvItem.parent.manager; - - const pkgManager = this.getSupportedPackageManager(envManager); + const { manager: pkgManager } = this.providers.resolvePackageManagerForEnvironment(environment); const parent = element as PythonEnvTreeItem; const views: EnvTreeItem[] = []; @@ -321,10 +315,6 @@ export class EnvManagerView implements TreeDataProvider, Disposable } } - private getSupportedPackageManager(manager: InternalEnvironmentManager): InternalPackageManager | undefined { - return this.providers.getPackageManager(manager.preferredPackageManagerId); - } - private onDidChangeEnvironmentManager(_args: DidChangeEnvironmentManagerEventArgs) { this.fireDataChanged(undefined); } diff --git a/src/features/views/projectView.ts b/src/features/views/projectView.ts index 4a2ce9aaf..78c0e35b2 100644 --- a/src/features/views/projectView.ts +++ b/src/features/views/projectView.ts @@ -66,6 +66,9 @@ export class ProjectView implements TreeDataProvider { this.envManagers.onDidChangeEnvironments(() => { this.debouncedUpdateProject.trigger(); }), + this.envManagers.onDidChangeProjectPackageManager(() => { + this.debouncedUpdateProject.trigger(); + }), this.envManagers.onDidChangePackages((e) => { this.updatePackagesForEnvironment(e.environment); }), @@ -237,9 +240,12 @@ export class ProjectView implements TreeDataProvider { const environmentItem = element as ProjectEnvironment; const parent = environmentItem.parent; - const uri = parent.id === 'global' ? undefined : parent.project.uri; - const pkgManager = this.envManagers.getPackageManager(uri); + const project = parent.id === 'global' ? undefined : parent.project; + const uri = project?.uri; const environment = environmentItem.environment; + const pkgManager = project + ? this.envManagers.getPackageManagerForProject(project) + : this.envManagers.resolvePackageManagerForEnvironment(environment).manager; if (!pkgManager) { return [new ProjectEnvironmentInfo(environmentItem, ProjectViews.noPackageManager)]; diff --git a/src/managers/common/errors.ts b/src/managers/common/errors.ts new file mode 100644 index 000000000..eacb7e31c --- /dev/null +++ b/src/managers/common/errors.ts @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { l10n } from 'vscode'; +import { PackageManagerRequiresProjectError as PublicPackageManagerRequiresProjectError } from '../../publicErrors'; + +export class PackageManagerRequiresProjectError extends PublicPackageManagerRequiresProjectError { + constructor() { + super(l10n.t('Package operations require a Python project.')); + } +} diff --git a/src/managers/common/packageWatcher.ts b/src/managers/common/packageWatcher.ts index f6a3e8778..720a59cfd 100644 --- a/src/managers/common/packageWatcher.ts +++ b/src/managers/common/packageWatcher.ts @@ -45,12 +45,14 @@ function getDefaultPackageWatchTargets(env: PythonEnvironment): RelativePattern[ * @param env - The Python environment to watch. * @param packageManager - The package manager to call refresh on when changes occur. * @param log - Logger for diagnostic messages. + * @param resolvePackageManager - Resolves the current package manager before each refresh. * @returns A disposable that removes the watcher when disposed. */ export function watchPackageChangesForEnvironment( env: PythonEnvironment, packageManager: PackageManager, log: LogOutputChannel, + resolvePackageManager: () => PackageManager | undefined = () => packageManager, ): Disposable { const watchTargets = [ ...getDefaultPackageWatchTargets(env), @@ -63,7 +65,12 @@ export function watchPackageChangesForEnvironment( const debouncedRefresh = createSimpleDebounce(500, () => { log.debug(`Package change detected for environment ${env.envId.id}, refreshing packages.`); - void packageManager.refresh(env).catch((ex) => { + const currentPackageManager = resolvePackageManager(); + if (!currentPackageManager) { + log.debug(`No current package manager found for environment ${env.envId.id}`); + return; + } + void currentPackageManager.refresh(env).catch((ex) => { log.error( `Failed to refresh packages for environment ${env.envId.id}: ${ex instanceof Error ? ex.message : String(ex)}`, ); @@ -153,7 +160,9 @@ export function registerPackageWatchers( return; } - const watcherKey = `${environment.envId.managerId}:${environment.envId.id}:${selectedPackageManager.id}`; + const packageManagerKey = + `${selectedPackageManager.id}:${selectedPackageManager.project?.uri.toString() ?? ''}`; + const watcherKey = `${environment.envId.managerId}:${environment.envId.id}:${packageManagerKey}`; if (activeWatcherByConsumer.get(consumer) === watcherKey) { return; } @@ -164,8 +173,24 @@ export function registerPackageWatchers( if (sharedWatcher) { sharedWatcher.references += 1; } else { + const resolvePackageManager = () => { + const currentPackageManager = + envManagers.getPackageManager(packageManagerContext) ?? envManagers.getPackageManager(environment); + if ( + selectedPackageManager.project && + currentPackageManager?.project?.uri.toString() !== selectedPackageManager.project.uri.toString() + ) { + return undefined; + } + return currentPackageManager; + }; sharedWatchers.set(watcherKey, { - disposable: watchPackageChangesForEnvironment(environment, selectedPackageManager, log), + disposable: watchPackageChangesForEnvironment( + environment, + selectedPackageManager, + log, + resolvePackageManager, + ), references: 1, }); } @@ -186,7 +211,22 @@ export function registerPackageWatchers( const terminalActivationDisposable = terminalActivation.onDidChangeTerminalActivationState((changes) => { if (changes.activated) { if (!closedTerminals.has(changes.terminal)) { - watchEnvironment(changes.terminal, changes.environment, changes.environment); + const projectScope = Array.from(activeEnvironmentByScope.values()).find( + ({ scope, environment }) => + scope && + environment.envId.id === changes.environment.envId.id && + environment.envId.managerId === changes.environment.envId.managerId, + )?.scope; + const managerContext = projectScope ?? changes.environment; + const packageManager = envManagers.getPackageManager(managerContext); + if (!projectScope && packageManager?.createForProject) { + releaseConsumer(changes.terminal); + log.debug( + `Skipping unscoped package watcher for project-aware manager ${packageManager.id}`, + ); + return; + } + watchEnvironment(changes.terminal, managerContext, changes.environment); } } else { releaseConsumer(changes.terminal); diff --git a/src/managers/common/projectScopedPackageManagerCache.ts b/src/managers/common/projectScopedPackageManagerCache.ts new file mode 100644 index 000000000..43d180ae0 --- /dev/null +++ b/src/managers/common/projectScopedPackageManagerCache.ts @@ -0,0 +1,123 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Disposable } from 'vscode'; +import { PythonProject } from '../../api'; +import { InternalPackageManager } from './registeredManagers'; + +interface ProjectScopedPackageManagerEntry { + provider: InternalPackageManager; + manager: InternalPackageManager; + subscription: Disposable; +} + +/** + * Owns the package manager scoped to each tracked Python project. + * + * A project has at most one active scoped manager. Changing its configured provider replaces and + * disposes the previous manager. Project removal, provider removal, and cache disposal also release + * the scoped manager and its event subscription. + */ +export class ProjectScopedPackageManagerCache implements Disposable { + private readonly entries = new Map(); + + constructor( + private readonly subscribe: (manager: InternalPackageManager) => Disposable, + private readonly onDidInvalidate: () => void, + ) {} + + /** + * Returns the manager for a project, creating and caching a scoped manager when supported. + * + * @param provider The project's currently configured root package manager. + * @param project The canonical tracked project. + * @returns The scoped manager, the shared provider, or undefined when no provider is configured. + */ + getOrCreate( + provider: InternalPackageManager | undefined, + project: PythonProject, + ): InternalPackageManager | undefined { + const existing = this.entries.get(project); + if (!provider?.createForProject) { + if (existing) { + this.entries.delete(project); + this.disposeEntry(existing); + this.onDidInvalidate(); + } + return provider; + } + if (existing?.provider === provider) { + return existing.manager; + } + + const manager = provider.createForProject(project); + let subscription: Disposable; + try { + subscription = this.subscribe(manager); + } catch (error) { + manager.dispose(); + throw error; + } + + if (existing) { + this.disposeEntry(existing); + } + this.entries.set(project, { provider, manager, subscription }); + if (existing) { + this.onDidInvalidate(); + } + return manager; + } + + /** + * Disposes entries whose canonical projects are no longer tracked. + * + * @param projects The complete current set of tracked projects. + */ + reconcileProjects(projects: readonly PythonProject[]): void { + const activeProjects = new Set(projects); + let invalidated = false; + for (const [project, entry] of this.entries) { + if (!activeProjects.has(project)) { + this.entries.delete(project); + this.disposeEntry(entry); + invalidated = true; + } + } + if (invalidated) { + this.onDidInvalidate(); + } + } + + /** + * Disposes all scoped managers created from a provider. + * + * @param provider The provider being unregistered. + */ + removeProvider(provider: InternalPackageManager): void { + let invalidated = false; + for (const [project, entry] of this.entries) { + if (entry.provider === provider) { + this.entries.delete(project); + this.disposeEntry(entry); + invalidated = true; + } + } + if (invalidated) { + this.onDidInvalidate(); + } + } + + /** Disposes every cached scoped manager and its event subscription. */ + dispose(): void { + for (const entry of this.entries.values()) { + this.disposeEntry(entry); + } + this.entries.clear(); + } + + private disposeEntry(entry: ProjectScopedPackageManagerEntry): void { + entry.subscription.dispose(); + entry.manager.dispose(); + } +} diff --git a/src/managers/common/registeredManagers.ts b/src/managers/common/registeredManagers.ts index 5daaff64d..4c929eaa2 100644 --- a/src/managers/common/registeredManagers.ts +++ b/src/managers/common/registeredManagers.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import type { Pep440Version } from '@renovatebot/pep440'; -import { CancellationError, Disposable, LogOutputChannel, MarkdownString, RelativePattern } from 'vscode'; +import { CancellationError, Disposable, Event, LogOutputChannel, MarkdownString, RelativePattern } from 'vscode'; import { PackageVersionLookupNotSupportedError } from '../../publicErrors'; import { ISSUES_URL } from '../../common/constants'; import { CreateEnvironmentNotSupported, RemoveEnvironmentNotSupported } from '../../common/errors/NotSupportedError'; @@ -27,6 +27,7 @@ import type { PackageManagementOptions, PackageManager, PythonEnvironment, + PythonProject, QuickCreateConfig, RefreshEnvironmentsScope, RemoveEnvironmentOptions, @@ -210,10 +211,45 @@ function inferPackageManagementTrigger( } export class InternalPackageManager implements PackageManager { + private readonly relatedManagers: WeakSet; + private readonly packageChangeEventValue: Event | undefined; + private isDisposed = false; + public readonly createForProject?: (project: PythonProject) => InternalPackageManager; + public constructor( public readonly id: string, private readonly manager: PackageManager, - ) {} + public readonly project?: PythonProject, + relatedManagers?: WeakSet, + ) { + this.relatedManagers = relatedManagers ?? new WeakSet(); + this.relatedManagers.add(manager); + const packageChangeEvent = manager.onDidChangePackages; + if (packageChangeEvent) { + this.packageChangeEventValue = (listener) => + packageChangeEvent((event) => { + if (event.manager === manager) { + listener(event); + } + }); + } + const createForProject = manager.createForProject?.bind(manager); + if (createForProject) { + this.createForProject = (scopedProject) => { + this.throwIfDisposed(); + const scopedManager = createForProject(scopedProject); + if (!scopedManager) { + throw new Error(`Package manager ${this.id} did not create a manager for the requested project`); + } + return new InternalPackageManager( + this.id, + scopedManager, + scopedProject, + this.relatedManagers, + ); + }; + } + } public get name(): string { return this.manager.name; @@ -235,6 +271,7 @@ export class InternalPackageManager implements PackageManager { } async manage(environment: PythonEnvironment, options: PackageManagementOptions): Promise { + this.throwIfDisposed(); const stopWatch = new StopWatch(); const triggerSource = inferPackageManagementTrigger(options); try { @@ -264,26 +301,45 @@ export class InternalPackageManager implements PackageManager { } refresh(environment: PythonEnvironment): Promise { + this.throwIfDisposed(); return this.manager.refresh(environment); } getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + this.throwIfDisposed(); return this.manager.getPackages(environment, options); } getPackageWatchTargets(environment: PythonEnvironment): RelativePattern[] { + this.throwIfDisposed(); return this.manager.getPackageWatchTargets?.(environment) ?? []; } onDidChangePackages(handler: (e: DidChangePackagesEventArgs) => void): Disposable { - return this.manager.onDidChangePackages ? this.manager.onDidChangePackages(handler) : new Disposable(() => {}); + return this.packageChangeEventValue ? this.packageChangeEventValue(handler) : new Disposable(() => {}); } - equals(other: PackageManager): boolean { + get packageChangeEvent(): Event | undefined { + return this.packageChangeEventValue; + } + + wraps(other: PackageManager): boolean { return this.manager === other; } + equals(other: PackageManager): boolean { + return this.relatedManagers.has(other); + } + + dispose(): void { + if (!this.isDisposed) { + this.isDisposed = true; + this.manager.dispose?.(); + } + } + getVersion(environment: PythonEnvironment): Promise { + this.throwIfDisposed(); return this.manager.getVersion ? this.manager.getVersion(environment) : Promise.resolve(undefined); } @@ -306,6 +362,7 @@ export class InternalPackageManager implements PackageManager { packageName: string, options?: GetPackageAvailableVersionsOptions, ): Promise { + this.throwIfDisposed(); const shouldThrow = options?.errorMode === 'throw'; try { if (!this.manager.getPackageAvailableVersions) { @@ -329,14 +386,22 @@ export class InternalPackageManager implements PackageManager { } getDirectPackageNames(environment: PythonEnvironment): Promise | undefined> { + this.throwIfDisposed(); return this.manager.getDirectPackageNames ? this.manager.getDirectPackageNames(environment) : Promise.resolve(undefined); } formatInstallSpec(packageName: string, version: string): string { + this.throwIfDisposed(); return this.manager.formatInstallSpec ? this.manager.formatInstallSpec(packageName, version) : `${packageName}==${version}`; } + + private throwIfDisposed(): void { + if (this.isDisposed) { + throw new Error(`Package manager ${this.id} has been disposed`); + } + } } diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 430c8ac6e..a62b30c92 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -1,11 +1,11 @@ import type { Pep440Version } from '@renovatebot/pep440'; -import * as fsapi from 'fs-extra'; import * as path from 'path'; import { CancellationError, CancellationToken, Event, EventEmitter, + FileType, l10n, LogOutputChannel, MarkdownString, @@ -23,8 +23,11 @@ import { PackageVersionLookupNotSupportedError, PythonEnvironment, PythonEnvironmentApi, + PythonProject, } from '../../api'; import { showErrorMessage, showInputBox, withProgress } from '../../common/window.apis'; +import * as workspaceFs from '../../common/workspace.fs.apis'; +import { PackageManagerRequiresProjectError } from '../common/errors'; import { updatePackagesAndNotify } from '../common/packageChanges'; import { parsePackageSpecs } from '../common/packageUtils'; import { @@ -38,15 +41,16 @@ import { PoetryManager } from './poetryManager'; import { getPoetry } from './poetryUtils'; export class PoetryPackageManager implements PackageManager, Disposable { - private readonly _onDidChangePackages = new EventEmitter(); - onDidChangePackages: Event = this._onDidChangePackages.event; + private readonly packagesChangedEmitter = new EventEmitter(); + readonly onDidChangePackages: Event = this.packagesChangedEmitter.event; private packages: Map = new Map(); constructor( private readonly api: PythonEnvironmentApi, public readonly log: LogOutputChannel, - _poetry: PoetryManager, + private readonly poetryManager: PoetryManager, + private readonly project?: PythonProject, ) { this.name = 'poetry'; this.displayName = 'Poetry'; @@ -60,7 +64,23 @@ export class PoetryPackageManager implements PackageManager, Disposable { readonly tooltip?: string | MarkdownString; readonly iconPath?: IconPath; + /** + * Creates a Poetry package manager bound to a Python project. + * + * @param project The project whose working directory Poetry commands should use. + * @returns A Poetry package manager scoped to the project. + */ + createForProject(project: PythonProject): PoetryPackageManager { + return new PoetryPackageManager( + this.api, + this.log, + this.poetryManager, + project, + ); + } + async manage(environment: PythonEnvironment, options: PackageManagementOptions): Promise { + const cwd = await this.getProjectCwd(); let toInstall: string[] = [...(options.install ?? [])]; let toUninstall: string[] = [...(options.uninstall ?? [])]; @@ -89,13 +109,13 @@ export class PoetryPackageManager implements PackageManager, Disposable { const execute = async (token?: CancellationToken): Promise => { try { - await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, token); + await this.runPoetryManage({ install: toInstall, uninstall: toUninstall }, cwd, token); await updatePackagesAndNotify( this, environment, this.packages.get(environment.envId.id), (changes) => { - this._onDidChangePackages.fire({ environment, manager: this, changes }); + this.packagesChangedEmitter.fire({ environment, manager: this, changes }); }, ); } catch (e) { @@ -131,6 +151,9 @@ export class PoetryPackageManager implements PackageManager, Disposable { } async refresh(environment: PythonEnvironment): Promise { + if (!this.project) { + throw new PackageManagerRequiresProjectError(); + } await withProgress( { location: ProgressLocation.Window, @@ -143,7 +166,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { environment, this.packages.get(environment.envId.id), (changes) => { - this._onDidChangePackages.fire({ environment, manager: this, changes }); + this.packagesChangedEmitter.fire({ environment, manager: this, changes }); }, ); this.packages.set(environment.envId.id, packages ?? []); @@ -162,6 +185,9 @@ export class PoetryPackageManager implements PackageManager, Disposable { } async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { + if (!this.project) { + return undefined; + } if (options?.skipCache || !this.packages.has(environment.envId.id)) { const packages = await this.fetchPackagesFromTool(environment); this.packages.set(environment.envId.id, packages); @@ -197,12 +223,13 @@ export class PoetryPackageManager implements PackageManager, Disposable { } dispose(): void { - this._onDidChangePackages.dispose(); + this.packagesChangedEmitter.dispose(); this.packages.clear(); } private async runPoetryManage( options: { install?: string[]; uninstall?: string[] }, + cwd: string, token?: CancellationToken, ): Promise { const poetry = await getPoetry(); @@ -217,6 +244,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { if (options.uninstall && options.uninstall.length > 0) { const removeCmd = new PoetryRemoveCommand({ pythonExecutable: poetry, + cwd, log: this.log, }); const packages = parsePackageSpecs(options.uninstall); @@ -227,6 +255,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { if (options.install && options.install.length > 0) { const addCmd = new PoetryAddCommand({ pythonExecutable: poetry, + cwd, log: this.log, }); const packages = parsePackageSpecs(options.install); @@ -244,7 +273,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { ); } - const cwd = await this.getPoetryCwd(environment); + const cwd = await this.getProjectCwd(); const showCmd = new PoetryShowCommand({ pythonExecutable: poetry, cwd, @@ -260,6 +289,9 @@ export class PoetryPackageManager implements PackageManager, Disposable { } async getDirectPackageNames(_environment: PythonEnvironment): Promise | undefined> { + if (!this.project) { + return undefined; + } try { const poetry = await getPoetry(); if (!poetry) { @@ -267,6 +299,7 @@ export class PoetryPackageManager implements PackageManager, Disposable { } const showTopLevelCmd = new PoetryShowTopLevelCommand({ pythonExecutable: poetry, + cwd: await this.getProjectCwd(), log: this.log, }); return await showTopLevelCmd.execute(); @@ -276,35 +309,20 @@ export class PoetryPackageManager implements PackageManager, Disposable { } } - private async getPoetryCwd(environment: PythonEnvironment): Promise { - const projects = this.api.getPythonProjects(); - if (projects.length === 0) { - return undefined; + private async getProjectCwd(): Promise { + if (!this.project) { + throw new PackageManagerRequiresProjectError(); } - const toDirectory = async (fsPath: string): Promise => { - try { - const stat = await fsapi.stat(fsPath); - return stat.isDirectory() ? fsPath : path.dirname(fsPath); - } catch { - return path.dirname(fsPath); - } - }; - - if (projects.length === 1) { - return toDirectory(projects[0].uri.fsPath); + try { + const stat = await workspaceFs.stat(this.project.uri); + return (stat.type & FileType.Directory) === FileType.Directory + ? this.project.uri.fsPath + : path.dirname(this.project.uri.fsPath); + } catch (error) { + const message = l10n.t('Unable to access the Python project at "{0}".', this.project.uri.fsPath); + this.log.error(message, error); + throw new Error(message); } - - const matchingDirectories = new Set(); - await Promise.all( - projects.map(async (project) => { - const projectEnvironment = await this.api.getEnvironment(project.uri); - if (projectEnvironment?.envId.id === environment.envId.id) { - matchingDirectories.add(await toDirectory(project.uri.fsPath)); - } - }), - ); - - return Array.from(matchingDirectories).sort((a, b) => b.length - a.length)[0]; } } diff --git a/src/publicErrors.ts b/src/publicErrors.ts index f4799d8ce..efc68519f 100644 --- a/src/publicErrors.ts +++ b/src/publicErrors.ts @@ -7,6 +7,48 @@ * that facade small. */ +/** + * Error thrown when a project-aware package manager cannot determine which Python project to use. + * + * The {@link code} property is a stable discriminator that can be checked across extension bundle + * boundaries with {@link isPackageManagerRequiresProjectError}. + */ +export class PackageManagerRequiresProjectError extends Error { + /** + * Stable discriminator identifying this error type across bundle boundaries. + */ + public readonly code = 'PackageManagerRequiresProject'; + + /** + * Creates a project-required package error. + * + * @param message Optional caller-facing explanation. + */ + constructor(message?: string) { + super(message ?? 'Package operations require a Python project.'); + this.name = 'PackageManagerRequiresProjectError'; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +/** + * Reports whether an error means that a package operation requires an unambiguous Python project. + * + * @param error The value to test. + * @returns `true` when the error carries the project-required discriminator. + */ +export function isPackageManagerRequiresProjectError( + error: unknown, +): error is PackageManagerRequiresProjectError { + return ( + error instanceof PackageManagerRequiresProjectError || + (typeof error === 'object' && + error !== null && + 'code' in error && + (error as { code?: unknown }).code === 'PackageManagerRequiresProject') + ); +} + /** * Error thrown when a package manager cannot list available package versions. * diff --git a/src/test/extensionApi.unit.test.ts b/src/test/extensionApi.unit.test.ts index d49bc79c3..12aa2802f 100644 --- a/src/test/extensionApi.unit.test.ts +++ b/src/test/extensionApi.unit.test.ts @@ -1,10 +1,16 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { EventEmitter, Uri } from 'vscode'; -import { PythonEnvironment, PythonProject } from '../api'; +import { + isPackageManagerRequiresProjectError, + PythonEnvironment, + PythonProject, +} from '../api'; import * as managerReady from '../features/common/managerReady'; import { PythonEnvironmentApiImpl } from '../extensionApi'; +import type { PackageManagerResolution } from '../features/envManagers'; import type { PythonProjectManager } from '../features/projectManager'; +import type { InternalPackageManager } from '../managers/common/registeredManagers'; suite('PythonEnvironmentApiImpl - onDidChangePythonProjects', () => { test('fires event with correct added and removed projects', () => { @@ -16,7 +22,10 @@ suite('PythonEnvironmentApiImpl - onDidChangePythonProjects', () => { } as unknown as PythonProjectManager; type ApiArgs = ConstructorParameters; - const mockEnvManagers = { onDidChangeActiveEnvironment: new EventEmitter().event } as unknown as ApiArgs[0]; + const mockEnvManagers = { + onDidChangeActiveEnvironment: new EventEmitter().event, + onDidChangePackageProviderPackages: new EventEmitter().event, + } as unknown as ApiArgs[0]; const mockProjectCreators = {} as unknown as ApiArgs[2]; const mockTerminalManager = {} as unknown as ApiArgs[3]; const mockEnvVarManager = { onDidChangeEnvironmentVariables: new EventEmitter().event } as unknown as ApiArgs[4]; @@ -91,6 +100,7 @@ suite('PythonEnvironmentApiImpl - getEnvironment timeout fallback', () => { type ApiArgs = ConstructorParameters; const mockEnvManagers = { onDidChangeActiveEnvironment: new EventEmitter().event, + onDidChangePackageProviderPackages: new EventEmitter().event, getEnvironment: sinon.stub().returns( new Promise((resolve) => { resolveEnvironment = resolve; @@ -134,6 +144,7 @@ suite('PythonEnvironmentApiImpl - getEnvironment timeout fallback', () => { type ApiArgs = ConstructorParameters; const mockEnvManagers = { onDidChangeActiveEnvironment: new EventEmitter().event, + onDidChangePackageProviderPackages: new EventEmitter().event, getEnvironment: sinon.stub().returns( new Promise((resolve) => { resolveEnvironment = resolve; @@ -160,3 +171,77 @@ suite('PythonEnvironmentApiImpl - getEnvironment timeout fallback', () => { assert.strictEqual(await pending, undefined); }); }); + +suite('PythonEnvironmentApiImpl - project-aware package resolution', () => { + setup(() => { + sinon.stub(managerReady, 'waitForEnvManagerId').resolves(); + }); + + teardown(() => { + sinon.restore(); + }); + + function createApi(resolution: PackageManagerResolution): { + api: PythonEnvironmentApiImpl; + resolvePackageManager: sinon.SinonStub; + } { + type ApiArgs = ConstructorParameters; + const resolvePackageManager = sinon.stub().returns(resolution); + const envManagers = { + onDidChangeActiveEnvironment: new EventEmitter().event, + onDidChangePackageProviderPackages: new EventEmitter().event, + resolvePackageManagerForEnvironment: resolvePackageManager, + } as unknown as ApiArgs[0]; + const projectManager = { + getProjects: () => [], + onDidChangeProjects: new EventEmitter().event, + } as unknown as ApiArgs[1]; + return { + api: new PythonEnvironmentApiImpl( + envManagers, + projectManager, + {} as ApiArgs[2], + {} as ApiArgs[3], + { onDidChangeEnvironmentVariables: new EventEmitter().event } as unknown as ApiArgs[4], + ), + resolvePackageManager, + }; + } + + const environment = { + envId: { id: 'environment', managerId: 'environment-manager' }, + } as PythonEnvironment; + + test('rejects mutations and refreshes when a project-aware manager is unresolved', async () => { + const { api } = createApi({ kind: 'projectRequired' }); + + await assert.rejects( + api.managePackages(environment, { install: ['example'] }), + isPackageManagerRequiresProjectError, + ); + await assert.rejects(api.refreshPackages(environment), isPackageManagerRequiresProjectError); + }); + + test('returns undefined for reads when a project-aware manager is unresolved', async () => { + const { api } = createApi({ kind: 'projectRequired' }); + + assert.strictEqual(await api.getPackages(environment), undefined); + }); + + test('uses the resolved manager without a second provider lookup', async () => { + const manage = sinon.stub().resolves(); + const manager = { manage } as unknown as InternalPackageManager; + const { api, resolvePackageManager } = createApi({ kind: 'resolved', manager }); + + await api.managePackages(environment, { install: ['example'] }); + + assert.ok(resolvePackageManager.calledOnceWithExactly(environment)); + assert.ok(manage.calledOnceWithExactly(environment, { install: ['example'] })); + }); + + test('preserves the no-package-manager failure', async () => { + const { api } = createApi({ kind: 'notFound' }); + + await assert.rejects(api.refreshPackages(environment), /No package manager found/); + }); +}); diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index 40618b996..63b96979f 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -14,6 +14,7 @@ import { clearEnvironmentCachesCommand, clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, + handlePackageUninstall, removeEnvironmentCommand, removePythonProject, revealEnvInManagerView, @@ -26,10 +27,11 @@ import * as shellProviders from '../../features/terminal/shells/providers'; import { ShellStartupScriptProvider } from '../../features/terminal/shells/startupProvider'; import { TerminalManager } from '../../features/terminal/terminalManager'; import { EnvManagerView } from '../../features/views/envManagersView'; -import { ProjectEnvironment, ProjectItem } from '../../features/views/treeViewItems'; +import { EnvManagerTreeItem, PackageTreeItem, ProjectEnvironment, ProjectItem, PythonEnvTreeItem } from '../../features/views/treeViewItems'; import type { EnvironmentManagers } from '../../features/envManagers'; import type { PythonProjectManager } from '../../features/projectManager'; -import { InternalEnvironmentManager } from '../../managers/common/registeredManagers'; +import { InternalEnvironmentManager, InternalPackageManager } from '../../managers/common/registeredManagers'; +import { PackageManagerRequiresProjectError } from '../../managers/common/errors'; import { setupNonThenable } from '../mocks/helper'; import { createMockPythonEnvironment } from '../mocks/pythonEnvironment'; @@ -640,3 +642,48 @@ suite('Run In Terminal Command Tests', () => { sinon.assert.notCalled(runInTerminalStub); }); }); + +suite('handlePackageUninstall - unbound package manager', () => { + let showError: sinon.SinonStub; + + setup(() => { + showError = sinon.stub(windowApis, 'showErrorMessage').resolves(undefined); + }); + + teardown(() => sinon.restore()); + + test('shows a friendly message instead of throwing when the resolved manager requires a project', async () => { + const environment = createMockPythonEnvironment({ + envPath: path.join(process.cwd(), 'unbound-poetry-env'), + managerId: 'ms-python.python:poetry', + }); + const rawManager = { + name: 'poetry', + manage: sinon.stub().rejects(new PackageManagerRequiresProjectError()), + refresh: async () => undefined, + getPackages: async () => undefined, + }; + const packageManager = new InternalPackageManager('ms-python.python:poetry', rawManager as never); + const provider = { + name: 'poetry', + preferredPackageManagerId: 'ms-python.python:poetry', + get: async () => environment, + set: async () => undefined, + getEnvironments: async () => [environment], + refresh: async () => undefined, + resolve: async () => undefined, + }; + const parent = new EnvManagerTreeItem(new InternalEnvironmentManager('ms-python.python:poetry', provider)); + const envItem = new PythonEnvTreeItem(environment, parent); + const pkg = { + name: 'requests', + displayName: 'requests', + pkgId: { id: 'requests', managerId: 'ms-python.python:poetry', environmentId: environment.envId.id }, + }; + const context = new PackageTreeItem(pkg, envItem, packageManager); + + await handlePackageUninstall(context); + + assert.ok(showError.calledOnceWithExactly(new PackageManagerRequiresProjectError().message)); + }); +}); diff --git a/src/test/features/envManagers.packageEvents.unit.test.ts b/src/test/features/envManagers.packageEvents.unit.test.ts index 7694a6da2..073a60ebf 100644 --- a/src/test/features/envManagers.packageEvents.unit.test.ts +++ b/src/test/features/envManagers.packageEvents.unit.test.ts @@ -4,7 +4,7 @@ import assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; -import { Disposable, EventEmitter } from 'vscode'; +import { Disposable, Event, EventEmitter, Uri, WorkspaceConfiguration } from 'vscode'; import { DidChangeEnvironmentVariablesEventArgs, DidChangePackagesEventArgs, @@ -15,6 +15,7 @@ import { import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; import * as telemetry from '../../common/telemetry/sender'; import * as frameUtils from '../../common/utils/frameUtils'; +import * as workspaceApis from '../../common/workspace.apis'; import { PythonEnvironmentManagers } from '../../features/envManagers'; import { PythonEnvironmentApiImpl } from '../../extensionApi'; import type { InternalDidChangePackagesEventArgs } from '../../features/envManagers'; @@ -31,7 +32,11 @@ for (const inlineEnabled of [false, true]) { let managers: PythonEnvironmentManagers; let api: PythonEnvironmentApiImpl; let provider: PackageManager; + let scopedProvider: PackageManager | undefined; let emitter: EventEmitter; + let scopedEmitter: EventEmitter; + let scopedEvent: Event; + let project: PythonProject; let routing: InlineScriptRoutingRegistry | undefined; let disposables: Disposable[]; let internalEvents: InternalDidChangePackagesEventArgs[]; @@ -45,10 +50,12 @@ for (const inlineEnabled of [false, true]) { disposables = []; internalEvents = []; publicEvents = []; + project = { name: 'project', uri: Uri.file(path.join(process.cwd(), 'project')) }; const projectEvents = new EventEmitter(); const variableEvents = new EventEmitter(); const projectManager: Partial = { getProjects: () => [], + get: () => project, onDidChangeProjects: projectEvents.event, }; routing = inlineEnabled ? new InlineScriptRoutingRegistry() : undefined; @@ -60,15 +67,31 @@ for (const inlineEnabled of [false, true]) { {} as ApiArgs[2], {} as ApiArgs[3], variables as ApiArgs[4], disposables, ); emitter = new EventEmitter(); + scopedEmitter = new EventEmitter(); + scopedEvent = scopedEmitter.event; provider = { name: 'custom', manage: async () => undefined, refresh: async () => undefined, getPackages: async () => [], onDidChangePackages: emitter.event, + createForProject: () => { + scopedProvider = { + name: 'custom', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + onDidChangePackages: scopedEvent, + }; + return scopedProvider; + }, }; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? 'test.packages:custom' : defaultValue, + } as WorkspaceConfiguration); disposables.push( - projectEvents, variableEvents, emitter, + projectEvents, variableEvents, emitter, scopedEmitter, api.registerPackageManager(provider, { extensionId: 'test.packages' }), managers.onDidChangePackages((event) => internalEvents.push(event)), api.onDidChangePackages((event) => publicEvents.push(event)), @@ -124,5 +147,56 @@ for (const inlineEnabled of [false, true]) { verifyForwarding(event); }); + + test('forwards events from a scoped manager with an independent event emitter', () => { + const scopedManager = managers.getPackageManager(project.uri); + assert.ok(scopedManager); + assert.ok(scopedProvider); + + const scopedChanges: DidChangePackagesEventArgs['changes'] = [{ + kind: PackageChangeKind.add, + pkg: api.createPackageItem( + { name: 'scoped', displayName: 'scoped', version: '1.0' }, + environment, + scopedProvider, + ), + }]; + const event: DidChangePackagesEventArgs = { + environment, + manager: scopedProvider, + changes: scopedChanges, + }; + + scopedEmitter.fire(event); + clock.runAll(); + + assert.strictEqual(publicEvents.length, 1); + assert.strictEqual(publicEvents[0], event); + assert.strictEqual(internalEvents.length, 1); + assert.strictEqual(internalEvents[0].manager, scopedManager); + assert.strictEqual(internalEvents[0].changes, scopedChanges); + assert.strictEqual(scopedChanges[0].pkg.pkgId.managerId, scopedManager.id); + }); + + test('routes a shared emitter event by the exact scoped manager instance', () => { + scopedEvent = emitter.event; + const scopedManager = managers.getPackageManager(project.uri); + assert.ok(scopedManager); + assert.ok(scopedProvider); + + const event: DidChangePackagesEventArgs = { + environment, + manager: scopedProvider, + changes, + }; + + emitter.fire(event); + clock.runAll(); + + assert.strictEqual(publicEvents.length, 1); + assert.strictEqual(publicEvents[0], event); + assert.strictEqual(internalEvents.length, 1); + assert.strictEqual(internalEvents[0].manager, scopedManager); + }); }); } diff --git a/src/test/features/packageManager.api.unit.test.ts b/src/test/features/packageManager.api.unit.test.ts index 40902c350..32e753bb9 100644 --- a/src/test/features/packageManager.api.unit.test.ts +++ b/src/test/features/packageManager.api.unit.test.ts @@ -14,9 +14,10 @@ import { Extension } from 'vscode'; import * as assert from 'assert'; +import * as path from 'path'; import * as sinon from 'sinon'; import * as typeMoq from 'typemoq'; -import { Disposable, EventEmitter, Uri } from 'vscode'; +import { Disposable, EventEmitter, Uri, WorkspaceConfiguration } from 'vscode'; import { DidChangeEnvironmentEventArgs, DidChangeEnvironmentsEventArgs, @@ -26,10 +27,13 @@ import { PackageManagementOptions, PackageManager, PythonEnvironment, + PythonProject, } from '../../api'; import * as extensionApis from '../../common/extension.apis'; +import * as workspaceApis from '../../common/workspace.apis'; import { PythonEnvironmentManagers } from '../../features/envManagers'; import type { PythonProjectManager } from '../../features/projectManager'; +import { InternalPackageManager } from '../../managers/common/registeredManagers'; import { setupNonThenable } from '../mocks/helper'; /** @@ -45,7 +49,9 @@ suite('PythonPackageManagerApi Tests', () => { let environment: typeMoq.IMock; let packageManager: typeMoq.IMock; let onDidChangePackagesEmitter: EventEmitter; + let projectChangesEmitter: EventEmitter; let getExtensionStub: sinon.SinonStub; + let projects: PythonProject[]; setup(() => { // Mock extension APIs to avoid registration errors @@ -68,6 +74,10 @@ suite('PythonPackageManagerApi Tests', () => { // Mock project manager projectManager = typeMoq.Mock.ofType(); + projectChangesEmitter = new EventEmitter(); + projects = []; + projectManager.setup((pm) => pm.getProjects()).returns(() => projects); + projectManager.setup((pm) => pm.onDidChangeProjects).returns(() => projectChangesEmitter.event); setupNonThenable(projectManager); // Create environment managers instance @@ -93,6 +103,7 @@ suite('PythonPackageManagerApi Tests', () => { sinon.restore(); envManagers.dispose(); onDidChangePackagesEmitter.dispose(); + projectChangesEmitter.dispose(); }); /** @@ -645,6 +656,91 @@ suite('PythonPackageManagerApi Tests', () => { disposable.dispose(); }); + function registerEnvironmentProvider( + preferredPackageManagerId: string, + usesEnvironment: boolean, + ): { + environment: PythonEnvironment; + getEnvironment: sinon.SinonStub; + getLastKnownEnvironment: sinon.SinonStub; + managerId: string; + disposable: Disposable; + } { + const onDidChangeEnvironmentsEmitter = new EventEmitter(); + const onDidChangeEnvironmentEmitter = new EventEmitter(); + let selectedEnvironment: PythonEnvironment | undefined; + const getEnvironment = sinon.stub().callsFake(async () => selectedEnvironment); + const registration = envManagers.registerEnvironmentManager( + { + name: 'resolver-env-mgr', + preferredPackageManagerId, + onDidChangeEnvironments: onDidChangeEnvironmentsEmitter.event, + onDidChangeEnvironment: onDidChangeEnvironmentEmitter.event, + refresh: async () => undefined, + getEnvironments: async () => [], + set: async () => undefined, + get: getEnvironment, + resolve: async () => undefined, + }, + { extensionId: 'test-ext' }, + ); + const managerId = envManagers.managers.find((manager) => manager.name === 'resolver-env-mgr')!.id; + const resolvedEnvironment: PythonEnvironment = { + ...environment.object, + envId: { id: environment.object.envId.id, managerId }, + }; + selectedEnvironment = usesEnvironment ? resolvedEnvironment : undefined; + const getLastKnownEnvironment = sinon + .stub(envManagers, 'getLastKnownEnvironment') + .callsFake(() => selectedEnvironment); + return { + environment: resolvedEnvironment, + getEnvironment, + getLastKnownEnvironment, + managerId, + disposable: Disposable.from( + registration, + onDidChangeEnvironmentsEmitter, + onDidChangeEnvironmentEmitter, + ), + }; + } + + function registerProjectAwarePackageManager(): { + manager: InternalPackageManager; + createForProject: sinon.SinonStub; + } { + disposable.dispose(); + const createForProject = sinon.stub().callsFake(() => ({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + })); + disposable = envManagers.registerPackageManager({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject, + }); + return { manager: envManagers.packageManagers[0], createForProject }; + } + + function configureDefaultManagers(packageManagerId: string, environmentManagerId: string): void { + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => { + if (section === 'defaultPackageManager') { + return packageManagerId; + } + if (section === 'defaultEnvManager') { + return environmentManagerId; + } + return defaultValue; + }, + } as WorkspaceConfiguration); + } + test('Should retrieve package manager by ID string', () => { // Mock - Get registered package manager ID const managerId = envManagers.packageManagers[0].id; @@ -737,5 +833,334 @@ suite('PythonPackageManagerApi Tests', () => { // Assert assert.strictEqual(manager, undefined, 'Should return undefined for non-existent ID'); }); + + test('Should report when no package manager can be resolved for an environment', () => { + disposable.dispose(); + + const resolution = envManagers.resolvePackageManagerForEnvironment(environment.object); + + assert.strictEqual(resolution.kind, 'notFound'); + assert.strictEqual(resolution.manager, undefined); + }); + + test('Should cache project-bound package managers by project', () => { + disposable.dispose(); + const firstProject = { + name: 'first', + uri: Uri.file(path.join(process.cwd(), 'first-project')), + } as PythonProject; + const secondProject = { + name: 'second', + uri: Uri.file(path.join(process.cwd(), 'second-project')), + } as PythonProject; + projectManager.setup((pm) => pm.get(firstProject.uri)).returns(() => firstProject); + projectManager.setup((pm) => pm.get(secondProject.uri)).returns(() => secondProject); + + const scopedManagers: PackageManager[] = []; + const scopedProvider: PackageManager = { + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject: () => { + const scopedManager: PackageManager = { + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + }; + scopedManagers.push(scopedManager); + return scopedManager; + }, + }; + disposable = envManagers.registerPackageManager(scopedProvider); + const registeredManager = envManagers.packageManagers[0]; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? registeredManager.id : defaultValue, + } as WorkspaceConfiguration); + + const first = envManagers.getPackageManager(firstProject.uri); + const repeatedFirst = envManagers.getPackageManager(firstProject.uri); + const second = envManagers.getPackageManager(secondProject.uri); + + assert.strictEqual(first, repeatedFirst); + assert.notStrictEqual(first, second); + assert.strictEqual(scopedManagers.length, 2); + assert.strictEqual(first?.project, firstProject); + assert.strictEqual(second?.project, secondProject); + assert.ok(registeredManager.equals(scopedManagers[0])); + assert.ok(registeredManager.equals(scopedManagers[1])); + assert.strictEqual(envManagers.packageManagers.length, 1); + }); + + test('Should share a project-independent package manager across projects', () => { + disposable.dispose(); + disposable = envManagers.registerPackageManager({ + name: 'project-independent-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + }); + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'project')), + } as PythonProject; + projectManager.setup((pm) => pm.get(project.uri)).returns(() => project); + const registeredManager = envManagers.packageManagers[0]; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? registeredManager.id : defaultValue, + } as WorkspaceConfiguration); + + assert.strictEqual(envManagers.getPackageManager(project.uri), registeredManager); + }); + + test('Should return a project-independent package manager without resolving projects', () => { + disposable.dispose(); + disposable = envManagers.registerPackageManager({ + name: 'project-independent-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + }); + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'project-independent')), + } as PythonProject; + projects = [project]; + const registeredManager = envManagers.packageManagers[0]; + const provider = registerEnvironmentProvider(registeredManager.id, true); + + const resolution = envManagers.resolvePackageManagerForEnvironment(provider.environment); + + assert.strictEqual(resolution.kind, 'resolved'); + assert.strictEqual(resolution.manager, registeredManager); + assert.ok(provider.getEnvironment.notCalled); + assert.ok(provider.getLastKnownEnvironment.notCalled); + + provider.disposable.dispose(); + }); + + test('Should bind the provided project without inferring it from the environment', () => { + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'explicit-project')), + } as PythonProject; + projectManager.setup((pm) => pm.get(typeMoq.It.isAny())).returns(() => project); + const packageProvider = registerProjectAwarePackageManager(); + const environmentProvider = registerEnvironmentProvider(packageProvider.manager.id, false); + configureDefaultManagers(packageProvider.manager.id, environmentProvider.managerId); + + const manager = envManagers.getPackageManagerForProject(project); + + assert.strictEqual(manager?.project, project); + assert.ok(packageProvider.createForProject.calledOnceWithExactly(project)); + assert.ok(environmentProvider.getEnvironment.notCalled); + assert.ok(environmentProvider.getLastKnownEnvironment.notCalled); + + environmentProvider.disposable.dispose(); + }); + + test('Should resolve a project-aware package manager for the unique project using an environment', () => { + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'unique-project')), + } as PythonProject; + projects = [project]; + projectManager.setup((pm) => pm.get(typeMoq.It.isAny())).returns(() => project); + const packageProvider = registerProjectAwarePackageManager(); + const environmentProvider = registerEnvironmentProvider(packageProvider.manager.id, true); + configureDefaultManagers(packageProvider.manager.id, environmentProvider.managerId); + + const resolution = envManagers.resolvePackageManagerForEnvironment(environmentProvider.environment); + + assert.ok(packageProvider.createForProject.calledOnceWithExactly(project)); + assert.strictEqual(resolution.kind, 'resolved'); + assert.strictEqual(resolution.manager?.project, project); + assert.strictEqual(environmentProvider.getLastKnownEnvironment.callCount, 1); + assert.ok(environmentProvider.getEnvironment.notCalled); + + environmentProvider.disposable.dispose(); + }); + + test('Should not resolve a project-aware package manager without a matching project', () => { + const packageProvider = registerProjectAwarePackageManager(); + const environmentProvider = registerEnvironmentProvider(packageProvider.manager.id, false); + + const resolution = envManagers.resolvePackageManagerForEnvironment(environmentProvider.environment); + + assert.strictEqual(resolution.kind, 'projectRequired'); + assert.strictEqual(resolution.manager, undefined); + assert.ok(packageProvider.createForProject.notCalled); + assert.ok(environmentProvider.getEnvironment.notCalled); + + environmentProvider.disposable.dispose(); + }); + + test('Should not choose a project-aware package manager when multiple projects use an environment', () => { + disposable.dispose(); + const firstProject = { + name: 'first', + uri: Uri.file(path.join(process.cwd(), 'ambiguous-first-project')), + } as PythonProject; + const secondProject = { + name: 'second', + uri: Uri.file(path.join(process.cwd(), 'ambiguous-second-project')), + } as PythonProject; + projects = [firstProject, secondProject]; + + const packageProvider = registerProjectAwarePackageManager(); + const environmentProvider = registerEnvironmentProvider(packageProvider.manager.id, true); + configureDefaultManagers(packageProvider.manager.id, environmentProvider.managerId); + + const resolution = envManagers.resolvePackageManagerForEnvironment(environmentProvider.environment); + + assert.strictEqual(resolution.kind, 'projectRequired'); + assert.strictEqual(resolution.manager, undefined); + assert.ok(packageProvider.createForProject.notCalled); + assert.strictEqual(environmentProvider.getLastKnownEnvironment.callCount, 2); + assert.ok(environmentProvider.getEnvironment.notCalled); + + environmentProvider.disposable.dispose(); + }); + + test('Should evict scoped package managers when their project is removed', async () => { + disposable.dispose(); + const projectUri = Uri.file(path.join(process.cwd(), 'removed-project')); + let currentProject = { name: 'original', uri: projectUri } as PythonProject; + projectManager.setup((pm) => pm.get(projectUri)).returns(() => currentProject); + const scopedManagers: PackageManager[] = []; + const scopedEmitters: EventEmitter[] = []; + const scopedDisposers: sinon.SinonStub[] = []; + const provider: PackageManager = { + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject: () => { + const emitter = new EventEmitter(); + const dispose = sinon.stub(); + const scopedManager: PackageManager = { + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + onDidChangePackages: emitter.event, + dispose, + }; + scopedEmitters.push(emitter); + scopedDisposers.push(dispose); + scopedManagers.push(scopedManager); + return scopedManager; + }, + }; + disposable = envManagers.registerPackageManager(provider); + const registeredManager = envManagers.packageManagers[0]; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? registeredManager.id : defaultValue, + } as WorkspaceConfiguration); + const events: unknown[] = []; + const eventDisposable = envManagers.onDidChangePackages((event) => events.push(event)); + + const original = envManagers.getPackageManager(projectUri); + const originalProject = currentProject; + currentProject = { name: 'replacement', uri: projectUri } as PythonProject; + projectChangesEmitter.fire([currentProject]); + assert.strictEqual(envManagers.getPackageManagerForProject(originalProject), undefined); + const replacement = envManagers.getPackageManager(projectUri); + + assert.notStrictEqual(original, replacement); + assert.strictEqual(scopedManagers.length, 2); + assert.strictEqual(replacement?.project, currentProject); + assert.ok(scopedDisposers[0].calledOnce); + assert.ok(scopedDisposers[1].notCalled); + await assert.rejects( + () => original!.manage(environment.object, { install: ['example'] }), + /Package manager .* has been disposed/, + ); + + const packageChange = { + environment: environment.object, + changes: [], + }; + scopedEmitters[0].fire({ ...packageChange, manager: scopedManagers[0] }); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(events.length, 0); + + scopedEmitters[1].fire({ ...packageChange, manager: scopedManagers[1] }); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(events.length, 1); + + eventDisposable.dispose(); + scopedEmitters.forEach((emitter) => emitter.dispose()); + }); + + test('Should dispose scoped package managers when their provider is unregistered', () => { + disposable.dispose(); + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'unregistered-provider-project')), + } as PythonProject; + projectManager.setup((pm) => pm.get(typeMoq.It.isAny())).returns(() => project); + const scopedDispose = sinon.stub(); + disposable = envManagers.registerPackageManager({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject: () => ({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + dispose: scopedDispose, + }), + }); + const registeredManager = envManagers.packageManagers[0]; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? registeredManager.id : defaultValue, + } as WorkspaceConfiguration); + assert.ok(envManagers.getPackageManager(project.uri)?.project); + + disposable.dispose(); + + assert.ok(scopedDispose.calledOnce); + }); + + test('Should dispose scoped package managers during environment-manager shutdown', () => { + disposable.dispose(); + const project = { + name: 'project', + uri: Uri.file(path.join(process.cwd(), 'shutdown-project')), + } as PythonProject; + projectManager.setup((pm) => pm.get(typeMoq.It.isAny())).returns(() => project); + const scopedDispose = sinon.stub(); + disposable = envManagers.registerPackageManager({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject: () => ({ + name: 'project-pkg-mgr', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + dispose: scopedDispose, + }), + }); + const registeredManager = envManagers.packageManagers[0]; + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (section: string, defaultValue?: unknown) => + section === 'defaultPackageManager' ? registeredManager.id : defaultValue, + } as WorkspaceConfiguration); + assert.ok(envManagers.getPackageManager(project.uri)?.project); + + envManagers.dispose(); + + assert.ok(scopedDispose.calledOnce); + }); }); }); diff --git a/src/test/features/views/envManagersView.unit.test.ts b/src/test/features/views/envManagersView.unit.test.ts index 96a604205..79ad6d2ed 100644 --- a/src/test/features/views/envManagersView.unit.test.ts +++ b/src/test/features/views/envManagersView.unit.test.ts @@ -1,3 +1,4 @@ +import * as assert from 'assert'; import * as sinon from 'sinon'; import * as typeMoq from 'typemoq'; import { EventEmitter, TreeView, Uri } from 'vscode'; @@ -28,6 +29,7 @@ suite('EnvManagerView.reveal Tests', () => { let onDidChangeEnvironmentManagerEmitter: EventEmitter; let onDidChangePackagesEmitter: EventEmitter; let onDidChangePackageManagerEmitter: EventEmitter; + let onDidChangeProjectPackageManagerEmitter: EventEmitter; let onDidChangeStateEmitter: EventEmitter<{ itemId: string; stateKey: string }>; setup(() => { @@ -36,6 +38,7 @@ suite('EnvManagerView.reveal Tests', () => { onDidChangeEnvironmentManagerEmitter = new EventEmitter(); onDidChangePackagesEmitter = new EventEmitter(); onDidChangePackageManagerEmitter = new EventEmitter(); + onDidChangeProjectPackageManagerEmitter = new EventEmitter(); onDidChangeStateEmitter = new EventEmitter(); // Mock manager @@ -53,6 +56,12 @@ suite('EnvManagerView.reveal Tests', () => { .returns(() => onDidChangeEnvironmentManagerEmitter.event); envManagers.setup((e) => e.onDidChangePackages).returns(() => onDidChangePackagesEmitter.event); envManagers.setup((e) => e.onDidChangePackageManager).returns(() => onDidChangePackageManagerEmitter.event); + envManagers + .setup((e) => e.resolvePackageManagerForEnvironment(typeMoq.It.isAny())) + .returns(() => ({ kind: 'notFound' })); + envManagers + .setup((e) => e.onDidChangeProjectPackageManager) + .returns(() => onDidChangeProjectPackageManagerEmitter.event); setupNonThenable(envManagers); // Mock state manager @@ -75,6 +84,7 @@ suite('EnvManagerView.reveal Tests', () => { onDidChangeEnvironmentManagerEmitter.dispose(); onDidChangePackagesEmitter.dispose(); onDidChangePackageManagerEmitter.dispose(); + onDidChangeProjectPackageManagerEmitter.dispose(); onDidChangeStateEmitter.dispose(); }); @@ -218,4 +228,18 @@ suite('EnvManagerView.reveal Tests', () => { view.dispose(); }); + + test('Refreshes the tree when a project-scoped package manager is invalidated', () => { + const clock = sinon.useFakeTimers(); + const view = new EnvManagerView(envManagers.object, stateManager.object); + const changed = sinon.stub(); + const listener = view.onDidChangeTreeData(changed); + + onDidChangeProjectPackageManagerEmitter.fire(); + clock.tick(500); + + assert.ok(changed.calledOnce); + listener.dispose(); + view.dispose(); + }); }); diff --git a/src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts b/src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts index 9e65ae415..46eaf7f5c 100644 --- a/src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts +++ b/src/test/managers/common/packageManagerHeadlessConformance.unit.test.ts @@ -3,7 +3,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; -import { LogOutputChannel, Uri } from 'vscode'; +import { FileType, LogOutputChannel, Uri } from 'vscode'; import { PackageManager, PythonEnvironment, @@ -13,6 +13,7 @@ import { import * as childProcessApis from '../../../common/childProcess.apis'; import * as errorUtils from '../../../common/errors/utils'; import * as windowApis from '../../../common/window.apis'; +import * as workspaceFs from '../../../common/workspace.fs.apis'; import * as workspaceApis from '../../../common/workspace.apis'; import { InternalPackageManager } from '../../../managers/common/registeredManagers'; import { PipInstallCommand } from '../../../managers/builtin/commands/install'; @@ -40,6 +41,15 @@ suite('Package manager headless conformance', () => { version: '3.12.0', } as unknown as PythonEnvironment; + setup(() => { + sinon.stub(workspaceFs, 'stat').resolves({ + type: FileType.Directory, + ctime: 0, + mtime: 0, + size: 0, + }); + }); + teardown(() => { sinon.restore(); }); @@ -202,7 +212,10 @@ suite('Package manager headless conformance', () => { getProjectsByEnvironment: sinon.stub().returns([]), } as unknown as VenvManager); const conda = new CondaPackageManager(api, log); - const poetry = new PoetryPackageManager(api, log, {} as PoetryManager); + const poetry = new PoetryPackageManager(api, log, {} as PoetryManager).createForProject({ + name: 'project', + uri: Uri.file(process.cwd()), + }); return { pip, conda, poetry, all: [pip, conda, poetry] }; } diff --git a/src/test/managers/common/packageWatcher.unit.test.ts b/src/test/managers/common/packageWatcher.unit.test.ts index c8f09f7f6..6612b2659 100644 --- a/src/test/managers/common/packageWatcher.unit.test.ts +++ b/src/test/managers/common/packageWatcher.unit.test.ts @@ -4,7 +4,13 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { Disposable, EventEmitter, LogOutputChannel, RelativePattern, Terminal, Uri } from 'vscode'; -import { DidChangeEnvironmentEventArgs, PackageManager, PythonEnvironment, PythonEnvironmentId } from '../../../api'; +import { + DidChangeEnvironmentEventArgs, + PackageManager, + PythonEnvironment, + PythonEnvironmentId, + PythonProject, +} from '../../../api'; import * as windowApis from '../../../common/window.apis'; import * as workspaceApis from '../../../common/workspace.apis'; import type { EnvironmentManagers } from '../../../features/envManagers'; @@ -406,6 +412,36 @@ suite('Package Watcher', () => { assert.ok((mockWatcher.dispose as sinon.SinonStub).called, 'Should dispose watcher after the final scope'); }); + test('should use separate watchers for project-bound package managers', () => { + createFileSystemWatcherStub.returns(createMockWatcher()); + const environmentChanges = new EventEmitter(); + const firstScope = Uri.file('workspace-one'); + const secondScope = Uri.file('workspace-two'); + const firstPackageManager = new InternalPackageManager( + 'poetry', + createMockPackageManager() as PackageManager, + { name: 'first', uri: firstScope } as PythonProject, + ); + const secondPackageManager = new InternalPackageManager( + 'poetry', + createMockPackageManager() as PackageManager, + { name: 'second', uri: secondScope } as PythonProject, + ); + const envManagers = { + onDidChangeActiveEnvironment: environmentChanges.event, + getPackageManager: sandbox + .stub() + .callsFake((scope) => (scope === firstScope ? firstPackageManager : secondPackageManager)), + } as unknown as EnvironmentManagers; + const env = createMockEnvironment(); + + registerPackageWatchers(envManagers, mockTerminalActivation, mockLogOutputChannel as LogOutputChannel); + environmentChanges.fire({ uri: firstScope, new: env, old: undefined }); + environmentChanges.fire({ uri: secondScope, new: env, old: undefined }); + + assert.strictEqual(createFileSystemWatcherStub.callCount, 2); + }); + test('should stop watching an environment when the active environment changes', () => { const firstWatcher = createMockWatcher(); const secondWatcher = createMockWatcher(); @@ -502,6 +538,61 @@ suite('Package Watcher', () => { assert.strictEqual(createFileSystemWatcherStub.callCount, 1); }); + test('should reuse a project-scoped watcher for terminal activation', () => { + createFileSystemWatcherStub.returns(createMockWatcher()); + const environmentChanges = new EventEmitter(); + const scope = Uri.file('workspace'); + const project = { name: 'project', uri: scope } as PythonProject; + const rootManager = new InternalPackageManager('poetry', { + ...createMockPackageManager(), + createForProject: () => createMockPackageManager() as PackageManager, + } as PackageManager); + const scopedManager = new InternalPackageManager( + 'poetry', + createMockPackageManager() as PackageManager, + project, + ); + const env = createMockEnvironment(); + const terminal = { name: 'terminal' } as Terminal; + const envManagers = { + onDidChangeActiveEnvironment: environmentChanges.event, + getPackageManager: sandbox + .stub() + .callsFake((context) => (context instanceof Uri ? scopedManager : rootManager)), + } as unknown as EnvironmentManagers; + + registerPackageWatchers(envManagers, mockTerminalActivation, mockLogOutputChannel as LogOutputChannel); + environmentChanges.fire({ uri: scope, new: env, old: undefined }); + terminalActivationChanges.fire({ terminal, environment: env, activated: true }); + + assert.strictEqual(createFileSystemWatcherStub.callCount, 1); + assert.ok((envManagers.getPackageManager as sinon.SinonStub).calledWith(scope)); + }); + + test('should skip terminal-only watchers for project-aware package managers', () => { + const environmentChanges = new EventEmitter(); + const packageManager = new InternalPackageManager('poetry', { + ...createMockPackageManager(), + createForProject: () => createMockPackageManager() as PackageManager, + } as PackageManager); + const env = createMockEnvironment(); + const terminal = { name: 'terminal' } as Terminal; + const envManagers = { + onDidChangeActiveEnvironment: environmentChanges.event, + getPackageManager: sandbox.stub().returns(packageManager), + } as unknown as EnvironmentManagers; + + registerPackageWatchers(envManagers, mockTerminalActivation, mockLogOutputChannel as LogOutputChannel); + terminalActivationChanges.fire({ terminal, environment: env, activated: true }); + + assert.strictEqual(createFileSystemWatcherStub.callCount, 0); + assert.ok( + (mockLogOutputChannel.debug as sinon.SinonStub).calledWith( + 'Skipping unscoped package watcher for project-aware manager poetry', + ), + ); + }); + test('should release a terminal environment watcher when the terminal closes', () => { const mockWatcher = createMockWatcher(); createFileSystemWatcherStub.returns(mockWatcher); @@ -573,10 +664,16 @@ suite('Package Watcher', () => { configurationChanges.event(listener), ); const environmentChanges = new EventEmitter(); - const firstPackageManager = new InternalPackageManager('pip', createMockPackageManager() as PackageManager); + const project = { name: 'project', uri: Uri.file('workspace') } as PythonProject; + const firstPackageManager = new InternalPackageManager( + 'first', + createMockPackageManager() as PackageManager, + project, + ); const secondPackageManager = new InternalPackageManager( - 'conda', + 'second', createMockPackageManager() as PackageManager, + project, ); let selectedPackageManager = firstPackageManager; const envManagers = { @@ -598,5 +695,50 @@ suite('Package Watcher', () => { assert.strictEqual(createFileSystemWatcherStub.callCount, 2); configurationChanges.dispose(); }); + + test('should resolve the current scoped manager when refreshing', async () => { + const clock = sandbox.useFakeTimers(); + const mockWatcher = createMockWatcher(); + createFileSystemWatcherStub.returns(mockWatcher); + const environmentChanges = new EventEmitter(); + const project = { name: 'project', uri: Uri.file('workspace') } as PythonProject; + const firstProvider = createMockPackageManager(); + const secondProvider = createMockPackageManager(); + const firstManager = new InternalPackageManager( + 'poetry', + firstProvider as PackageManager, + project, + ); + const secondManager = new InternalPackageManager( + 'poetry', + secondProvider as PackageManager, + { name: 'replacement', uri: project.uri } as PythonProject, + ); + const rootProvider = createMockPackageManager(); + const rootManager = new InternalPackageManager('poetry', rootProvider as PackageManager); + let selectedManager = firstManager; + const envManagers = { + onDidChangeActiveEnvironment: environmentChanges.event, + getPackageManager: sandbox.stub().callsFake(() => selectedManager), + } as unknown as EnvironmentManagers; + const env = createMockEnvironment(); + + registerPackageWatchers(envManagers, mockTerminalActivation, mockLogOutputChannel as LogOutputChannel); + environmentChanges.fire({ uri: project.uri, new: env, old: undefined }); + + selectedManager = secondManager; + mockWatcher._changeEmitter.fire(Uri.file('replacement.dist-info')); + await clock.tickAsync(600); + + assert.ok((firstProvider.refresh as sinon.SinonStub).notCalled); + assert.ok((secondProvider.refresh as sinon.SinonStub).calledOnceWithExactly(env)); + + selectedManager = rootManager; + mockWatcher._changeEmitter.fire(Uri.file('removed.dist-info')); + await clock.tickAsync(600); + + assert.ok((secondProvider.refresh as sinon.SinonStub).calledOnce); + assert.ok((rootProvider.refresh as sinon.SinonStub).notCalled); + }); }); }); diff --git a/src/test/managers/common/projectScopedPackageManagerCache.unit.test.ts b/src/test/managers/common/projectScopedPackageManagerCache.unit.test.ts new file mode 100644 index 000000000..5e59dcd97 --- /dev/null +++ b/src/test/managers/common/projectScopedPackageManagerCache.unit.test.ts @@ -0,0 +1,183 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { Disposable, Uri } from 'vscode'; +import { PackageManager, PythonProject } from '../../../api'; +import { ProjectScopedPackageManagerCache } from '../../../managers/common/projectScopedPackageManagerCache'; +import { InternalPackageManager } from '../../../managers/common/registeredManagers'; + +suite('ProjectScopedPackageManagerCache', () => { + let invalidate: sinon.SinonStub; + let subscribe: sinon.SinonStub; + let subscriptionDisposers: sinon.SinonStub[]; + let cache: ProjectScopedPackageManagerCache; + + setup(() => { + invalidate = sinon.stub(); + subscriptionDisposers = []; + subscribe = sinon.stub().callsFake(() => { + const dispose = sinon.stub(); + subscriptionDisposers.push(dispose); + return new Disposable(dispose); + }); + cache = new ProjectScopedPackageManagerCache(subscribe, invalidate); + }); + + teardown(() => { + cache.dispose(); + sinon.restore(); + }); + + function createProject(name: string): PythonProject { + return { + name, + uri: Uri.file(path.join(process.cwd(), name)), + }; + } + + function createProvider(name: string): { + provider: InternalPackageManager; + createForProject: sinon.SinonStub; + scopedDisposers: sinon.SinonStub[]; + } { + const scopedDisposers: sinon.SinonStub[] = []; + const createForProject = sinon.stub().callsFake(() => { + const dispose = sinon.stub(); + scopedDisposers.push(dispose); + return { + name, + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + dispose, + } satisfies PackageManager; + }); + const provider = new InternalPackageManager(name, { + name, + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + createForProject, + }); + return { provider, createForProject, scopedDisposers }; + } + + test('memoizes one scoped manager per canonical project', () => { + const { provider, createForProject } = createProvider('project-aware'); + const firstProject = createProject('first-project'); + const secondProject = createProject('second-project'); + + const first = cache.getOrCreate(provider, firstProject); + const repeatedFirst = cache.getOrCreate(provider, firstProject); + const second = cache.getOrCreate(provider, secondProject); + + assert.strictEqual(first, repeatedFirst); + assert.notStrictEqual(first, second); + assert.strictEqual(first?.project, firstProject); + assert.strictEqual(second?.project, secondProject); + assert.strictEqual(createForProject.callCount, 2); + assert.strictEqual(subscribe.callCount, 2); + assert.ok(invalidate.notCalled); + }); + + test('replaces and disposes a scoped manager when the provider changes', () => { + const firstProvider = createProvider('first-provider'); + const secondProvider = createProvider('second-provider'); + const project = createProject('provider-change-project'); + const first = cache.getOrCreate(firstProvider.provider, project); + + const second = cache.getOrCreate(secondProvider.provider, project); + + assert.notStrictEqual(first, second); + assert.ok(firstProvider.scopedDisposers[0].calledOnce); + assert.ok(subscriptionDisposers[0].calledOnce); + assert.ok(secondProvider.scopedDisposers[0].notCalled); + assert.ok(subscriptionDisposers[1].notCalled); + assert.ok(invalidate.calledOnce); + }); + + test('reconciles replaced and removed canonical projects', () => { + const { provider, scopedDisposers } = createProvider('project-aware'); + const original = createProject('reconciled-project'); + const replacement = createProject('reconciled-project'); + cache.getOrCreate(provider, original); + + cache.reconcileProjects([replacement]); + + assert.ok(scopedDisposers[0].calledOnce); + assert.ok(subscriptionDisposers[0].calledOnce); + assert.ok(invalidate.calledOnce); + + cache.getOrCreate(provider, replacement); + cache.reconcileProjects([]); + + assert.ok(scopedDisposers[1].calledOnce); + assert.ok(subscriptionDisposers[1].calledOnce); + assert.ok(invalidate.calledTwice); + }); + + test('removes only scoped managers created by the requested provider', () => { + const firstProvider = createProvider('first-provider'); + const secondProvider = createProvider('second-provider'); + cache.getOrCreate(firstProvider.provider, createProject('first-provider-project')); + cache.getOrCreate(secondProvider.provider, createProject('second-provider-project')); + + cache.removeProvider(firstProvider.provider); + + assert.ok(firstProvider.scopedDisposers[0].calledOnce); + assert.ok(subscriptionDisposers[0].calledOnce); + assert.ok(secondProvider.scopedDisposers[0].notCalled); + assert.ok(subscriptionDisposers[1].notCalled); + assert.ok(invalidate.calledOnce); + }); + + test('disposes all scoped managers and subscriptions', () => { + const { provider, scopedDisposers } = createProvider('project-aware'); + cache.getOrCreate(provider, createProject('first-project')); + cache.getOrCreate(provider, createProject('second-project')); + + cache.dispose(); + + assert.ok(scopedDisposers[0].calledOnce); + assert.ok(scopedDisposers[1].calledOnce); + assert.ok(subscriptionDisposers[0].calledOnce); + assert.ok(subscriptionDisposers[1].calledOnce); + }); + + test('returns project-independent providers without caching or subscribing', () => { + const provider = new InternalPackageManager('shared', { + name: 'shared', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + }); + + const resolved = cache.getOrCreate(provider, createProject('shared-project')); + + assert.strictEqual(resolved, provider); + assert.ok(subscribe.notCalled); + assert.ok(invalidate.notCalled); + }); + + test('disposes a scoped manager when its project switches to a project-independent provider', () => { + const projectAwareProvider = createProvider('project-aware'); + const sharedProvider = new InternalPackageManager('shared', { + name: 'shared', + manage: async () => undefined, + refresh: async () => undefined, + getPackages: async () => [], + }); + const project = createProject('provider-kind-change-project'); + cache.getOrCreate(projectAwareProvider.provider, project); + + const resolved = cache.getOrCreate(sharedProvider, project); + + assert.strictEqual(resolved, sharedProvider); + assert.ok(projectAwareProvider.scopedDisposers[0].calledOnce); + assert.ok(subscriptionDisposers[0].calledOnce); + assert.ok(invalidate.calledOnce); + }); +}); diff --git a/src/test/managers/poetry/poetryPackageManager.unit.test.ts b/src/test/managers/poetry/poetryPackageManager.unit.test.ts index e96cf746a..a22062cfb 100644 --- a/src/test/managers/poetry/poetryPackageManager.unit.test.ts +++ b/src/test/managers/poetry/poetryPackageManager.unit.test.ts @@ -4,9 +4,11 @@ import assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; -import { LogOutputChannel, Uri } from 'vscode'; +import { FileType, LogOutputChannel, Uri } from 'vscode'; import { PythonEnvironmentApi } from '../../../api'; import * as windowApis from '../../../common/window.apis'; +import * as workspaceFs from '../../../common/workspace.fs.apis'; +import { PackageManagerRequiresProjectError } from '../../../managers/common/errors'; import * as packageChanges from '../../../managers/common/packageChanges'; import * as runPoetryModule from '../../../managers/poetry/commands/runPoetry'; import { PoetryPackageManager } from '../../../managers/poetry/poetryPackageManager'; @@ -21,17 +23,13 @@ suite('PoetryPackageManager', () => { }); let logError: sinon.SinonStub; let manager: PoetryPackageManager; + let projectManager: PoetryPackageManager; let runPoetryStub: sinon.SinonStub; + let statStub: sinon.SinonStub; + const projectUri = Uri.file(path.join(process.cwd(), 'project', 'pyproject.toml')); setup(() => { - const api = { - getPythonProjects: () => [ - { - name: 'project', - uri: Uri.file(path.join(process.cwd(), 'project', 'pyproject.toml')), - }, - ], - } as unknown as PythonEnvironmentApi; + const api = {} as PythonEnvironmentApi; logError = sinon.stub(); const log = { append: sinon.stub(), @@ -44,7 +42,10 @@ suite('PoetryPackageManager', () => { sinon.stub(windowApis, 'withProgress').callsFake((_options, task) => task({} as never, {} as never)); sinon.stub(packageChanges, 'updatePackagesAndNotify').resolves([]); runPoetryStub = sinon.stub(runPoetryModule, 'runPoetry').resolves(''); + statStub = sinon.stub(workspaceFs, 'stat'); + statStub.resolves({ type: FileType.File, ctime: 0, mtime: 0, size: 0 }); manager = new PoetryPackageManager(api, log, {} as PoetryManager); + projectManager = manager.createForProject({ name: 'project', uri: projectUri }); }); teardown(() => { @@ -52,28 +53,100 @@ suite('PoetryPackageManager', () => { sinon.restore(); }); - test('package management inherits the process working directory', async () => { - await manager.manage(environment, { install: ['requests'], uninstall: ['flask'] }); + test('package management uses the project working directory', async () => { + await projectManager.manage(environment, { install: ['requests'], uninstall: ['flask'] }); assert.strictEqual(runPoetryStub.callCount, 2); - assert.strictEqual(runPoetryStub.firstCall.args[1], undefined); - assert.strictEqual(runPoetryStub.secondCall.args[1], undefined); + assert.strictEqual(runPoetryStub.firstCall.args[1], path.dirname(projectUri.fsPath)); + assert.strictEqual(runPoetryStub.secondCall.args[1], path.dirname(projectUri.fsPath)); }); - test('direct package listing inherits the process working directory', async () => { - await manager.getDirectPackageNames(environment); + test('direct package listing uses the project working directory', async () => { + await projectManager.getDirectPackageNames(environment); assert.strictEqual(runPoetryStub.callCount, 1); - assert.strictEqual(runPoetryStub.firstCall.args[1], undefined); + assert.strictEqual(runPoetryStub.firstCall.args[1], path.dirname(projectUri.fsPath)); + }); + + test('directory project URIs are used directly as the working directory', async () => { + const directoryUri = Uri.file(process.cwd()); + statStub.withArgs(directoryUri).resolves({ type: FileType.Directory, ctime: 0, mtime: 0, size: 0 }); + const directoryManager = manager.createForProject({ name: 'directory-project', uri: directoryUri }); + + await directoryManager.getDirectPackageNames(environment); + + assert.strictEqual(runPoetryStub.callCount, 1); + assert.strictEqual(runPoetryStub.firstCall.args[1], directoryUri.fsPath); + }); + + test('symlinked directory project URIs are used directly as the working directory', async () => { + const symlinkedDirectoryUri = Uri.file(path.join(process.cwd(), 'symlinked-project')); + statStub + .withArgs(symlinkedDirectoryUri) + .resolves({ type: FileType.Directory | FileType.SymbolicLink, ctime: 0, mtime: 0, size: 0 }); + const symlinkedDirectoryManager = manager.createForProject({ + name: 'symlinked-directory-project', + uri: symlinkedDirectoryUri, + }); + + await symlinkedDirectoryManager.getDirectPackageNames(environment); + + assert.strictEqual(runPoetryStub.callCount, 1); + assert.strictEqual(runPoetryStub.firstCall.args[1], symlinkedDirectoryUri.fsPath); + }); + + test('inaccessible projects reject instead of falling back to the parent directory', async () => { + const inaccessibleUri = Uri.file(path.join(process.cwd(), 'missing-project')); + statStub.withArgs(inaccessibleUri).rejects(new Error('access denied')); + const inaccessibleManager = manager.createForProject({ name: 'missing', uri: inaccessibleUri }); + + await assert.rejects( + inaccessibleManager.manage(environment, { install: ['requests'] }), + /Unable to access the Python project/, + ); + + assert.strictEqual(runPoetryStub.callCount, 0); }); test('package loading returns an empty list when poetry show fails', async () => { const showError = new Error('poetry show failed'); runPoetryStub.rejects(showError); - const packages = await manager.getPackages(environment, { skipCache: true }); + const packages = await projectManager.getPackages(environment, { skipCache: true }); assert.deepStrictEqual(packages, []); assert.ok(logError.calledOnceWithExactly(`Error refreshing packages with Poetry: ${showError}`)); }); + + test('project-sensitive reads are unavailable without a project', async () => { + assert.strictEqual(await manager.getPackages(environment), undefined); + assert.strictEqual(await manager.getDirectPackageNames(environment), undefined); + assert.strictEqual(runPoetryStub.callCount, 0); + }); + + test('package management rejects operations without a project', async () => { + await assert.rejects( + manager.manage(environment, { install: ['requests'] }), + /require a Python project/, + ); + assert.strictEqual(runPoetryStub.callCount, 0); + }); + + test('unbound package management rejects before no-op exits or prompts', async () => { + const showInputBox = sinon.stub(windowApis, 'showInputBox'); + + await assert.rejects( + manager.manage(environment, { install: [], runHeadless: true }), + /require a Python project/, + ); + await assert.rejects(manager.manage(environment, { install: [] }), /require a Python project/); + + assert.ok(showInputBox.notCalled); + assert.strictEqual(runPoetryStub.callCount, 0); + }); + + test('refresh rejects without a project', async () => { + await assert.rejects(manager.refresh(environment), PackageManagerRequiresProjectError); + assert.strictEqual(runPoetryStub.callCount, 0); + }); }); diff --git a/src/types.ts b/src/types.ts index 12c93df99..8699ce618 100644 --- a/src/types.ts +++ b/src/types.ts @@ -715,9 +715,36 @@ export interface PackageManager { /** * Event that is fired when packages change. + * + * A manager returned by createForProject must report that exact manager instance in + * DidChangePackagesEventArgs.manager, even when root and scoped managers share an emitter. */ onDidChangePackages?: Event; + /** + * Creates a package manager bound to a Python project. + * + * The extension uses the explicit project supplied by project-based callers. When a caller + * provides only an environment, the extension uses a project-bound manager only if exactly + * one tracked project uses that environment. It does not invoke project-sensitive operations + * on the registered root manager when no project can be selected safely. + * + * Project-independent package managers can omit this method. Implementations should keep + * project-specific caches and mutable state on the returned manager rather than the root. + * + * @param project - The project to bind to the package manager. + * @returns A package manager that uses the project for project-sensitive operations. + */ + createForProject?(project: PythonProject): PackageManager; + + /** + * Releases resources owned by this package manager. + * + * The extension invokes this method for managers returned by createForProject when their + * project is removed or replaced, their provider is unregistered, or the extension shuts down. + */ + dispose?(): void; + /** * Fetches the names of direct (non-transitive) packages for the specified Python environment. * @@ -1152,6 +1179,8 @@ export interface PythonPackageGetterApi { * * @param environment The Python Environment for which the list of packages is to be refreshed. * @returns A promise that resolves when the list of packages has been refreshed. + * @throws {@link PackageManagerRequiresProjectError} if a project-aware manager cannot identify + * a unique project for the environment. */ refreshPackages(environment: PythonEnvironment): Promise; @@ -1160,7 +1189,8 @@ export interface PythonPackageGetterApi { * * @param environment The Python Environment for which the list of packages is required. * @param options Optional settings for package retrieval. - * @returns The list of packages in the Python Environment. + * @returns The list of packages in the Python Environment, or `undefined` if no package manager + * can be resolved for the environment. */ getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise; @@ -1213,8 +1243,9 @@ export interface PythonPackageManagementApi { * Install/Uninstall packages into a Python Environment. * * @param environment The Python Environment into which packages are to be installed. - * @param packages The packages to install. * @param options Options for installing packages. + * @throws {@link PackageManagerRequiresProjectError} if a project-aware manager cannot identify + * a unique project for the environment. */ managePackages(environment: PythonEnvironment, options: PackageManagementOptions): Promise; }