From aaa1cbeb09f3fec80bed514a3a3785b1bb28fd44 Mon Sep 17 00:00:00 2001 From: Derek Cormier Date: Wed, 21 Feb 2024 19:46:59 -0800 Subject: [PATCH] feat: correctly sort bazel module versions (#145) --- package.json | 2 - src/domain/metadata-file.spec.ts | 42 ++++----------- src/domain/metadata-file.ts | 22 ++------ src/domain/version.spec.ts | 72 +++++++++++++++++++++++++ src/domain/version.ts | 93 ++++++++++++++++++++++++++++++++ yarn.lock | 5 -- 6 files changed, 178 insertions(+), 58 deletions(-) create mode 100644 src/domain/version.spec.ts create mode 100644 src/domain/version.ts diff --git a/package.json b/package.json index 146faa2..134530b 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,6 @@ "extract-zip": "^2.0.1", "gcp-metadata": "^6.0.0", "nodemailer": "^6.7.8", - "semver": "^7.5.4", "simple-git": "^3.16.0", "source-map-support": "^0.5.21", "tar": "^6.2.0", @@ -43,7 +42,6 @@ "@types/mailparser": "^3.4.4", "@types/node": "^18.6.2", "@types/nodemailer": "^6.4.5", - "@types/semver": "^7.5.6", "@types/source-map-support": "^0.5.4", "@types/tar": "^6.1.10", "@types/uuid": "^9.0.0", diff --git a/src/domain/metadata-file.spec.ts b/src/domain/metadata-file.spec.ts index 9df3420..17f452f 100644 --- a/src/domain/metadata-file.spec.ts +++ b/src/domain/metadata-file.spec.ts @@ -284,7 +284,7 @@ describe("constructor", () => { expect(metadata.maintainers[0].github).toBeUndefined(); }); - test("sorts versions by semver", () => { + test("sorts semver versions", () => { mockMetadataFile(`\ { "homepage": "https://foo.bar", @@ -333,8 +333,8 @@ describe("constructor", () => { ]); }); - test("sorts non-semver versions above semver versions", () => { - // See: https://docs.bazel.build/versions/5.0.0/bzlmod.html#version-format + test("sorts versions with a different number of identifiers", () => { + // See: https://bazel.build/external/module#version_format mockMetadataFile(`\ { "homepage": "https://foo.bar", @@ -352,11 +352,11 @@ describe("constructor", () => { `); const metadata = new MetadataFile("metadata.json"); - expect(metadata.versions).toEqual(["20210324.2", "1.0.0", "2.0.0"]); + expect(metadata.versions).toEqual(["1.0.0", "2.0.0", "20210324.2"]); }); - test("sorts non-semver versions lexicographically", () => { - // See: https://docs.bazel.build/versions/5.0.0/bzlmod.html#version-format + test("sorts non-numeric versions lexicographically", () => { + // See: https://bazel.build/external/module#version_format mockMetadataFile(`\ { "homepage": "https://foo.bar", @@ -365,38 +365,16 @@ describe("constructor", () => { "github:bar/rules_foo" ], "versions": [ - "55", - "12.4.2.1.1", - "20210324.2" + "xyz", + "abc.e", + "abc.d" ], "yanked_versions": {} } `); const metadata = new MetadataFile("metadata.json"); - expect(metadata.versions).toEqual(["12.4.2.1.1", "20210324.2", "55"]); - }); - - test("sorts non-semver versions that look like semver as non-semver", () => { - // https://github.com/bazel-contrib/publish-to-bcr/issues/97 - mockMetadataFile(`\ -{ - "homepage": "https://foo.bar", - "maintainers": [], - "repository": [ - "github:bar/rules_foo" - ], - "versions": [ - "1.0.0-rc0", - "1.0.0-rc1", - "1.0.0rc1" - ], - "yanked_versions": {} -} -`); - const metadata = new MetadataFile("metadata.json"); - - expect(metadata.versions).toEqual(["1.0.0rc1", "1.0.0-rc0", "1.0.0-rc1"]); + expect(metadata.versions).toEqual(["abc.d", "abc.e", "xyz"]); }); }); diff --git a/src/domain/metadata-file.ts b/src/domain/metadata-file.ts index 4478d4e..19cce56 100644 --- a/src/domain/metadata-file.ts +++ b/src/domain/metadata-file.ts @@ -1,5 +1,5 @@ import fs from "node:fs"; -import { compare as semverCompare, valid as validSemver } from "semver"; +import { compareVersions } from "./version.js"; export class MetadataFileError extends Error { constructor(path: string, message: string) { @@ -62,7 +62,7 @@ export class MetadataFile { } this.metadata = json; - this.sortVersions(); + this.metadata.versions.sort(compareVersions); } public get maintainers(): ReadonlyArray { @@ -87,7 +87,7 @@ export class MetadataFile { public addVersions(...versions: ReadonlyArray): void { this.metadata.versions.push(...versions); - this.sortVersions(); + this.metadata.versions.sort(compareVersions); } public addYankedVersions(yankedVersions: { @@ -135,20 +135,4 @@ export class MetadataFile { return []; } - - private sortVersions(): void { - const semver = this.metadata.versions.filter( - (v: string) => !!validSemver(v, { loose: false }) - ); - const nonSemver = this.metadata.versions.filter( - (v: string) => !validSemver(v) - ); - - this.metadata.versions = [ - ...nonSemver.sort(), - ...semver.sort((a: string, b: string) => - semverCompare(a, b, { loose: false }) - ), - ]; - } } diff --git a/src/domain/version.spec.ts b/src/domain/version.spec.ts new file mode 100644 index 0000000..99ec1e3 --- /dev/null +++ b/src/domain/version.spec.ts @@ -0,0 +1,72 @@ +import { compareVersions } from "./version"; + +describe("compareVersions", () => { + it("should sort semvers", () => { + expect( + [ + "2.0.0", + "1.0.0", + "0.32.1", + "0.32.11", + "2.11.0", + "1.0.0-rc1", + "1.0.0-rc0", + "1.0.0-rc23", + "1.0.1-rc1", + "2.10.1", + ].sort(compareVersions) + ).toEqual([ + "0.32.1", + "0.32.11", + "1.0.0-rc0", + "1.0.0-rc1", + "1.0.0-rc23", + "1.0.0", + "1.0.1-rc1", + "2.0.0", + "2.10.1", + "2.11.0", + ]); + }); + + it("should sort versions with more than 3 components", () => { + expect(["6.4.0.2", "6.4.0", "6.4.0.2-rc0"].sort(compareVersions)).toEqual([ + "6.4.0", + "6.4.0.2-rc0", + "6.4.0.2", + ]); + }); + + it("should sort duplciates", () => { + expect(["1.0.0", "2.0.0", "1.0.0"].sort(compareVersions)).toEqual([ + "1.0.0", + "1.0.0", + "2.0.0", + ]); + }); + + it("should sort versions with non-numeric identifiers", () => { + expect( + ["z", "b.aa.b", "a.ab.b-rcfoo", "a.ab.b", "a.ab.a", "a.aa.b", "x.y"].sort( + compareVersions + ) + ).toEqual([ + "a.aa.b", + "a.ab.a", + "a.ab.b-rcfoo", + "a.ab.b", + "b.aa.b", + "x.y", + "z", + ]); + }); + + it("should sort numeric and non-numeric identifiers", () => { + expect(["x.7.z", "1.2.3", "x.6.y", "a.b.c"].sort(compareVersions)).toEqual([ + "1.2.3", + "a.b.c", + "x.6.y", + "x.7.z", + ]); + }); +}); diff --git a/src/domain/version.ts b/src/domain/version.ts new file mode 100644 index 0000000..4adbae8 --- /dev/null +++ b/src/domain/version.ts @@ -0,0 +1,93 @@ +/** + * Compare bazel module versions + * + * Adapted from https://github.com/bazelbuild/bazel-central-registry/blob/127d91703baf4e39eb66fc907d255b37d6162792/tools/registry.py#L85 + */ +export function compareVersions(a: string, b: string) { + return Version.compare(new Version(a), new Version(b)); +} + +class Version { + public static compare(a: Version, b: Version): number { + const result = Version.compareIdentifiers(a.release, b.release); + if (result) { + return result; + } + + if (a.prerelease.length === 0) { + return 1; + } + if (b.prerelease.length === 0) { + return -1; + } + + return Version.compareIdentifiers(a.prerelease, b.prerelease); + } + + private static compareIdentifiers(a: Identifier[], b: Identifier[]) { + const l = Math.min(a.length, b.length); + for (let i = 0; i < l; i++) { + const result = Identifier.compare(a[i], b[i]); + if (result) { + return result; + } + } + + if (a.length > b.length) { + return 1; + } else if (b.length > a.length) { + return -1; + } + + return 0; + } + + private readonly prerelease: Identifier[]; + private readonly release: Identifier[]; + + public constructor(version: string) { + const pattern = + /^([a-zA-Z0-9.]+)(?:-([a-zA-Z0-9.-]+))?(?:\+[a-zA-Z0-9.-]+)?$/; + const match = version.match(pattern); + if (!match) { + throw new Error(`Invalid module version '${version}'`); + } + + this.release = this.convertToIdentifiers(match[1]); + this.prerelease = this.convertToIdentifiers(match[2]); + } + + private convertToIdentifiers(version: string): Identifier[] { + return (version && version.split(".").map((i) => new Identifier(i))) || []; + } +} + +class Identifier { + public static compare(a: Identifier, b: Identifier): number { + if (typeof a.value !== typeof b.value) { + if (typeof a.value === "number") { + return -1; + } else { + return 1; + } + } + + if (typeof a.value === "string") { + if (a.value < b.value) { + return -1; + } else if (a.value === b.value) { + return 0; + } + return 1; + } else { + return a.value - (b.value as number); + } + } + + private readonly value: string | number; + + public constructor(value: string) { + const numeric = parseInt(value); + this.value = isNaN(numeric) ? value : numeric; + } +} diff --git a/yarn.lock b/yarn.lock index 968f83b..aaef61a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1348,11 +1348,6 @@ "@types/tough-cookie" "*" form-data "^2.5.0" -"@types/semver@^7.5.6": - version "7.5.6" - resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.5.6.tgz#c65b2bfce1bec346582c07724e3f8c1017a20339" - integrity sha512-dn1l8LaMea/IjDoHNd9J52uBbInB796CDffS6VdIxvqYCPSG0V0DzHp76GpaWnlhg88uYyPbXCDIowa86ybd5A== - "@types/send@*": version "0.17.4" resolved "https://registry.yarnpkg.com/@types/send/-/send-0.17.4.tgz#6619cd24e7270793702e4e6a4b958a9010cfc57a"