Skip to content

Commit c64b29d

Browse files
karthiknadigkarthiknadig
authored
Fix bugs found during TPI (#65)
Fixes #54 Fixes #53 Fixes #46
1 parent 40dbaaf commit c64b29d

File tree

6 files changed

+79
-25
lines changed

6 files changed

+79
-25
lines changed

‎src/common/pickers/packages.ts

+8-6
Original file line numberDiff line numberDiff line change
@@ -325,19 +325,21 @@ async function getCommonPackagesToInstall(
325325
export async function getPackagesToInstallFromPackageManager(
326326
packageManager: InternalPackageManager,
327327
environment: PythonEnvironment,
328+
cachedInstallables?: Installable[],
328329
): Promise<string[] | undefined> {
329-
const packageType = packageManager.supportsGetInstallable
330-
? await getPackageType()
331-
: PackageManagement.commonPackages;
330+
let installable: Installable[] = cachedInstallables ?? [];
331+
if (installable.length === 0 && packageManager.supportsGetInstallable) {
332+
installable = await getInstallables(packageManager, environment);
333+
}
334+
const packageType = installable.length > 0 ? await getPackageType() : PackageManagement.commonPackages;
332335

333336
if (packageType === PackageManagement.workspaceDependencies) {
334337
try {
335-
const installable = await getInstallables(packageManager, environment);
336338
const result = await getWorkspacePackages(installable);
337339
return result;
338340
} catch (ex) {
339341
if (packageManager.supportsGetInstallable && ex === QuickInputButtons.Back) {
340-
return getPackagesToInstallFromPackageManager(packageManager, environment);
342+
return getPackagesToInstallFromPackageManager(packageManager, environment, installable);
341343
}
342344
if (ex === QuickInputButtons.Back) {
343345
throw ex;
@@ -352,7 +354,7 @@ export async function getPackagesToInstallFromPackageManager(
352354
return result;
353355
} catch (ex) {
354356
if (packageManager.supportsGetInstallable && ex === QuickInputButtons.Back) {
355-
return getPackagesToInstallFromPackageManager(packageManager, environment);
357+
return getPackagesToInstallFromPackageManager(packageManager, environment, installable);
356358
}
357359
if (ex === QuickInputButtons.Back) {
358360
throw ex;

‎src/managers/conda/condaPackageManager.ts

+24-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
import { Disposable, Event, EventEmitter, LogOutputChannel, MarkdownString, ProgressLocation, window } from 'vscode';
1+
import {
2+
CancellationError,
3+
Disposable,
4+
Event,
5+
EventEmitter,
6+
LogOutputChannel,
7+
MarkdownString,
8+
ProgressLocation,
9+
window,
10+
} from 'vscode';
211
import {
312
DidChangePackagesEventArgs,
413
IconPath,
@@ -45,15 +54,20 @@ export class CondaPackageManager implements PackageManager, Disposable {
4554
{
4655
location: ProgressLocation.Notification,
4756
title: 'Installing packages',
57+
cancellable: true,
4858
},
49-
async () => {
59+
async (_progress, token) => {
5060
try {
5161
const before = this.packages.get(environment.envId.id) ?? [];
52-
const after = await installPackages(environment, packages, options, this.api, this);
62+
const after = await installPackages(environment, packages, options, this.api, this, token);
5363
const changes = getChanges(before, after);
5464
this.packages.set(environment.envId.id, after);
5565
this._onDidChangePackages.fire({ environment: environment, manager: this, changes });
5666
} catch (e) {
67+
if (e instanceof CancellationError) {
68+
return;
69+
}
70+
5771
this.log.error('Error installing packages', e);
5872
setImmediate(async () => {
5973
const result = await window.showErrorMessage('Error installing packages', 'View Output');
@@ -71,15 +85,20 @@ export class CondaPackageManager implements PackageManager, Disposable {
7185
{
7286
location: ProgressLocation.Notification,
7387
title: 'Uninstalling packages',
88+
cancellable: true,
7489
},
75-
async () => {
90+
async (_progress, token) => {
7691
try {
7792
const before = this.packages.get(environment.envId.id) ?? [];
78-
const after = await uninstallPackages(environment, packages, this.api, this);
93+
const after = await uninstallPackages(environment, packages, this.api, this, token);
7994
const changes = getChanges(before, after);
8095
this.packages.set(environment.envId.id, after);
8196
this._onDidChangePackages.fire({ environment: environment, manager: this, changes });
8297
} catch (e) {
98+
if (e instanceof CancellationError) {
99+
return;
100+
}
101+
83102
this.log.error('Error uninstalling packages', e);
84103
setImmediate(async () => {
85104
const result = await window.showErrorMessage('Error installing packages', 'View Output');

‎src/managers/conda/condaUtils.ts

+12-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
import * as path from 'path';
1313
import * as os from 'os';
1414
import * as fsapi from 'fs-extra';
15-
import { LogOutputChannel, ProgressLocation, Uri, window } from 'vscode';
15+
import { CancellationError, CancellationToken, LogOutputChannel, ProgressLocation, Uri, window } from 'vscode';
1616
import { ENVS_EXTENSION_ID } from '../../common/constants';
1717
import { createDeferred } from '../../common/utils/deferred';
1818
import {
@@ -121,12 +121,17 @@ export async function getConda(): Promise<string> {
121121
throw new Error('Conda not found');
122122
}
123123

124-
async function runConda(args: string[]): Promise<string> {
124+
async function runConda(args: string[], token?: CancellationToken): Promise<string> {
125125
const conda = await getConda();
126126

127127
const deferred = createDeferred<string>();
128128
const proc = ch.spawn(conda, args, { shell: true });
129129

130+
token?.onCancellationRequested(() => {
131+
proc.kill();
132+
deferred.reject(new CancellationError());
133+
});
134+
130135
let stdout = '';
131136
let stderr = '';
132137
proc.stdout?.on('data', (data) => {
@@ -591,7 +596,7 @@ export async function refreshPackages(
591596
name: parts[0],
592597
displayName: parts[0],
593598
version: parts[1],
594-
description: parts[2],
599+
description: parts[1],
595600
},
596601
environment,
597602
manager,
@@ -608,6 +613,7 @@ export async function installPackages(
608613
options: PackageInstallOptions,
609614
api: PythonEnvironmentApi,
610615
manager: PackageManager,
616+
token: CancellationToken,
611617
): Promise<Package[]> {
612618
if (!packages || packages.length === 0) {
613619
// TODO: Ask user to pick packages
@@ -620,7 +626,7 @@ export async function installPackages(
620626
}
621627
args.push(...packages);
622628

623-
await runConda(args);
629+
await runConda(args, token);
624630
return refreshPackages(environment, api, manager);
625631
}
626632

@@ -629,6 +635,7 @@ export async function uninstallPackages(
629635
packages: PackageInfo[] | string[],
630636
api: PythonEnvironmentApi,
631637
manager: PackageManager,
638+
token: CancellationToken,
632639
): Promise<Package[]> {
633640
const remove = [];
634641
for (let pkg of packages) {
@@ -642,7 +649,7 @@ export async function uninstallPackages(
642649
throw new Error('No packages to remove');
643650
}
644651

645-
await runConda(['remove', '--prefix', environment.environmentPath.fsPath, '--yes', ...remove]);
652+
await runConda(['remove', '--prefix', environment.environmentPath.fsPath, '--yes', ...remove], token);
646653

647654
return refreshPackages(environment, api, manager);
648655
}

‎src/managers/sysPython/pipManager.ts

+6-4
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,12 @@ export class PipPackageManager implements PackageManager, Disposable {
5959
{
6060
location: ProgressLocation.Notification,
6161
title: 'Installing packages',
62+
cancellable: true,
6263
},
63-
async () => {
64+
async (_progress, token) => {
6465
try {
6566
const before = this.packages.get(environment.envId.id) ?? [];
66-
const after = await installPackages(environment, packages, options, this.api, this);
67+
const after = await installPackages(environment, packages, options, this.api, this, token);
6768
const changes = getChanges(before, after);
6869
this.packages.set(environment.envId.id, after);
6970
this._onDidChangePackages.fire({ environment, manager: this, changes });
@@ -85,11 +86,12 @@ export class PipPackageManager implements PackageManager, Disposable {
8586
{
8687
location: ProgressLocation.Notification,
8788
title: 'Uninstalling packages',
89+
cancellable: true,
8890
},
89-
async () => {
91+
async (_progress, token) => {
9092
try {
9193
const before = this.packages.get(environment.envId.id) ?? [];
92-
const after = await uninstallPackages(environment, this.api, this, packages);
94+
const after = await uninstallPackages(environment, this.api, this, packages, token);
9395
const changes = getChanges(before, after);
9496
this.packages.set(environment.envId.id, after);
9597
this._onDidChangePackages.fire({ environment: environment, manager: this, changes });

‎src/managers/sysPython/utils.ts

+29-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from 'path';
2-
import { LogOutputChannel, QuickPickItem, Uri, window } from 'vscode';
2+
import { CancellationError, CancellationToken, LogOutputChannel, QuickPickItem, Uri, window } from 'vscode';
33
import {
44
EnvironmentManager,
55
Package,
@@ -110,10 +110,20 @@ export async function isUvInstalled(log?: LogOutputChannel): Promise<boolean> {
110110
return available.promise;
111111
}
112112

113-
export async function runUV(args: string[], cwd?: string, log?: LogOutputChannel): Promise<string> {
113+
export async function runUV(
114+
args: string[],
115+
cwd?: string,
116+
log?: LogOutputChannel,
117+
token?: CancellationToken,
118+
): Promise<string> {
114119
log?.info(`Running: uv ${args.join(' ')}`);
115120
return new Promise<string>((resolve, reject) => {
116121
const proc = ch.spawn('uv', args, { cwd: cwd });
122+
token?.onCancellationRequested(() => {
123+
proc.kill();
124+
reject(new CancellationError());
125+
});
126+
117127
let builder = '';
118128
proc.stdout?.on('data', (data) => {
119129
const s = data.toString('utf-8');
@@ -134,10 +144,20 @@ export async function runUV(args: string[], cwd?: string, log?: LogOutputChannel
134144
});
135145
}
136146

137-
export async function runPython(python: string, args: string[], cwd?: string, log?: LogOutputChannel): Promise<string> {
147+
export async function runPython(
148+
python: string,
149+
args: string[],
150+
cwd?: string,
151+
log?: LogOutputChannel,
152+
token?: CancellationToken,
153+
): Promise<string> {
138154
log?.info(`Running: ${python} ${args.join(' ')}`);
139155
return new Promise<string>((resolve, reject) => {
140156
const proc = ch.spawn(python, args, { cwd: cwd });
157+
token?.onCancellationRequested(() => {
158+
proc.kill();
159+
reject(new CancellationError());
160+
});
141161
let builder = '';
142162
proc.stdout?.on('data', (data) => {
143163
const s = data.toString('utf-8');
@@ -312,6 +332,7 @@ export async function installPackages(
312332
options: PackageInstallOptions,
313333
api: PythonEnvironmentApi,
314334
manager: PackageManager,
335+
token?: CancellationToken,
315336
): Promise<Package[]> {
316337
if (environment.version.startsWith('2.')) {
317338
throw new Error('Python 2.* is not supported (deprecated)');
@@ -333,13 +354,15 @@ export async function installPackages(
333354
[...installArgs, '--python', environment.execInfo.run.executable, ...packages],
334355
undefined,
335356
manager.log,
357+
token,
336358
);
337359
} else {
338360
await runPython(
339361
environment.execInfo.run.executable,
340362
['-m', ...installArgs, ...packages],
341363
undefined,
342364
manager.log,
365+
token,
343366
);
344367
}
345368

@@ -353,6 +376,7 @@ export async function uninstallPackages(
353376
api: PythonEnvironmentApi,
354377
manager: PackageManager,
355378
packages: string[] | Package[],
379+
token?: CancellationToken,
356380
): Promise<Package[]> {
357381
if (environment.version.startsWith('2.')) {
358382
throw new Error('Python 2.* is not supported (deprecated)');
@@ -383,13 +407,15 @@ export async function uninstallPackages(
383407
['pip', 'uninstall', '--python', environment.execInfo.run.executable, ...remove],
384408
undefined,
385409
manager.log,
410+
token,
386411
);
387412
} else {
388413
await runPython(
389414
environment.execInfo.run.executable,
390415
['-m', 'pip', 'uninstall', '-y', ...remove],
391416
undefined,
392417
manager.log,
418+
token,
393419
);
394420
}
395421
return refreshPackages(environment, api, manager);

‎src/managers/sysPython/venvUtils.ts

-2
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ export async function createPythonVenv(
273273
const basePython = await pickEnvironmentFrom(sortEnvironments(filtered));
274274
if (!basePython || !basePython.execInfo) {
275275
log.error('No base python selected, cannot create virtual environment.');
276-
showErrorMessage('No base python selected, cannot create virtual environment.');
277276
return;
278277
}
279278

@@ -298,7 +297,6 @@ export async function createPythonVenv(
298297
});
299298
if (!name) {
300299
log.error('No name entered, cannot create virtual environment.');
301-
showErrorMessage('No name entered, cannot create virtual environment.');
302300
return;
303301
}
304302

0 commit comments

Comments
 (0)