Skip to content

Commit eb06b79

Browse files
committed
Wait for install to finish to get results.
1 parent 3aab92a commit eb06b79

File tree

3 files changed

+34
-8
lines changed

3 files changed

+34
-8
lines changed

extensions/positron-python/src/client/common/application/commands/installPackages.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ export class InstallPackagesCommandHandler implements IExtensionSingleActivation
6161
// 3. Detailed per-package feedback helps the Assistant make better decisions
6262
for (const packageName of packages) {
6363
try {
64-
await installer.installModule(packageName, undefined, undefined, ModuleInstallFlags.none, undefined);
64+
await installer.installModule(packageName, undefined, undefined, ModuleInstallFlags.none, {
65+
waitForCompletion: true,
66+
});
6567
results.push(`${packageName} installed successfully using ${installer.displayName}`);
6668
} catch (error) {
6769
const errorMsg = error instanceof Error ? error.message : String(error);

extensions/positron-python/src/client/common/installer/moduleInstaller.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { IModuleInstaller, InstallOptions, InterpreterUri, ModuleInstallFlags }
2929
// --- Start Positron ---
3030
// eslint-disable-next-line import/newline-after-import
3131
import { IWorkspaceService } from '../application/types';
32-
class ExternallyManagedEnvironmentError extends Error {}
32+
class ExternallyManagedEnvironmentError extends Error { }
3333
// --- End Positron ---
3434

3535
@injectable()
@@ -42,7 +42,15 @@ export abstract class ModuleInstaller implements IModuleInstaller {
4242

4343
public abstract get type(): ModuleInstallerType;
4444

45-
constructor(protected serviceContainer: IServiceContainer) {}
45+
// --- Start Positron ---
46+
// This is a temporary flag to allow for the installation to wait for
47+
// completion. It can be used to override the behavior that the
48+
// python.installModulesInTerminal setting typically controls. This is used
49+
// when installing packages using the assistant to make sure it knows how
50+
// the installation went.
51+
private _waitForCompletion?: boolean;
52+
// --- End Positron ---
53+
constructor(protected serviceContainer: IServiceContainer) { }
4654

4755
public async installModule(
4856
productOrModuleName: Product | string,
@@ -53,6 +61,12 @@ export abstract class ModuleInstaller implements IModuleInstaller {
5361
): Promise<void> {
5462
// --- Start Positron ---
5563
const shouldExecuteInTerminal = this.installModulesInTerminal() || !options?.installAsProcess;
64+
// Store the waitForCompletion option so that we can use it in the
65+
// executeCommand method. This is temporary and is immediately unset
66+
// (before any awaits) in the executeCommand method which takes place in
67+
// a single call stack, thus it's not at risk for race conditions with
68+
// other calls to installModule..
69+
this._waitForCompletion = options?.waitForCompletion;
5670
// --- End Positron ---
5771
const name =
5872
typeof productOrModuleName === 'string'
@@ -159,7 +173,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
159173
if (ex instanceof ExternallyManagedEnvironmentError) {
160174
traceWarn(
161175
`Failed to install ${name} in ${resource?.path} because it is an ` +
162-
`externally-managed environment. Retrying with the --break-system-packages flag.`,
176+
`externally-managed environment. Retrying with the --break-system-packages flag.`,
163177
);
164178
await _install(token, (flags ?? ModuleInstallFlags.none) | ModuleInstallFlags.breakSystemPackages);
165179
} else {
@@ -259,13 +273,20 @@ export abstract class ModuleInstaller implements IModuleInstaller {
259273
.getTerminalService(options);
260274

261275
// --- Start Positron ---
262-
// When running with the `python.installModulesInTerminal` setting enabled, we want to
263-
// ensure that the terminal command is fully executed before returning. Otherwise, the
264-
// calling code of the install will not be able to tell when the installation is complete.
265-
if (this.installModulesInTerminal()) {
276+
// When running with the `python.installModulesInTerminal` setting
277+
// enabled or when the `waitForCompletion` option was set to true in
278+
// the parent `installModule` call, we want to ensure that the
279+
// terminal command is fully executed before returning. Otherwise,
280+
// the calling code of the install will not be able to tell when the
281+
// installation is complete.
282+
283+
if (this.installModulesInTerminal() || this._waitForCompletion) {
266284
// Ensure we pass a cancellation token so that we await the full terminal command
267285
// execution before returning.
268286
const cancelToken = token ?? new CancellationTokenSource().token;
287+
// Unset the waitForCompletion flag so that future calls are not blocked if not desired.
288+
// If a call needs to be blocked this will be set to true again in the installModule method.
289+
this._waitForCompletion = undefined;
269290
await terminalService.sendCommand(command, args, token ?? cancelToken);
270291
return;
271292
}

extensions/positron-python/src/client/common/installer/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,7 @@ export enum ModuleInstallFlags {
9090

9191
export type InstallOptions = {
9292
installAsProcess?: boolean;
93+
// --- Start Positron ---
94+
waitForCompletion?: boolean;
95+
// --- End Positron ---
9396
};

0 commit comments

Comments
 (0)