Skip to content

Commit

Permalink
feat: correctly compare bazel module versions
Browse files Browse the repository at this point in the history
  • Loading branch information
kormide committed Feb 6, 2024
1 parent 4b72df8 commit 7650d0f
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 52 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
42 changes: 10 additions & 32 deletions src/domain/metadata-file.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 different length components", () => {
// See: https://bazel.build/external/module#version_format
mockMetadataFile(`\
{
"homepage": "https://foo.bar",
Expand All @@ -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",
Expand All @@ -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"]);
});
});

Expand Down
22 changes: 3 additions & 19 deletions src/domain/metadata-file.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -62,7 +62,7 @@ export class MetadataFile {
}

this.metadata = json;
this.sortVersions();
this.metadata.versions.sort(compareVersions);
}

public get maintainers(): ReadonlyArray<Maintainer> {
Expand All @@ -87,7 +87,7 @@ export class MetadataFile {

public addVersions(...versions: ReadonlyArray<string>): void {
this.metadata.versions.push(...versions);
this.sortVersions();
this.metadata.versions.sort(compareVersions);
}

public addYankedVersions(yankedVersions: {
Expand Down Expand Up @@ -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 })
),
];
}
}
72 changes: 72 additions & 0 deletions src/domain/version.spec.ts
Original file line number Diff line number Diff line change
@@ -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",
]);
});
});
88 changes: 88 additions & 0 deletions src/domain/version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* 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") {
return a.value.localeCompare(b.value as string);
} 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;
}
}

0 comments on commit 7650d0f

Please sign in to comment.