From 4b72df89b42ce93f99d329d3f5e4c814ec4c3473 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Tue, 30 Jan 2024 21:52:58 +0100 Subject: [PATCH] Cope with missing `version` attribute on module (#139) Cope with missing `version` attribute on module --------- Co-authored-by: Derek Cormier --- e2e/__snapshots__/e2e.spec.ts.snap | 89 +++++++++++++++++-- e2e/e2e.spec.ts | 39 ++++++++ e2e/fixtures/unversioned/MODULE.bazel | 5 +- e2e/fixtures/unversioned/README.md | 2 +- .../.bcr/metadata.template.json | 15 ++++ .../zero-versioned/.bcr/presubmit.yml | 10 +++ .../zero-versioned/.bcr/source.template.json | 5 ++ e2e/fixtures/zero-versioned/MODULE.bazel | 4 + e2e/fixtures/zero-versioned/README.md | 1 + e2e/helpers/fixture.ts | 1 + src/domain/create-entry.ts | 3 +- src/domain/module-file.spec.ts | 42 ++++++++- src/domain/module-file.ts | 23 +++-- 13 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 e2e/fixtures/zero-versioned/.bcr/metadata.template.json create mode 100644 e2e/fixtures/zero-versioned/.bcr/presubmit.yml create mode 100644 e2e/fixtures/zero-versioned/.bcr/source.template.json create mode 100644 e2e/fixtures/zero-versioned/MODULE.bazel create mode 100644 e2e/fixtures/zero-versioned/README.md diff --git a/e2e/__snapshots__/e2e.spec.ts.snap b/e2e/__snapshots__/e2e.spec.ts.snap index 63a3ef5..aa12259 100644 --- a/e2e/__snapshots__/e2e.spec.ts.snap +++ b/e2e/__snapshots__/e2e.spec.ts.snap @@ -170,21 +170,21 @@ modules/unversioned/1.0.0/MODULE.bazel ---------------------------------------------------- module( name = \\"unversioned\\", - version = \\"1.0.0\\", + version = \\"1.0.0\\", ) + ---------------------------------------------------- modules/unversioned/1.0.0/patches/module_dot_bazel_version.patch ---------------------------------------------------- =================================================================== --- a/MODULE.bazel +++ b/MODULE.bazel -@@ -1,4 +1,4 @@ +@@ -1,3 +1,4 @@ module( - name = \\"unversioned\\", -- version = \\"0.0.0\\", -+ version = \\"1.0.0\\", +- name = \\"unversioned\\" ++ name = \\"unversioned\\", ++ version = \\"1.0.0\\", ) -\\\\ No newline at end of file ---------------------------------------------------- modules/unversioned/1.0.0/presubmit.yml @@ -204,11 +204,11 @@ bcr_test_module: modules/unversioned/1.0.0/source.json ---------------------------------------------------- { - \\"integrity\\": \\"sha256-eXQOU+DNwg/Q29tX59jP0+SFLvRHrQf3z+KJVS/gIRk=\\", + \\"integrity\\": \\"sha256-oLg/mgWyhr6DFepRYkjoPphQ8KOriLgu6M4x2RSsLbU=\\", \\"strip_prefix\\": \\"unversioned-1.0.0\\", \\"url\\": \\"https://github.com/testorg/unversioned/archive/refs/tags/v1.0.0.tar.gz\\", \\"patches\\": { - \\"module_dot_bazel_version.patch\\": \\"sha256-y0kC8heeH9bQKhrfx2JuX+RK0KyjwHhPac3wBc4Nkg4=\\" + \\"module_dot_bazel_version.patch\\": \\"sha256-LGXyh9FLhgIPbe0gHfxAPnEQ7HVR+HUP/IDbPdl3ZkA=\\" }, \\"patch_strip\\": 1 } @@ -292,6 +292,79 @@ modules/versioned/metadata.json " `; +exports[`e2e tests [snapshot] ruleset with zero-versioned module in source 1`] = ` +"---------------------------------------------------- +modules/zero-versioned/1.0.0/MODULE.bazel +---------------------------------------------------- +module( + name = \\"zero-versioned\\", + version = \\"1.0.0\\", +) + +---------------------------------------------------- +modules/zero-versioned/1.0.0/patches/module_dot_bazel_version.patch +---------------------------------------------------- +=================================================================== +--- a/MODULE.bazel ++++ b/MODULE.bazel +@@ -1,4 +1,4 @@ + module( + name = \\"zero-versioned\\", +- version = \\"0.0.0\\", ++ version = \\"1.0.0\\", + ) + +---------------------------------------------------- +modules/zero-versioned/1.0.0/presubmit.yml +---------------------------------------------------- +bcr_test_module: + module_path: \\"e2e/bzlmod\\" + matrix: + platform: [\\"debian10\\", \\"macos\\", \\"ubuntu2004\\", \\"windows\\"] + tasks: + run_tests: + name: \\"Run test module\\" + platform: \${{ platform }} + test_targets: + - \\"//...\\" + +---------------------------------------------------- +modules/zero-versioned/1.0.0/source.json +---------------------------------------------------- +{ + \\"integrity\\": \\"sha256-sLh05DKTEOuO3vz9+IDUJJNOf0GSLxdGDJ8qOLZRUCQ=\\", + \\"strip_prefix\\": \\"zero-versioned-1.0.0\\", + \\"url\\": \\"https://github.com/testorg/zero-versioned/archive/refs/tags/v1.0.0.tar.gz\\", + \\"patches\\": { + \\"module_dot_bazel_version.patch\\": \\"sha256-KpbuC1vv5mfhdTs5nnTl3/pH7Y/6JCnD1b1XLsqyOAo=\\" + }, + \\"patch_strip\\": 1 +} + +---------------------------------------------------- +modules/zero-versioned/metadata.json +---------------------------------------------------- +{ + \\"homepage\\": \\"https://github.com/testorg/zero-versioned\\", + \\"maintainers\\": [ + { + \\"name\\": \\"Foo McBar\\", + \\"email\\": \\"foo@test.org\\", + \\"github\\": \\"foobar\\" + } + ], + \\"repository\\": [ + \\"github:testorg/zero-versioned\\" + ], + \\"versions\\": [ + \\"1.0.0\\" + ], + \\"yanked_versions\\": {} +} + +" +`; + exports[`e2e tests [snapshot] ruleset with zip release archive 1`] = ` "---------------------------------------------------- modules/zip/1.0.0/MODULE.bazel diff --git a/e2e/e2e.spec.ts b/e2e/e2e.spec.ts index 39fbfe1..d9f7b6a 100644 --- a/e2e/e2e.spec.ts +++ b/e2e/e2e.spec.ts @@ -133,6 +133,45 @@ describe("e2e tests", () => { expect(snapshot).toMatchSnapshot(); }); + test("[snapshot] ruleset with zero-versioned module in source", async () => { + const repo = Fixture.ZeroVersioned; + const tag = "v1.0.0"; + await setupLocalRemoteRulesetRepo(repo, tag, releaser); + + fakeGitHub.mockUser(releaser); + fakeGitHub.mockRepository(testOrg, repo); + fakeGitHub.mockRepository( + testOrg, + "bazel-central-registry", + "bazelbuild", + "bazel-central-registry" + ); + const installationId = fakeGitHub.mockAppInstallation(testOrg, repo); + fakeGitHub.mockAppInstallation(testOrg, "bazel-central-registry"); + + const releaseArchive = await makeReleaseTarball(repo, "zero-versioned-1.0.0"); + await fakeGitHub.mockReleaseArchive( + `/${testOrg}/${repo}/archive/refs/tags/${tag}.tar.gz`, + releaseArchive + ); + + const response = await publishReleaseEvent( + cloudFunctions.getBaseUrl(), + secrets.webhookSecret, + installationId, + { + owner: testOrg, + repo, + tag, + releaser, + } + ); + expect(response.status).toEqual(200); + + const snapshot = await rollupEntryFiles(); + expect(snapshot).toMatchSnapshot(); + }); + test("[snapshot] ruleset with versioned module in source", async () => { const repo = Fixture.Versioned; const tag = "v1.0.0"; diff --git a/e2e/fixtures/unversioned/MODULE.bazel b/e2e/fixtures/unversioned/MODULE.bazel index 38276ac..93d4911 100644 --- a/e2e/fixtures/unversioned/MODULE.bazel +++ b/e2e/fixtures/unversioned/MODULE.bazel @@ -1,4 +1,3 @@ module( - name = "unversioned", - version = "0.0.0", -) \ No newline at end of file + name = "unversioned" +) diff --git a/e2e/fixtures/unversioned/README.md b/e2e/fixtures/unversioned/README.md index d1da306..9d8c5e3 100644 --- a/e2e/fixtures/unversioned/README.md +++ b/e2e/fixtures/unversioned/README.md @@ -1 +1 @@ -Ruleset repo that doesn't update the source MODULE.bazel's `version` field. +Ruleset repo that doesn't have the source MODULE.bazel's `version` field. diff --git a/e2e/fixtures/zero-versioned/.bcr/metadata.template.json b/e2e/fixtures/zero-versioned/.bcr/metadata.template.json new file mode 100644 index 0000000..effa93a --- /dev/null +++ b/e2e/fixtures/zero-versioned/.bcr/metadata.template.json @@ -0,0 +1,15 @@ +{ + "homepage": "https://github.com/testorg/zero-versioned", + "maintainers": [ + { + "name": "Foo McBar", + "email": "foo@test.org", + "github": "foobar" + } + ], + "repository": [ + "github:testorg/zero-versioned" + ], + "versions": [], + "yanked_versions": {} +} diff --git a/e2e/fixtures/zero-versioned/.bcr/presubmit.yml b/e2e/fixtures/zero-versioned/.bcr/presubmit.yml new file mode 100644 index 0000000..35881d9 --- /dev/null +++ b/e2e/fixtures/zero-versioned/.bcr/presubmit.yml @@ -0,0 +1,10 @@ +bcr_test_module: + module_path: "e2e/bzlmod" + matrix: + platform: ["debian10", "macos", "ubuntu2004", "windows"] + tasks: + run_tests: + name: "Run test module" + platform: ${{ platform }} + test_targets: + - "//..." diff --git a/e2e/fixtures/zero-versioned/.bcr/source.template.json b/e2e/fixtures/zero-versioned/.bcr/source.template.json new file mode 100644 index 0000000..4f14819 --- /dev/null +++ b/e2e/fixtures/zero-versioned/.bcr/source.template.json @@ -0,0 +1,5 @@ +{ + "integrity": "", + "strip_prefix": "{REPO}-{VERSION}", + "url": "https://github.com/{OWNER}/{REPO}/archive/refs/tags/{TAG}.tar.gz" +} diff --git a/e2e/fixtures/zero-versioned/MODULE.bazel b/e2e/fixtures/zero-versioned/MODULE.bazel new file mode 100644 index 0000000..fa1e047 --- /dev/null +++ b/e2e/fixtures/zero-versioned/MODULE.bazel @@ -0,0 +1,4 @@ +module( + name = "zero-versioned", + version = "0.0.0", +) diff --git a/e2e/fixtures/zero-versioned/README.md b/e2e/fixtures/zero-versioned/README.md new file mode 100644 index 0000000..6322257 --- /dev/null +++ b/e2e/fixtures/zero-versioned/README.md @@ -0,0 +1 @@ +Ruleset repo that doesn't set the source MODULE.bazel's `version` field. diff --git a/e2e/helpers/fixture.ts b/e2e/helpers/fixture.ts index 7273161..1446f31 100644 --- a/e2e/helpers/fixture.ts +++ b/e2e/helpers/fixture.ts @@ -16,6 +16,7 @@ export enum Fixture { NoPrefix = "no-prefix", Tarball = "tarball", Unversioned = "unversioned", + ZeroVersioned = "zero-versioned", Versioned = "versioned", Zip = "zip", } diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts index fea6ee0..9d93949 100644 --- a/src/domain/create-entry.ts +++ b/src/domain/create-entry.ts @@ -231,7 +231,8 @@ export class CreateEntryService { ): void { if (moduleFile.version !== version) { console.log( - "Archived MODULE.bazel version does not match release version. Creating a version patch." + `Archived MODULE.bazel version ${moduleFile.version} does not match release version ${version}.`, + "Creating a version patch.", ); const patchFileName = "module_dot_bazel_version.patch"; const existingContent = moduleFile.content; diff --git a/src/domain/module-file.spec.ts b/src/domain/module-file.spec.ts index bc42d49..5fd7330 100644 --- a/src/domain/module-file.spec.ts +++ b/src/domain/module-file.spec.ts @@ -36,11 +36,19 @@ describe("moduleName", () => { }); }); -describe("moduleVersion", () => { +describe("version", () => { test("parses module version", () => { const moduleFile = new ModuleFile("MODULE.bazel"); expect(moduleFile.version).toEqual("1.2.3"); }); + + test("returns undefined when the version is missing", () => { + mocked(fs.readFileSync).mockReturnValue(`\ +module(name = "rules_foo") +`); + const moduleFile = new ModuleFile("MODULE.bazel"); + expect(moduleFile.version).toBeUndefined(); + }); }); describe("content", () => { @@ -60,6 +68,38 @@ describe("stampVersion", () => { fakeModuleFile({ moduleName: "rules_foo", version: "4.5.6" }) ); }); + + test("stamps the version when the version field was originally missing", () => { + mocked(fs.readFileSync).mockReturnValue(`\ +module( + name = "rules_foo" +)`); + const moduleFile = new ModuleFile("MODULE.bazel"); + moduleFile.stampVersion("4.5.6"); + + expect(moduleFile.content).toEqual(`\ +module( + name = "rules_foo", + version = "4.5.6", +)`); + }); + + test("stamps the version when the version field was originally missing and the last field is comma-trailed", () => { + mocked(fs.readFileSync).mockReturnValue(`\ +module( + name = "rules_foo", + compatibility_level = 1, +)`); + const moduleFile = new ModuleFile("MODULE.bazel"); + moduleFile.stampVersion("4.5.6"); + + expect(moduleFile.content).toEqual(`\ +module( + name = "rules_foo", + compatibility_level = 1, + version = "4.5.6", +)`); + }); }); describe("save", () => { diff --git a/src/domain/module-file.ts b/src/domain/module-file.ts index ef66ea7..6da7ddc 100644 --- a/src/domain/module-file.ts +++ b/src/domain/module-file.ts @@ -21,10 +21,10 @@ export class ModuleFile { return name; } - public get version(): string { + public get version(): string | undefined { const regex = /module\([^)]*?version\s*=\s*"(.+?)"/s; - const version = this.moduleContent.match(regex)[1]; - return version; + const match = this.moduleContent.match(regex); + return match ? match[1] : undefined; } public get content(): string { @@ -32,10 +32,19 @@ export class ModuleFile { } public stampVersion(version: string): void { - this.moduleContent = this.moduleContent.replace( - /(^.*?module\(.*?version\s*=\s*")[\w.]+(".*$)/s, - `$1${version}$2` - ); + if (this.version !== undefined) { + // update the version + this.moduleContent = this.moduleContent.replace( + /(^.*?module\(.*?version\s*=\s*")[\w.]+(".*$)/s, + `$1${version}$2` + ); + } else { + // add the version + this.moduleContent = this.moduleContent.replace( + /(^.*?module\(.*?),?(\s*)\)/s, + `$1,\n version = "${version}",\n)` + ); + } } public save(destPath: string) {