From e2a585915531fa8660374dae93b0f4133adafd52 Mon Sep 17 00:00:00 2001 From: Cory Rivera Date: Mon, 8 Mar 2021 18:05:10 -0800 Subject: [PATCH] Only include package versions in Manage Packages dialog if they're supported for the user's version of Python (#14584) --- extensions/notebook/src/common/utils.ts | 85 +++++++++++++ .../src/jupyter/jupyterServerInstallation.ts | 13 ++ .../localCondaPackageManageProvider.ts | 15 ++- .../jupyter/localPipPackageManageProvider.ts | 10 +- .../notebook/src/test/common/utils.test.ts | 113 ++++++++++++++++++ .../localPackageManageProvider.test.ts | 25 ++-- 6 files changed, 245 insertions(+), 16 deletions(-) diff --git a/extensions/notebook/src/common/utils.ts b/extensions/notebook/src/common/utils.ts index 707f19fb58..428266ef52 100644 --- a/extensions/notebook/src/common/utils.ts +++ b/extensions/notebook/src/common/utils.ts @@ -153,6 +153,11 @@ export function comparePackageVersions(first: string, second: string): number { } for (let i = 0; i < firstVersion.length; ++i) { + // Using asterisks means any version number is equivalent, so skip this value + if (firstVersion[i] === '*' || secondVersion[i] === '*') { + continue; + } + let firstVersionNum: string | number = Number(firstVersion[i]); let secondVersionNum: string | number = Number(secondVersion[i]); @@ -182,6 +187,86 @@ export function sortPackageVersions(versions: string[], ascending: boolean = tru }); } +const specifierFirstCharMatch = /[><=!]/; + +// Determines if a given package is supported for the provided version of Python +// using the version constraints from the pypi metadata. +export function isPackageSupported(pythonVersion: string, packageVersionConstraints: string[]): boolean { + if (pythonVersion === '') { + return true; + } + + // Version constraint strings are formatted like '!=2.7, >=3.5, >=3.6', + // with each package release having its own set of version constraints. + let supportedVersionFound = true; + for (let packageVersionConstraint of packageVersionConstraints) { + if (!packageVersionConstraint) { + continue; + } + + let constraintParts = packageVersionConstraint.split(','); + for (let constraint of constraintParts) { + constraint = constraint.trim(); + if (constraint.length === 0) { + continue; + } + + let splitIndex: number; + if (!constraint[0].match(specifierFirstCharMatch)) { + splitIndex = -1; // No version specifier is included with this version number + } else if ((constraint[0] === '>' || constraint[0] === '<') && constraint[1] !== '=') { + splitIndex = 1; + } else { + splitIndex = 2; + } + + let versionSpecifier: string; + let version: string; + if (splitIndex === -1) { + versionSpecifier = '=='; // If there's no version specifier, then we need to match the version exactly + version = constraint; + } else { + versionSpecifier = constraint.slice(0, splitIndex); + version = constraint.slice(splitIndex).trim(); + } + let versionComparison = comparePackageVersions(pythonVersion, version); + switch (versionSpecifier) { + case '>=': + supportedVersionFound = versionComparison !== -1; + break; + case '<=': + supportedVersionFound = versionComparison !== 1; + break; + case '>': + supportedVersionFound = versionComparison === 1; + break; + case '<': + supportedVersionFound = versionComparison === -1; + break; + case '==': + supportedVersionFound = versionComparison === 0; + break; + case '!=': + supportedVersionFound = versionComparison !== 0; + break; + default: + // We hit an unexpected version specifier. Rather than throw an error here, we should + // let the package be installable so that we're not too restrictive by mistake. + // Trying to install the package will still throw its own unsupported version error later. + supportedVersionFound = true; // The package is tentatively supported until we find a constraint that fails + break; + } + if (!supportedVersionFound) { + break; // Failed at least one version check, so skip checking the other constraints + } + } + if (supportedVersionFound) { + break; // All constraints passed for this package, so we don't need to check any of the others + } + } + return supportedVersionFound; +} + export function isEditorTitleFree(title: string): boolean { let hasTextDoc = vscode.workspace.textDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title && !notebookLanguages.find(lang => lang === doc.languageId)) > -1; diff --git a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts index 47202a1bf1..2a3f3895aa 100644 --- a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts +++ b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts @@ -68,6 +68,7 @@ export interface IJupyterServerInstallation { uninstallPipPackages(packages: PythonPkgDetails[]): Promise; pythonExecutable: string; pythonInstallationPath: string; + installedPythonVersion: string; } export const requiredJupyterPkg: PythonPkgDetails = { @@ -107,6 +108,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { private _pythonExecutable: string; private _usingExistingPython: boolean; private _usingConda: boolean; + private _installedPythonVersion: string; private _installInProgress: boolean; private _installCompletion: Deferred; @@ -356,6 +358,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { if (pythonUserDir) { this.pythonEnvVarPath = pythonUserDir + delimiter + this.pythonEnvVarPath; } + this._installedPythonVersion = await this.getInstalledPythonVersion(this._pythonExecutable); } // Store the executable options to run child processes with env var without interfering parent env var. @@ -652,6 +655,10 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { return this._usingConda; } + public get installedPythonVersion(): string { + return this._installedPythonVersion; + } + private isCondaInstalled(): boolean { let condaExePath = this.getCondaExePath(); // eslint-disable-next-line no-sync @@ -740,6 +747,12 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { return undefined; } + private async getInstalledPythonVersion(pythonExecutable: string): Promise { + let cmd = `"${pythonExecutable}" -c "import platform;print(platform.python_version())"`; + let version = await utils.executeBufferedCommand(cmd, {}); + return version?.trim() ?? ''; + } + public getRequiredPackagesForKernel(kernelName: string): PythonPkgDetails[] { return this._requiredKernelPackages.get(kernelName) ?? []; } diff --git a/extensions/notebook/src/jupyter/localCondaPackageManageProvider.ts b/extensions/notebook/src/jupyter/localCondaPackageManageProvider.ts index bbb636a311..7c34342029 100644 --- a/extensions/notebook/src/jupyter/localCondaPackageManageProvider.ts +++ b/extensions/notebook/src/jupyter/localCondaPackageManageProvider.ts @@ -104,7 +104,20 @@ export class LocalCondaPackageManageProvider implements IPackageManageProvider { let packages = packageJson[packageName]; if (Array.isArray(packages)) { - let allVersions = packages.filter(pkg => pkg && pkg.version).map(pkg => pkg.version); + let allVersions = packages.filter(pkg => { + if (pkg && pkg.version) { + let dependencies = pkg.depends; + if (Array.isArray(dependencies)) { + let strDependencies = dependencies as string[]; + let pythonDependency = strDependencies.find(dependency => dependency.trim().toLowerCase().startsWith('python ')); + pythonDependency = pythonDependency?.replace('python ', ''); + return utils.isPackageSupported(this.jupyterInstallation.installedPythonVersion, [pythonDependency]); + } + return true; + } + return false; + }).map(pkg => pkg.version); + let singletonVersions = new Set(allVersions); let sortedVersions = utils.sortPackageVersions(Array.from(singletonVersions), false); return { diff --git a/extensions/notebook/src/jupyter/localPipPackageManageProvider.ts b/extensions/notebook/src/jupyter/localPipPackageManageProvider.ts index 72f776a629..9860e35433 100644 --- a/extensions/notebook/src/jupyter/localPipPackageManageProvider.ts +++ b/extensions/notebook/src/jupyter/localPipPackageManageProvider.ts @@ -18,7 +18,7 @@ export class LocalPipPackageManageProvider implements IPackageManageProvider { constructor( private jupyterInstallation: IJupyterServerInstallation, - private pipyClient: IPyPiClient) { + private pyPiClient: IPyPiClient) { } /** @@ -89,7 +89,7 @@ export class LocalPipPackageManageProvider implements IPackageManageProvider { } private async fetchPypiPackage(packageName: string): Promise { - let body = await this.pipyClient.fetchPypiPackage(packageName); + let body = await this.pyPiClient.fetchPypiPackage(packageName); let packagesJson = JSON.parse(body); let versionNums: string[] = []; let packageSummary = ''; @@ -106,7 +106,11 @@ export class LocalPipPackageManageProvider implements IPackageManageProvider { let versionKeys = Object.keys(packagesJson.releases); versionKeys = versionKeys.filter(versionKey => { let releaseInfo = packagesJson.releases[versionKey]; - return Array.isArray(releaseInfo) && releaseInfo.length > 0; + if (Array.isArray(releaseInfo) && releaseInfo.length > 0) { + let pythonVersionConstraints = releaseInfo.map(info => info.requires_python); + return utils.isPackageSupported(this.jupyterInstallation.installedPythonVersion, pythonVersionConstraints); + } + return false; }); versionNums = utils.sortPackageVersions(versionKeys, false); diff --git a/extensions/notebook/src/test/common/utils.test.ts b/extensions/notebook/src/test/common/utils.test.ts index 3623122bf5..66466b6edf 100644 --- a/extensions/notebook/src/test/common/utils.test.ts +++ b/extensions/notebook/src/test/common/utils.test.ts @@ -78,6 +78,22 @@ describe('Utils Tests', function () { it('correctly compares version with only minor version difference', () => { should(utils.comparePackageVersions(version1Revision, version1)).equal(1); }); + + it('equivalent versions with wildcard characters', () => { + should(utils.comparePackageVersions('1.*.3', '1.5.3')).equal(0); + }); + + it('lower version with wildcard characters', () => { + should(utils.comparePackageVersions('1.4.*', '1.5.3')).equal(-1); + }); + + it('higher version with wildcard characters', () => { + should(utils.comparePackageVersions('4.5.6', '3.*')).equal(1); + }); + + it('all wildcard strings should be equal', () => { + should(utils.comparePackageVersions('*.*', '*.*.*')).equal(0); + }); }); describe('sortPackageVersions', () => { @@ -139,6 +155,103 @@ describe('Utils Tests', function () { }); }); + describe('isPackageSupported', () => { + it('Constraints have no version specifier', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['3.6.*', '3.*']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + + versionConstraints = ['3.5.*', '3.5']; + result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.false(); + }); + + it('Package is valid for version constraints', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['>=3.5,!=3.2,!=3.4.*']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Version constraints string has lots of spaces', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['>= 3.5, != 3.2, != 3.4.*']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Strictly greater or less than comparisons', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['> 3.5, > 3.4.*', '< 3.8']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Strict equality', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['== 3.6', '== 3.6.*']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Package is valid for first set of constraints, but not the second', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['>=3.5, !=3.2, !=3.4.*', '!=3.6, >=3.5']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Package is valid for second set of constraints, but not the first', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['!=3.6, >=3.5', '>=3.5, !=3.2, !=3.4.*']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Package is not valid for constraints', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['>=3.4, !=3.6, >=3.5']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.false(); + }); + + it('Package is not valid for several sets of constraints', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['>=3.7', '!=3.6, >=3.5', '>=3.8']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.false(); + }); + + it('Constraints are all empty strings', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints = ['', '', '']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Constraints are all undefined', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints: string[] = [undefined, undefined, undefined]; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Constraints are a bunch of commas', async function (): Promise { + let pythonVersion = '3.6'; + let versionConstraints: string[] = [',,,', ',,,,', ', , , , , , ,']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + + it('Installed python version is an empty string', async function (): Promise { + let pythonVersion = ''; + let versionConstraints = ['>=3.7', '!=3.6, >=3.5', '>=3.8']; + let result = await utils.isPackageSupported(pythonVersion, versionConstraints); + should(result).be.true(); + }); + }); + describe('executeBufferedCommand', () => { it('runs successfully', async () => { diff --git a/extensions/notebook/src/test/managePackages/localPackageManageProvider.test.ts b/extensions/notebook/src/test/managePackages/localPackageManageProvider.test.ts index b69c460c2c..f7cd890fbb 100644 --- a/extensions/notebook/src/test/managePackages/localPackageManageProvider.test.ts +++ b/extensions/notebook/src/test/managePackages/localPackageManageProvider.test.ts @@ -14,7 +14,7 @@ import { IPyPiClient, PyPiClient } from '../../jupyter/pypiClient'; interface TestContext { serverInstallation: IJupyterServerInstallation; - piPyClient: IPyPiClient; + pyPiClient: IPyPiClient; } describe('Manage Package Providers', () => { @@ -29,7 +29,7 @@ describe('Manage Package Providers', () => { it('Pip should return valid package target', async function (): Promise { let testContext = createContext(); let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); should.deepEqual(provider.packageTarget, { location: constants.localhostName, packageType: constants.PythonPkgType.Pip }); }); @@ -46,7 +46,7 @@ describe('Manage Package Providers', () => { return Promise.resolve(packages); }; let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); should.deepEqual(await provider.listPackages(), packages); @@ -79,7 +79,7 @@ describe('Manage Package Providers', () => { ]; let testContext = createContext(); let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); await provider.installPackages(packages, true); @@ -110,7 +110,7 @@ describe('Manage Package Providers', () => { ]; let testContext = createContext(); let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); await provider.uninstallPackages(packages); @@ -143,7 +143,7 @@ describe('Manage Package Providers', () => { it('Pip canUseProvider should return true', async function (): Promise { let testContext = createContext(); let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); should.equal(await provider.canUseProvider(), true); @@ -151,11 +151,11 @@ describe('Manage Package Providers', () => { it('Pip getPackageOverview should return package info successfully', async function (): Promise { let testContext = createContext(); - testContext.piPyClient.fetchPypiPackage = (packageName) => { + testContext.pyPiClient.fetchPypiPackage = (packageName) => { return Promise.resolve(`{"info":{"summary":"package summary"}, "releases":{"0.0.1":[{"comment_text":""}], "0.0.2":[{"comment_text":""}]}}`); }; let serverInstallation = createJupyterServerInstallation(testContext); - let client = createPipyClient(testContext); + let client = createPypiClient(testContext); let provider = new LocalPipPackageManageProvider(serverInstallation.object, client.object); await should(provider.getPackageOverview('name')).resolvedWith({ @@ -215,9 +215,10 @@ describe('Manage Package Providers', () => { getCondaExePath: () => { return ''; }, pythonExecutable: '', pythonInstallationPath: '', - usingConda: false + usingConda: false, + installedPythonVersion: '', }, - piPyClient: { + pyPiClient: { fetchPypiPackage: (packageName) => { return Promise.resolve(); } } }; @@ -236,10 +237,10 @@ describe('Manage Package Providers', () => { return mockInstance; } - function createPipyClient(testContext: TestContext): TypeMoq.IMock { + function createPypiClient(testContext: TestContext): TypeMoq.IMock { let mockInstance = TypeMoq.Mock.ofType(PyPiClient); mockInstance.setup(x => x.fetchPypiPackage(TypeMoq.It.isAny())).returns((packageName) => - testContext.piPyClient.fetchPypiPackage(packageName)); + testContext.pyPiClient.fetchPypiPackage(packageName)); return mockInstance; } });