fix: poetry execution bug fixes

karthiknadig · karthiknadig · commit 5b087e7ac40d · 2025-05-22T14:28:04.000-07:00
diff --git a/src/managers/common/utils.ts b/src/managers/common/utils.ts
@@ -87,3 +87,28 @@ export function mergePackages(common: Installable[], installed: string[]): Insta
 export function pathForGitBash(binPath: string): string {
     return isWindows() ? binPath.replace(/\\/g, '/').replace(/^([a-zA-Z]):/, '/$1') : binPath;
 }
+
+/**
+ * Compares two semantic version strings. Support sonly simple 1.1.1 style versions.
+ * @param version1 First version
+ * @param version2 Second version
+ * @returns -1 if version1 < version2, 0 if equal, 1 if version1 > version2
+ */
+export function compareVersions(version1: string, version2: string): number {
+    const v1Parts = version1.split('.').map(Number);
+    const v2Parts = version2.split('.').map(Number);
+
+    for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) {
+        const v1Part = v1Parts[i] || 0;
+        const v2Part = v2Parts[i] || 0;
+
+        if (v1Part > v2Part) {
+            return 1;
+        }
+        if (v1Part < v2Part) {
+            return -1;
+        }
+    }
+
+    return 0;
+}
diff --git a/src/managers/poetry/main.ts b/src/managers/poetry/main.ts
@@ -1,11 +1,13 @@
-import { Disposable, LogOutputChannel } from 'vscode';
+import { Disposable, l10n, LogOutputChannel } from 'vscode';
 import { PythonEnvironmentApi } from '../../api';
 import { traceInfo } from '../../common/logging';
+import { showErrorMessage } from '../../common/window.apis';
 import { getPythonApi } from '../../features/pythonApi';
 import { NativePythonFinder } from '../common/nativePythonFinder';
+import { compareVersions } from '../common/utils';
 import { PoetryManager } from './poetryManager';
 import { PoetryPackageManager } from './poetryPackageManager';
