From be2d1903c923fcb45ac53b1ac25b00ad9a1e60db Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 4 May 2021 10:50:38 -0700 Subject: [PATCH] Support (Jupyter) to pass additional install args --- news/3 Code Health/16131.md | 1 + src/client/common/installer/condaInstaller.ts | 12 +++++++--- .../common/installer/moduleInstaller.ts | 8 +++---- .../common/installer/pipEnvInstaller.ts | 11 ++++++--- src/client/common/installer/pipInstaller.ts | 20 +++++++++++----- .../common/installer/productInstaller.ts | 23 ++++++++++--------- src/client/common/installer/types.ts | 10 ++++++-- src/client/common/types.ts | 6 ++--- src/client/jupyter/jupyterIntegration.ts | 13 +++++++++-- src/client/tensorBoard/tensorBoardSession.ts | 5 +++- .../installer/moduleInstaller.unit.test.ts | 22 ++++++++++++++---- .../tensorBoard/tensorBoardSession.test.ts | 3 ++- 12 files changed, 93 insertions(+), 41 deletions(-) create mode 100644 news/3 Code Health/16131.md diff --git a/news/3 Code Health/16131.md b/news/3 Code Health/16131.md new file mode 100644 index 000000000000..72682ea66850 --- /dev/null +++ b/news/3 Code Health/16131.md @@ -0,0 +1 @@ +Add ability for Jupyter extension to pass addtional installer arguments. diff --git a/src/client/common/installer/condaInstaller.ts b/src/client/common/installer/condaInstaller.ts index 3653bbc18ba0..e09737a1abcd 100644 --- a/src/client/common/installer/condaInstaller.ts +++ b/src/client/common/installer/condaInstaller.ts @@ -9,7 +9,7 @@ import { inDiscoveryExperiment } from '../experiments/helpers'; import { ExecutionInfo, IConfigurationService, IExperimentService } from '../types'; import { isResource } from '../utils/misc'; import { ModuleInstaller } from './moduleInstaller'; -import { InterpreterUri } from './types'; +import { InterpreterUri, ModuleInstallFlags } from './types'; /** * A Python module installer for a conda environment. @@ -64,7 +64,7 @@ export class CondaInstaller extends ModuleInstaller { protected async getExecutionInfo( moduleName: string, resource?: InterpreterUri, - isUpgrade?: boolean, + flags: ModuleInstallFlags = 0, ): Promise { const condaService = this.serviceContainer.get(ICondaService); const condaFile = await condaService.getCondaFile(); @@ -77,7 +77,7 @@ export class CondaInstaller extends ModuleInstaller { ? this.serviceContainer.get(IComponentAdapter) : this.serviceContainer.get(ICondaLocatorService); const info = await condaLocatorService.getCondaEnvironment(pythonPath); - const args = [isUpgrade ? 'update' : 'install']; + const args = [flags & ModuleInstallFlags.upgrade ? 'update' : 'install']; // Temporarily ensure tensorboard is installed from the conda-forge // channel since 2.4.1 is not yet available in the default index @@ -93,6 +93,12 @@ export class CondaInstaller extends ModuleInstaller { args.push('--prefix'); args.push(info.path.fileToCommandArgument()); } + if (flags & ModuleInstallFlags.updateDependencies) { + args.push('--update-deps'); + } + if (flags & ModuleInstallFlags.reInstall) { + args.push('--force-reinstall'); + } args.push(moduleName); args.push('-y'); return { diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index ec0ea881294e..45e517b3540d 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -19,7 +19,7 @@ import { ExecutionInfo, IConfigurationService, IOutputChannel, ModuleNamePurpose import { Products } from '../utils/localize'; import { isResource } from '../utils/misc'; import { ProductNames } from './productNames'; -import { IModuleInstaller, InterpreterUri } from './types'; +import { IModuleInstaller, InterpreterUri, ModuleInstallFlags } from './types'; @injectable() export abstract class ModuleInstaller implements IModuleInstaller { @@ -33,7 +33,7 @@ export abstract class ModuleInstaller implements IModuleInstaller { productOrModuleName: Product | string, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { const name = typeof productOrModuleName == 'string' @@ -48,7 +48,7 @@ export abstract class ModuleInstaller implements IModuleInstaller { } else { options.interpreter = resource; } - const executionInfo = await this.getExecutionInfo(name, resource, isUpgrade); + const executionInfo = await this.getExecutionInfo(name, resource, flags); const terminalService = this.serviceContainer .get(ITerminalServiceFactory) .getTerminalService(options); @@ -132,7 +132,7 @@ export abstract class ModuleInstaller implements IModuleInstaller { protected abstract getExecutionInfo( moduleName: string, resource?: InterpreterUri, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; private async processInstallArgs(args: string[], resource?: InterpreterUri): Promise { const indexOfPylint = args.findIndex((arg) => arg.toUpperCase() === 'PYLINT'); diff --git a/src/client/common/installer/pipEnvInstaller.ts b/src/client/common/installer/pipEnvInstaller.ts index d479779688d0..621988417cc7 100644 --- a/src/client/common/installer/pipEnvInstaller.ts +++ b/src/client/common/installer/pipEnvInstaller.ts @@ -11,7 +11,7 @@ import { inDiscoveryExperiment } from '../experiments/helpers'; import { ExecutionInfo, IExperimentService } from '../types'; import { isResource } from '../utils/misc'; import { ModuleInstaller } from './moduleInstaller'; -import { InterpreterUri } from './types'; +import { InterpreterUri, ModuleInstallFlags } from './types'; export const pipenvName = 'pipenv'; @@ -61,9 +61,14 @@ export class PipEnvInstaller extends ModuleInstaller { protected async getExecutionInfo( moduleName: string, _resource?: InterpreterUri, - isUpgrade?: boolean, + flags: ModuleInstallFlags = 0, ): Promise { - const args = [isUpgrade ? 'update' : 'install', moduleName, '--dev']; + // In pipenv the only way to update/upgrade or re-install is update (apart from a complete uninstall and re-install). + const update = + flags & ModuleInstallFlags.reInstall || + flags & ModuleInstallFlags.updateDependencies || + flags & ModuleInstallFlags.upgrade; + const args = [update ? 'update' : 'install', moduleName, '--dev']; if (moduleName === 'black') { args.push('--pre'); } diff --git a/src/client/common/installer/pipInstaller.ts b/src/client/common/installer/pipInstaller.ts index b2d13a26e03a..de4ddbaa73dc 100644 --- a/src/client/common/installer/pipInstaller.ts +++ b/src/client/common/installer/pipInstaller.ts @@ -8,7 +8,7 @@ import { IPythonExecutionFactory } from '../process/types'; import { ExecutionInfo } from '../types'; import { isResource } from '../utils/misc'; import { ModuleInstaller } from './moduleInstaller'; -import { InterpreterUri } from './types'; +import { InterpreterUri, ModuleInstallFlags } from './types'; @injectable() export class PipInstaller extends ModuleInstaller { @@ -28,16 +28,24 @@ export class PipInstaller extends ModuleInstaller { public isSupported(resource?: InterpreterUri): Promise { return this.isPipAvailable(resource); } - protected async getExecutionInfo(moduleName: string, _resource?: InterpreterUri): Promise { - const proxyArgs: string[] = []; + protected async getExecutionInfo( + moduleName: string, + _resource?: InterpreterUri, + flags: ModuleInstallFlags = 0, + ): Promise { + const args: string[] = []; const workspaceService = this.serviceContainer.get(IWorkspaceService); const proxy = workspaceService.getConfiguration('http').get('proxy', ''); if (proxy.length > 0) { - proxyArgs.push('--proxy'); - proxyArgs.push(proxy); + args.push('--proxy'); + args.push(proxy); + } + args.push(...['install', '-U']); + if (flags & ModuleInstallFlags.reInstall) { + args.push('--force-reinstall'); } return { - args: [...proxyArgs, 'install', '-U', moduleName], + args: [...args, moduleName], moduleName: 'pip', }; } diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 7b9a73e5f583..fd65c2d725af 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -36,6 +36,7 @@ import { InterpreterUri, IProductPathService, IProductService, + ModuleInstallFlags, } from './types'; export { Product } from '../types'; @@ -72,7 +73,7 @@ abstract class BaseInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { // If this method gets called twice, while previous promise has not been resolved, then return that same promise. // E.g. previous promise is not resolved as a message has been displayed to the user, so no point displaying @@ -83,7 +84,7 @@ abstract class BaseInstaller { if (BaseInstaller.PromptPromises.has(key)) { return BaseInstaller.PromptPromises.get(key)!; } - const promise = this.promptToInstallImplementation(product, resource, cancel, isUpgrade); + const promise = this.promptToInstallImplementation(product, resource, cancel, flags); BaseInstaller.PromptPromises.set(key, promise); promise.then(() => BaseInstaller.PromptPromises.delete(key)).ignoreErrors(); promise.catch(() => BaseInstaller.PromptPromises.delete(key)).ignoreErrors(); @@ -95,7 +96,7 @@ abstract class BaseInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { if (product === Product.unittest) { return InstallerResponse.Installed; @@ -112,7 +113,7 @@ abstract class BaseInstaller { } await installer - .installModule(product, resource, cancel, isUpgrade) + .installModule(product, resource, cancel, flags) .catch((ex) => traceError(`Error in installing the product '${ProductNames.get(product)}', ${ex}`)); return this.isInstalled(product, resource).then((isInstalled) => { @@ -207,7 +208,7 @@ abstract class BaseInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { @@ -349,7 +350,7 @@ class DataScienceInstaller extends BaseInstaller { product: Product, interpreterUri?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { // Precondition if (isResource(interpreterUri)) { @@ -390,7 +391,7 @@ class DataScienceInstaller extends BaseInstaller { } await installerModule - .installModule(product, interpreter, cancel, isUpgrade) + .installModule(product, interpreter, cancel, flags) .catch((ex) => traceError(`Error in installing the module '${moduleName}', ${ex}`)); return this.isInstalled(product, interpreter).then((isInstalled) => { @@ -447,7 +448,7 @@ export class ProductInstaller implements IInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { const currentInterpreter = isResource(resource) ? await this.interpreterService.getActiveInterpreter(resource) @@ -455,7 +456,7 @@ export class ProductInstaller implements IInstaller { if (!currentInterpreter) { return InstallerResponse.Ignore; } - return this.createInstaller(product).promptToInstall(product, resource, cancel, isUpgrade); + return this.createInstaller(product).promptToInstall(product, resource, cancel, flags); } public async isProductVersionCompatible( @@ -470,9 +471,9 @@ export class ProductInstaller implements IInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise { - return this.createInstaller(product).install(product, resource, cancel, isUpgrade); + return this.createInstaller(product).install(product, resource, cancel, flags); } public async isInstalled(product: Product, resource?: InterpreterUri): Promise { diff --git a/src/client/common/installer/types.ts b/src/client/common/installer/types.ts index 3b567a1efa50..c06b73b010aa 100644 --- a/src/client/common/installer/types.ts +++ b/src/client/common/installer/types.ts @@ -27,7 +27,7 @@ export interface IModuleInstaller { product: string, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; /** * Installs a Product @@ -44,7 +44,7 @@ export interface IModuleInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; isSupported(resource?: InterpreterUri): Promise; } @@ -76,3 +76,9 @@ export const IExtensionBuildInstaller = Symbol('IExtensionBuildInstaller'); export interface IExtensionBuildInstaller { install(): Promise; } + +export enum ModuleInstallFlags { + upgrade = 1, + updateDependencies = 2, + reInstall = 4, +} diff --git a/src/client/common/types.ts b/src/client/common/types.ts index f9cf4e09db3d..27e4d00ab54b 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -21,7 +21,7 @@ import { import { LanguageServerType } from '../activation/types'; import { LogLevel } from '../logging/levels'; import type { ExtensionChannels } from './insidersBuild/types'; -import type { InterpreterUri } from './installer/types'; +import type { InterpreterUri, ModuleInstallFlags } from './installer/types'; import { EnvironmentVariables } from './variables/types'; import { ITestingSettings } from '../testing/configuration/types'; @@ -122,13 +122,13 @@ export interface IInstaller { product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; install( product: Product, resource?: InterpreterUri, cancel?: CancellationToken, - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ): Promise; isInstalled(product: Product, resource?: InterpreterUri): Promise; isProductVersionCompatible( diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts index c6cec568374d..7362f8134655 100644 --- a/src/client/jupyter/jupyterIntegration.ts +++ b/src/client/jupyter/jupyterIntegration.ts @@ -10,7 +10,7 @@ import { CancellationToken, Disposable, Event, Extension, Memento, Uri } from 'v import * as lsp from 'vscode-languageserver-protocol'; import { ILanguageServerCache, ILanguageServerConnection } from '../activation/types'; import { JUPYTER_EXTENSION_ID } from '../common/constants'; -import { InterpreterUri } from '../common/installer/types'; +import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types'; import { GLOBAL_MEMENTO, IExperimentService, @@ -179,7 +179,16 @@ export class JupyterExtensionIntegration { product: JupyterProductToInstall, resource?: InterpreterUri, cancel?: CancellationToken, - ): Promise => this.installer.install(ProductMapping[product], resource, cancel), + reInstallAndUpdate?: boolean, + ): Promise => + this.installer.install( + ProductMapping[product], + resource, + cancel, + reInstallAndUpdate === true + ? ModuleInstallFlags.updateDependencies | ModuleInstallFlags.reInstall + : undefined, + ), getDebuggerPath: async () => dirname(getDebugpyPackagePath()), getInterpreterPathSelectedForJupyterServer: () => this.globalState.get('INTERPRETER_PATH_SELECTED_FOR_JUPYTER_SERVER'), diff --git a/src/client/tensorBoard/tensorBoardSession.ts b/src/client/tensorBoard/tensorBoardSession.ts index 36e45fdc40d6..8f87a6a802ad 100644 --- a/src/client/tensorBoard/tensorBoardSession.ts +++ b/src/client/tensorBoard/tensorBoardSession.ts @@ -45,6 +45,7 @@ import { EventName } from '../telemetry/constants'; import { ImportTracker } from '../telemetry/importTracker'; import { TensorBoardPromptSelection, TensorBoardSessionStartResult } from './constants'; import { IMultiStepInputFactory } from '../common/utils/multiStepInput'; +import { ModuleInstallFlags } from '../common/installer/types'; enum Messages { JumpToSource = 'jump_to_source', @@ -236,7 +237,9 @@ export class TensorBoardSession { Product.tensorboard, interpreter, installerToken, - tensorboardInstallStatus === ProductInstallStatus.NeedsUpgrade, + tensorboardInstallStatus === ProductInstallStatus.NeedsUpgrade + ? ModuleInstallFlags.upgrade + : undefined, ), ); } diff --git a/src/test/common/installer/moduleInstaller.unit.test.ts b/src/test/common/installer/moduleInstaller.unit.test.ts index 9700c354859a..53ec6c219a8f 100644 --- a/src/test/common/installer/moduleInstaller.unit.test.ts +++ b/src/test/common/installer/moduleInstaller.unit.test.ts @@ -27,7 +27,11 @@ import { ModuleInstaller } from '../../../client/common/installer/moduleInstalle import { PipEnvInstaller, pipenvName } from '../../../client/common/installer/pipEnvInstaller'; import { PipInstaller } from '../../../client/common/installer/pipInstaller'; import { ProductInstaller } from '../../../client/common/installer/productInstaller'; -import { IInstallationChannelManager, IModuleInstaller } from '../../../client/common/installer/types'; +import { + IInstallationChannelManager, + IModuleInstaller, + ModuleInstallFlags, +} from '../../../client/common/installer/types'; import { IFileSystem } from '../../../client/common/platform/types'; import { ITerminalService, ITerminalServiceFactory } from '../../../client/common/terminal/types'; import { @@ -340,7 +344,7 @@ suite('Module Installer', () => { async function installModuleAndVerifyCommand( command: string, expectedArgs: string[], - isUpgrade?: boolean, + flags?: ModuleInstallFlags, ) { terminalService .setup((t) => @@ -353,7 +357,7 @@ suite('Module Installer', () => { .returns(() => Promise.resolve()) .verifiable(TypeMoq.Times.once()); - await installer.installModule(product.value, resource, undefined, isUpgrade); + await installer.installModule(product.value, resource, undefined, flags); terminalService.verifyAll(); } @@ -604,7 +608,11 @@ suite('Module Installer', () => { if (moduleName === 'black') { expectedArgs.push('--pre'); } - await installModuleAndVerifyCommand(pipenvName, expectedArgs, isUpgrade); + await installModuleAndVerifyCommand( + pipenvName, + expectedArgs, + isUpgrade ? ModuleInstallFlags.upgrade : undefined, + ); }); }); } @@ -625,7 +633,11 @@ suite('Module Installer', () => { } expectedArgs.push(moduleName); expectedArgs.push('-y'); - await installModuleAndVerifyCommand(condaExecutable, expectedArgs, isUpgrade); + await installModuleAndVerifyCommand( + condaExecutable, + expectedArgs, + isUpgrade ? ModuleInstallFlags.upgrade : undefined, + ); }); }); } diff --git a/src/test/tensorBoard/tensorBoardSession.test.ts b/src/test/tensorBoard/tensorBoardSession.test.ts index 3389da5777f9..3926b04021c1 100644 --- a/src/test/tensorBoard/tensorBoardSession.test.ts +++ b/src/test/tensorBoard/tensorBoardSession.test.ts @@ -24,6 +24,7 @@ import { PYTHON_PATH } from '../common'; import { TorchProfiler } from '../../client/common/experiments/groups'; import { ImportTracker } from '../../client/telemetry/importTracker'; import { IMultiStepInput, IMultiStepInputFactory } from '../../client/common/utils/multiStepInput'; +import { ModuleInstallFlags } from '../../client/common/installer/types'; // Class methods exposed just for testing purposes interface ITensorBoardSessionTestAPI { @@ -176,7 +177,7 @@ suite('TensorBoard session creation', async () => { Product.tensorboard, sinon.match.any, sinon.match.any, - expectTensorBoardUpgrade, + expectTensorBoardUpgrade ? ModuleInstallFlags.upgrade : undefined, ), ); }