From c8483fb5c3b2efc75a123fd4816859758b8612dd Mon Sep 17 00:00:00 2001 From: Craig Burdulis Date: Mon, 25 Mar 2024 08:35:29 -0400 Subject: [PATCH] fix(jsii-pacmak): fully support the Python Version Identification part of PEP440 (#4462) `jsii-pacmak`'s current version logic does not fully implement/adhere to PEP440 in two respects: * There is no support for "[local version identifiers](https://packaging.python.org/en/latest/specifications/version-specifiers/#local-version-identifiers)", which are basically the same as SemVer's build metadata (e.g. `1.2.3+foobar`), when used in conjunction with a pre-release label. * The current pre-release logic doesn't reflect the ability for python pre-releases to include [post-release](https://packaging.python.org/en/latest/specifications/version-specifiers/#post-releases) and [developmental release](https://packaging.python.org/en/latest/specifications/version-specifiers/#developmental-releases) labels in conjunction with the pre-release itself (e.g. `1.2.3.rc1.post2.dev3`) This PR addresses these gaps so that the python release supports these features. I've kept support for the `pre` label as a synonym for `dev`, E.g. now `1.2.3-rc.1.dev.2.post.3+foobar` will now yield `1.2.3.rc1.post3.dev2+foobar` for python packages. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- .../jsii-pacmak/lib/targets/version-utils.ts | 96 +++++++++++++------ .../test/targets/version-utils.test.ts | 22 +++++ 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/version-utils.ts b/packages/jsii-pacmak/lib/targets/version-utils.ts index 9aeada27cf..7c1b6b62c2 100644 --- a/packages/jsii-pacmak/lib/targets/version-utils.ts +++ b/packages/jsii-pacmak/lib/targets/version-utils.ts @@ -93,42 +93,80 @@ export function toReleaseVersion( } switch (target) { case TargetName.PYTHON: + const baseVersion = `${version.major}.${version.minor}.${version.patch}`; + // Python supports a limited set of identifiers... And we have a mapping table... // https://packaging.python.org/guides/distributing-packages-using-setuptools/#pre-release-versioning - const [label, sequence, ...rest] = version.prerelease; - if (rest.filter((elt) => elt !== 0).length > 0 || sequence == null) { + const releaseLabels: Record = { + alpha: 'a', + beta: 'b', + rc: 'rc', + post: 'post', + dev: 'dev', + pre: 'pre', + }; + + const validationErrors: string[] = []; + + // Ensure that prerelease composed entirely of [label, sequence] pairs + version.prerelease.forEach((elem, idx, arr) => { + const next: string | number | undefined = arr[idx + 1]; + if (typeof elem === 'string') { + if (!Object.keys(releaseLabels).includes(elem)) { + validationErrors.push( + `Label ${elem} is not one of ${Object.keys(releaseLabels).join( + ',', + )}`, + ); + } + if (next === undefined || !Number.isInteger(next)) { + validationErrors.push( + `Label ${elem} must be followed by a positive integer`, + ); + } + } + }); + + if (validationErrors.length > 0) { throw new Error( `Unable to map prerelease identifier (in: ${assemblyVersion}) components to python: ${inspect( version.prerelease, - )}. The format should be 'X.Y.Z-label.sequence', where sequence is a positive integer, and label is "dev", "pre", "alpha", beta", or "rc"`, + )}. The format should be 'X.Y.Z-[label.sequence][.post.sequence][.(dev|pre).sequence]', where sequence is a positive integer and label is one of ${inspect( + Object.keys(releaseLabels), + )}. Validation errors encountered: ${validationErrors.join(', ')}`, ); } - if (!Number.isInteger(sequence)) { - throw new Error( - `Unable to map prerelease identifier (in: ${assemblyVersion}) to python, as sequence ${inspect( - sequence, - )} is not an integer`, - ); - } - const baseVersion = `${version.major}.${version.minor}.${version.patch}`; - // See PEP 440: https://www.python.org/dev/peps/pep-0440/#pre-releases - switch (label) { - case 'dev': - case 'pre': - return `${baseVersion}.dev${sequence}`; - case 'alpha': - return `${baseVersion}.a${sequence}`; - case 'beta': - return `${baseVersion}.b${sequence}`; - case 'rc': - return `${baseVersion}.rc${sequence}`; - default: - throw new Error( - `Unable to map prerelease identifier (in: ${assemblyVersion}) to python, as label ${inspect( - label, - )} is not mapped (only "dev", "pre", "alpha", "beta" and "rc" are)`, - ); - } + + // PEP440 supports multiple labels in a given version, so + // we should attempt to identify and map as many labels as + // possible from the given prerelease input + // e.g. 1.2.3-rc.123.dev.456.post.789 => 1.2.3.rc123.dev456.post789 + const postIdx = version.prerelease.findIndex( + (v) => v.toString() === 'post', + ); + const devIdx = version.prerelease.findIndex((v) => + ['dev', 'pre'].includes(v.toString()), + ); + const preReleaseIdx = version.prerelease.findIndex((v) => + ['alpha', 'beta', 'rc'].includes(v.toString()), + ); + const prereleaseVersion = [ + preReleaseIdx > -1 + ? `${releaseLabels[version.prerelease[preReleaseIdx]]}${ + version.prerelease[preReleaseIdx + 1] ?? 0 + }` + : undefined, + postIdx > -1 + ? `post${version.prerelease[postIdx + 1] ?? 0}` + : undefined, + devIdx > -1 ? `dev${version.prerelease[devIdx + 1] ?? 0}` : undefined, + ] + .filter((v) => v) + .join('.'); + + return version.build.length > 0 + ? `${baseVersion}.${prereleaseVersion}+${version.build.join('.')}` + : `${baseVersion}.${prereleaseVersion}`; case TargetName.DOTNET: case TargetName.GO: case TargetName.JAVA: diff --git a/packages/jsii-pacmak/test/targets/version-utils.test.ts b/packages/jsii-pacmak/test/targets/version-utils.test.ts index 8564706e2d..129ec494f0 100644 --- a/packages/jsii-pacmak/test/targets/version-utils.test.ts +++ b/packages/jsii-pacmak/test/targets/version-utils.test.ts @@ -127,6 +127,13 @@ describe(toReleaseVersion, () => { python: /Unable to map prerelease identifier \(in: 1\.2\.3-pre\) components to python: \[ 'pre' \]/, }, + '1.2.3-dev.123.0+abc123.foo.bar': { + dotnet: '1.2.3-dev.123.0+abc123.foo.bar', + go: '1.2.3-dev.123.0+abc123.foo.bar', + java: '1.2.3-dev.123.0+abc123.foo.bar', + js: '1.2.3-dev.123.0+abc123.foo.bar', + python: '1.2.3.dev123+abc123.foo.bar', + }, '1.2.3-alpha.1337': { dotnet: '1.2.3-alpha.1337', go: '1.2.3-alpha.1337', @@ -148,6 +155,21 @@ describe(toReleaseVersion, () => { js: '1.2.3-rc.9', python: '1.2.3.rc9', }, + '1.2.3-rc.123.post.456.dev.789': { + dotnet: '1.2.3-rc.123.post.456.dev.789', + go: '1.2.3-rc.123.post.456.dev.789', + java: '1.2.3-rc.123.post.456.dev.789', + js: '1.2.3-rc.123.post.456.dev.789', + python: '1.2.3.rc123.post456.dev789', + }, + '1.2.3-rc.alpha': { + dotnet: '1.2.3-rc.alpha', + go: '1.2.3-rc.alpha', + java: '1.2.3-rc.alpha', + js: '1.2.3-rc.alpha', + python: + /Unable to map prerelease identifier \(in: 1.2.3-rc.alpha\) components to python: \[ 'rc', 'alpha' \]/, + }, }; for (const [version, targets] of Object.entries(examples)) {