-import { getPoetry } from './poetryUtils';
+import { getPoetry, getPoetryVersion, isPoetryShellPluginInstalled } from './poetryUtils';
 
 export async function registerPoetryFeatures(
     nativeFinder: NativePythonFinder,
@@ -15,16 +17,35 @@ export async function registerPoetryFeatures(
     const api: PythonEnvironmentApi = await getPythonApi();
 
     try {
-        await getPoetry(nativeFinder);
+        const poetryPath = await getPoetry(nativeFinder);
+        let shellSupported = true;
+        if (poetryPath) {
+            const version = await getPoetryVersion(poetryPath);
+            if (!version) {
+                showErrorMessage(l10n.t('Poetry version could not be determined.'));
+                return;
+            }
+            if (version && compareVersions(version, '2.0.0') >= 0) {
+                shellSupported = await isPoetryShellPluginInstalled(poetryPath);
+                if (!shellSupported) {
+                    showErrorMessage(
+                        l10n.t(
+                            'Poetry 2.0.0+ detected. The `shell` command is not available by default. Please install the shell plugin to enable shell activation. See  [here](https://python-poetry.org/docs/managing-environments/#activating-the-environment), shell [plugin](https://github.com/python-poetry/poetry-plugin-shell)',
+                        ),
+                    );
+                    return;
+                }
+            }
+        }
 
         const envManager = new PoetryManager(nativeFinder, api);
         const pkgManager = new PoetryPackageManager(api, outputChannel, envManager);
-        
+
         disposables.push(
-            envManager, 
+            envManager,
             pkgManager,
             api.registerEnvironmentManager(envManager),
-            api.registerPackageManager(pkgManager)
+            api.registerPackageManager(pkgManager),
         );
     } catch (ex) {
         traceInfo('Poetry not found, turning off poetry features.', ex);
diff --git a/src/managers/poetry/poetryManager.ts b/src/managers/poetry/poetryManager.ts
@@ -25,7 +25,7 @@ import {
     clearPoetryCache,
     getPoetryForGlobal,
     getPoetryForWorkspace,
-    POETRY_ENVIRONMENTS,
+    POETRY_GLOBAL,
     refreshPoetry,
     resolvePoetryPath,
     setPoetryForGlobal,
@@ -98,7 +98,7 @@ export class PoetryManager implements EnvironmentManager, Disposable {
 
         if (scope === 'global') {
             return this.collection.filter((env) => {
-                return env.group === POETRY_ENVIRONMENTS;
+                return env.group === POETRY_GLOBAL;
             });
         }
 
@@ -155,7 +155,7 @@ export class PoetryManager implements EnvironmentManager, Disposable {
 
         return this.globalEnv;
     }
-    
+
     async set(scope: SetEnvironmentScope, environment?: PythonEnvironment | undefined): Promise<void> {
         if (scope === undefined) {
             await setPoetryForGlobal(environment?.environmentPath?.fsPath);
@@ -251,7 +251,7 @@ export class PoetryManager implements EnvironmentManager, Disposable {
         }
 
         if (!this.globalEnv) {
-            this.globalEnv = getLatest(this.collection.filter((e) => e.group === POETRY_ENVIRONMENTS));
+            this.globalEnv = getLatest(this.collection.filter((e) => e.group === POETRY_GLOBAL));
         }
 
         // Find any poetry environments that might be associated with the current projects
diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts
@@ -8,7 +8,6 @@ import {
     MarkdownString,
     ProgressLocation,
     ThemeIcon,
-    window
 } from 'vscode';
 import { Disposable } from 'vscode-jsonrpc';
 import {
@@ -21,6 +20,7 @@ import {
     PythonEnvironment,
     PythonEnvironmentApi,
 } from '../../api';
+import { showErrorMessage, showInputBox, withProgress } from '../../common/window.apis';
 import { PoetryManager } from './poetryManager';
 import { getPoetry } from './poetryUtils';
 
@@ -64,21 +64,24 @@ export class PoetryPackageManager implements PackageManager, Disposable {
 
         if (toInstall.length === 0 && toUninstall.length === 0) {
             // Show package input UI if no packages are specified
-            const installInput = await window.showInputBox({
+            const installInput = await showInputBox({
                 prompt: 'Enter packages to install (comma separated)',
-                placeHolder: 'e.g., requests, pytest, black'
+                placeHolder: 'e.g., requests, pytest, black',
             });
-            
+
             if (installInput) {
-                toInstall = installInput.split(',').map(p => p.trim()).filter(p => p.length > 0);
+                toInstall = installInput
+                    .split(',')
+                    .map((p) => p.trim())
+                    .filter((p) => p.length > 0);
             }
-            
+
             if (toInstall.length === 0) {
                 return;
             }
         }
 
-        await window.withProgress(
+        await withProgress(
             {
                 location: ProgressLocation.Notification,
                 title: 'Managing packages with Poetry',
@@ -87,7 +90,11 @@ export class PoetryPackageManager implements PackageManager, Disposable {
             async (_progress, token) => {
                 try {
                     const before = this.packages.get(environment.envId.id) ?? [];
-                    const after = await this.managePackages(environment, {install: toInstall, uninstall: toUninstall}, token);
+                    const after = await this.managePackages(
+                        environment,
+                        { install: toInstall, uninstall: toUninstall },
+                        token,
+                    );
                     const changes = getChanges(before, after);
                     this.packages.set(environment.envId.id, after);
                     this._onDidChangePackages.fire({ environment, manager: this, changes });
@@ -97,7 +104,7 @@ export class PoetryPackageManager implements PackageManager, Disposable {
                     }
                     this.log.error('Error managing packages with Poetry', e);
                     setImmediate(async () => {
-                        const result = await window.showErrorMessage('Error managing packages with Poetry', 'View Output');
+                        const result = await showErrorMessage('Error managing packages with Poetry', 'View Output');
                         if (result === 'View Output') {
                             this.log.show();
                         }
@@ -109,23 +116,34 @@ export class PoetryPackageManager implements PackageManager, Disposable {
     }
 
     async refresh(environment: PythonEnvironment): Promise<void> {
-        await window.withProgress(
+        await withProgress(
             {
                 location: ProgressLocation.Window,
                 title: 'Refreshing Poetry packages',
             },
             async () => {
-                const before = this.packages.get(environment.envId.id) ?? [];
-                const after = await this.refreshPackages(environment);
-                const changes = getChanges(before, after);
-                this.packages.set(environment.envId.id, after);
-                if (changes.length > 0) {
-                    this._onDidChangePackages.fire({ environment, manager: this, changes });
+                try {
+                    const before = this.packages.get(environment.envId.id) ?? [];
+                    const after = await this.refreshPackages(environment);
+                    const changes = getChanges(before, after);
+                    this.packages.set(environment.envId.id, after);
+                    if (changes.length > 0) {
+                        this._onDidChangePackages.fire({ environment, manager: this, changes });
+                    }
+                } catch (error) {
+                    this.log.error(`Failed to refresh packages: ${error}`);
+                    // Show error to user but don't break the UI
+                    setImmediate(async () => {
+                        const result = await showErrorMessage('Error refreshing Poetry packages', 'View Output');
+                        if (result === 'View Output') {
+                            this.log.show();
+                        }
+                    });
                 }
             },
         );
     }
-    
+
     async getPackages(environment: PythonEnvironment): Promise<Package[] | undefined> {
         if (!this.packages.has(environment.envId.id)) {
             await this.refresh(environment);
@@ -141,16 +159,14 @@ export class PoetryPackageManager implements PackageManager, Disposable {
     private async managePackages(
         environment: PythonEnvironment,
         options: { install?: string[]; uninstall?: string[] },
-        token?: CancellationToken
+        token?: CancellationToken,
     ): Promise<Package[]> {
-        const cwd = environment.environmentPath.fsPath;
-
         // Handle uninstalls first
         if (options.uninstall && options.uninstall.length > 0) {
             try {
                 const args = ['remove', ...options.uninstall];
                 this.log.info(`Running: poetry ${args.join(' ')}`);
-                const result = await runPoetry(args, cwd, this.log, token);
+                const result = await runPoetry(args, undefined, this.log, token);
                 this.log.info(result);
             } catch (err) {
                 this.log.error(`Error removing packages with Poetry: ${err}`);
@@ -163,7 +179,7 @@ export class PoetryPackageManager implements PackageManager, Disposable {
             try {
                 const args = ['add', ...options.install];
                 this.log.info(`Running: poetry ${args.join(' ')}`);
-                const result = await runPoetry(args, cwd, this.log, token);
+                const result = await runPoetry(args, undefined, this.log, token);
                 this.log.info(result);
             } catch (err) {
                 this.log.error(`Error adding packages with Poetry: ${err}`);
@@ -176,12 +192,12 @@ export class PoetryPackageManager implements PackageManager, Disposable {
     }
 
     private async refreshPackages(environment: PythonEnvironment): Promise<Package[]> {
-        const cwd = environment.environmentPath.fsPath;
         const poetryPackages: { name: string; version: string; displayName: string; description: string }[] = [];
 
         try {
-            const result = await runPoetry(['show', '--no-ansi'], cwd, this.log);
-            
+            this.log.info(`Running: ${await getPoetry()} show --no-ansi`);
+            const result = await runPoetry(['show', '--no-ansi'], undefined, this.log);
+
             // Parse poetry show output
             // Format: name         version    description
             const lines = result.split('\n');
@@ -199,28 +215,30 @@ export class PoetryPackageManager implements PackageManager, Disposable {
             }
         } catch (err) {
             this.log.error(`Error refreshing packages with Poetry: ${err}`);
-            throw err;
+            // Return empty array instead of throwing to avoid breaking the UI
+            return [];
         }
 
         // Convert to Package objects using the API
-        return poetryPackages.map(pkg => this.api.createPackageItem(pkg, environment, this));
+        return poetryPackages.map((pkg) => this.api.createPackageItem(pkg, environment, this));
     }
 }
 
 export async function runPoetry(
     args: string[],
     cwd?: string,
     log?: LogOutputChannel,
-    token?: CancellationToken
+    token?: CancellationToken,
 ): Promise<string> {
     const poetry = await getPoetry();
     if (!poetry) {
         throw new Error('Poetry executable not found');
     }
-    
+
     log?.info(`Running: ${poetry} ${args.join(' ')}`);
+
     return new Promise<string>((resolve, reject) => {
-        const proc = ch.spawn(poetry, args, { cwd: cwd });
+        const proc = ch.spawn(poetry, args, { cwd });
         token?.onCancellationRequested(() => {
             proc.kill();
             reject(new CancellationError());
@@ -239,6 +257,10 @@ export async function runPoetry(
         proc.on('close', () => {
             resolve(builder);
         });
+        proc.on('error', (error) => {
+            log?.error(`Error executing poetry command: ${error}`);
+            reject(error);
+        });
         proc.on('exit', (code) => {
             if (code !== 0) {
                 reject(new Error(`Failed to run poetry ${args.join(' ')}`));
diff --git a/src/managers/poetry/poetryUtils.ts b/src/managers/poetry/poetryUtils.ts
