fix: extraction of multi-module release archive (#147)

bazel-contrib · Feb 15, 2024 · db12297 · db12297
1 parent a1f7887
commit db12297
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 62 deletions.
diff --git a/e2e/e2e.spec.ts b/e2e/e2e.spec.ts
@@ -469,23 +469,11 @@ describe("e2e tests", () => {
     const installationId = fakeGitHub.mockAppInstallation(testOrg, repo);
     fakeGitHub.mockAppInstallation(testOrg, "bazel-central-registry");
 
-    const releaseArchive1 = await makeReleaseTarball(
-      repo,
-      "multi-module-1.0.0"
-    );
-    const releaseArchive2 = await makeReleaseTarball(
-      repo,
-      "multi-module-1.0.0"
-    );
+    const releaseArchive = await makeReleaseTarball(repo, "multi-module-1.0.0");
 
     await fakeGitHub.mockReleaseArchive(
-      `/${testOrg}/${repo}/releases/download/${tag}/module-${tag}.tar.gz`,
-      releaseArchive1
-    );
-
-    await fakeGitHub.mockReleaseArchive(
-      `/${testOrg}/${repo}/releases/download/${tag}/submodule-${tag}.tar.gz`,
-      releaseArchive2
+      `/${testOrg}/${repo}/releases/download/${tag}.tar.gz`,
+      releaseArchive
     );
 
     const response = await publishReleaseEvent(
@@ -791,18 +779,18 @@ describe("e2e tests", () => {
 const testReleaseArchives: string[] = [];
 async function makeReleaseTarball(
   fixture: Fixture,
-  stripPrefix?: string
+  prefix?: string
 ): Promise<string> {
-  const filename = await _makeReleaseTarball(fixture, stripPrefix);
+  const filename = await _makeReleaseTarball(fixture, prefix);
   testReleaseArchives.push(filename);
   return filename;
 }
 
 async function makeReleaseZip(
   fixture: string,
-  stripPrefix?: string
+  prefix?: string
 ): Promise<string> {
-  const filename = await _makeReleaseZip(fixture, stripPrefix);
+  const filename = await _makeReleaseZip(fixture, prefix);
   testReleaseArchives.push(filename);
   return filename;
 }

diff --git a/e2e/fixtures/multi-module/.bcr/source.template.json b/e2e/fixtures/multi-module/.bcr/source.template.json
@@ -1,5 +1,5 @@
 {
   "integrity": "",
-  "strip_prefix": "{REPO}-{VERSION}/submodule",
-  "url": "https://github.com/{OWNER}/{REPO}/releases/download/{TAG}/module-{TAG}.tar.gz"
+  "strip_prefix": "{REPO}-{VERSION}",
+  "url": "https://github.com/{OWNER}/{REPO}/releases/download/{TAG}.tar.gz"
 }
diff --git a/e2e/fixtures/multi-module/.bcr/submodule/source.template.json b/e2e/fixtures/multi-module/.bcr/submodule/source.template.json
@@ -1,5 +1,5 @@
 {
   "integrity": "",
-  "strip_prefix": "{REPO}-{VERSION}",
-  "url": "https://github.com/{OWNER}/{REPO}/releases/download/{TAG}/submodule-{TAG}.tar.gz"
+  "strip_prefix": "{REPO}-{VERSION}/submodule",
+  "url": "https://github.com/{OWNER}/{REPO}/releases/download/{TAG}.tar.gz"
 }
diff --git a/e2e/helpers/archive.ts b/e2e/helpers/archive.ts
@@ -7,7 +7,7 @@ import tar from "tar";
 
 export async function makeReleaseTarball(
   fixture: string,
-  stripPrefix?: string
+  prefix?: string
 ): Promise<string> {
   const filename = path.join(
     os.tmpdir(),
@@ -17,7 +17,7 @@ export async function makeReleaseTarball(
   await tar.create(
     {
       gzip: { level: 1 },
-      prefix: stripPrefix,
+      prefix,
       file: filename,
       cwd: path.join("e2e", "fixtures", fixture),
       portable: true,
@@ -31,7 +31,7 @@ export async function makeReleaseTarball(
 
 export async function makeReleaseZip(
   fixture: string,
-  stripPrefix?: string
+  prefix?: string
 ): Promise<string> {
   const filename = path.join(
     os.tmpdir(),
@@ -44,13 +44,9 @@ export async function makeReleaseZip(
   archive.pipe(output);
 
   const hermeticDate = new Date(0);
-  archive.directory(
-    path.join("e2e", "fixtures", fixture),
-    stripPrefix || false,
-    {
-      date: hermeticDate,
-    }
-  );
+  archive.directory(path.join("e2e", "fixtures", fixture), prefix || false, {
+    date: hermeticDate,
+  });
 
   await archive.finalize();
 

diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts
@@ -63,7 +63,7 @@ export class CreateEntryService {
       const integrityHash = computeIntegrityHash(releaseArchive.diskPath);
       sourceTemplate.setIntegrityHash(integrityHash);
 
-      const moduleFile = await releaseArchive.extractModuleFile(moduleRoot);
+      const moduleFile = await releaseArchive.extractModuleFile();
 
       const bcrEntryPath = path.resolve(
         bcrRepo.diskPath,

diff --git a/src/domain/release-archive.spec.ts b/src/domain/release-archive.spec.ts
@@ -149,7 +149,7 @@ describe("extractModuleFile", () => {
     );
 
     const thrownError = await expectThrownError(
-      () => releaseArchive.extractModuleFile("."),
+      () => releaseArchive.extractModuleFile(),
       UnsupportedArchiveFormat
     );
     expect(thrownError.message.includes("deb")).toEqual(true);
@@ -160,7 +160,7 @@ describe("extractModuleFile", () => {
       "https://foo.bar/rules-foo-v1.2.3.tar.gz",
       STRIP_PREFIX
     );
-    await releaseArchive.extractModuleFile(".");
+    await releaseArchive.extractModuleFile();
 
     expect(tar.x).toHaveBeenCalledWith({
       cwd: path.dirname(releaseArchive.diskPath),
@@ -173,7 +173,7 @@ describe("extractModuleFile", () => {
       RELEASE_ARCHIVE_URL,
       STRIP_PREFIX
     );
-    await releaseArchive.extractModuleFile(".");
+    await releaseArchive.extractModuleFile();
 
     const expectedPath = path.join(
       path.dirname(releaseArchive.diskPath),
@@ -183,29 +183,12 @@ describe("extractModuleFile", () => {
     expect(fs.readFileSync).toHaveBeenCalledWith(expectedPath, "utf8");
   });
 
-  test("loads an extracted MODULE.bazel file in a different module root", async () => {
-    const releaseArchive = await ReleaseArchive.fetch(
-      RELEASE_ARCHIVE_URL,
-      STRIP_PREFIX
-    );
-    await releaseArchive.extractModuleFile("sub/dir");
-
-    const expectedPath = path.join(
-      path.dirname(releaseArchive.diskPath),
-      STRIP_PREFIX,
-      "sub",
-      "dir",
-      "MODULE.bazel"
-    );
-    expect(fs.readFileSync).toHaveBeenCalledWith(expectedPath, "utf8");
-  });
-
   test("returns a module file representation", async () => {
     const releaseArchive = await ReleaseArchive.fetch(
       RELEASE_ARCHIVE_URL,
       STRIP_PREFIX
     );
-    const moduleFile = await releaseArchive.extractModuleFile(".");
+    const moduleFile = await releaseArchive.extractModuleFile();
 
     expect(moduleFile.moduleName).toEqual("rules_foo");
     expect(moduleFile.version).toEqual("1.2.3");
@@ -219,7 +202,7 @@ describe("extractModuleFile", () => {
 
     mocked(fs.existsSync).mockReturnValue(false);
 
-    await expect(releaseArchive.extractModuleFile(".")).rejects.toThrow(
+    await expect(releaseArchive.extractModuleFile()).rejects.toThrow(
       MissingModuleFileError
     );
   });

diff --git a/src/domain/release-archive.ts b/src/domain/release-archive.ts
@@ -58,7 +58,7 @@ export class ReleaseArchive {
     private readonly stripPrefix: string
   ) {}
 
-  public async extractModuleFile(moduleRoot: string): Promise<ModuleFile> {
+  public async extractModuleFile(): Promise<ModuleFile> {
     this.extractDir = path.dirname(this._diskPath);
 
     if (this._diskPath.endsWith(".tar.gz")) {
@@ -70,11 +70,7 @@ export class ReleaseArchive {
       throw new UnsupportedArchiveFormat(extension);
     }
 
-    const pathInArchive = path.join(
-      this.stripPrefix,
-      moduleRoot,
-      "MODULE.bazel"
-    );
+    const pathInArchive = path.join(this.stripPrefix, "MODULE.bazel");
 
     const extractedModulePath = path.join(this.extractDir, pathInArchive);