refactor: rework browser update script to properly deal with artifact extensions

Reworks the browser update script to properly deal with artifact extensions,
not incorrectly treating every file as `zip`, preventing Bazel's extraction
detection from working.

Author: devversion

bazel/browsers/update-script/browser-artifact.ts

@@ -0,0 +1,61 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Browser} from './browser';
+import * as path from 'path';
+
+/** Type describing possible artifact types for browser downloads. */
+export type ArtifactType = 'driver-bin' | 'browser-bin';
+
+/** Set of known artifact extensions, including chained extensions for gzipped files. */
+const KNOWN_EXTENSIONS = new Set(['zip', 'tar.gz', 'tar.bz2', 'dmg']);
+
+/** Class describing an artifact for a browser.  */
+export class BrowserArtifact {
+  /** Extension of the artifact, derived from the download URL. */
+  extension = getArtifactExtension(this.downloadUrl);
+
+  constructor(
+    /** Instance of the browser this artifact exists for. */
+    public browser: Browser<unknown>,
+    /** Type of the artifact. */
+    public type: ArtifactType,
+    /** URL for downloading the artifact.  */
+    public downloadUrl: string,
+  ) {}
+}
+
+/**
+ * Gets the extension of a given artifact file, excluding the dot/period.
+ *
+ * Since artifact download URLs can use chained extensions as for
+ * example with `.tar.gz`, we will need to keep track of known extensions
+ * and start looking with the first dot/period we discover.
+ */
+function getArtifactExtension(filePath: string) {
+  let tmpPath: string = filePath;
+  let extension: string = '';
+  let currentPart: string = '';
+
+  // Iterate from the end of the path, finding the largest possible
+  // extension substring, accounting for cases like `a/b.tmp/file.tar.gz`.
+  while ((currentPart = path.extname(tmpPath)) !== '') {
+    extension = currentPart + extension;
+    tmpPath = path.basename(tmpPath, currentPart);
+  }
+
+  // Strip off the leading period/dot from the extension.
+  // If there is no extension, this string would remain empty.
+  extension = extension.substring(1);
+
+  if (KNOWN_EXTENSIONS.has(extension)) {
+    return extension;
+  }
+
+  throw new Error(`Unable to find known extension for file path: ${filePath}`);
+}

bazel/browsers/update-script/browser.ts

@@ -6,15 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {BrowserArtifact, ArtifactType} from './browser-artifact';
 import {Platform} from './platform';
 
-/** Type describing possible archive types for browser downloads. */
-export type ArchiveType = 'driver-bin' | 'browser-bin';
-
 /** Interface describing a browser. */
 export interface Browser<T> {
   name: string;
   revision: T;
   supports(platform: Platform): boolean;
-  getDownloadUrl(platform: Platform, type: ArchiveType): string;
+  getArtifact(platform: Platform, type: ArtifactType): BrowserArtifact;
 }

bazel/browsers/update-script/chromium.ts

@@ -6,7 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Browser, ArchiveType} from './browser';
+import {Browser} from './browser';
+import {ArtifactType, BrowserArtifact} from './browser-artifact';
 import {Platform} from './platform';
 
 const cloudStorageArchiveUrl =
@@ -59,13 +60,16 @@ export class Chromium implements Browser<number> {
     return supportedPlatforms.has(platform);
   }
 
-  getDownloadUrl(platform: Platform, type: ArchiveType): string {
-    return Chromium.getDownloadArchiveUrl(this.revision, platform, type);
+  getArtifact(platform: Platform, type: ArtifactType): BrowserArtifact {
+    return new BrowserArtifact(this, type, this.getDownloadUrl(platform, type));
   }
 
-  static getDownloadArchiveUrl(revision: number, platform: Platform, archive: ArchiveType): string {
-    const archiveMap =
-      archive === 'driver-bin' ? PlatformDriverArchiveMap : PlatformBrowserArchiveMap;
+  getDownloadUrl(platform: Platform, type: ArtifactType): string {
+    return Chromium.getDownloadArtifactUrl(this.revision, platform, type);
+  }
+
+  static getDownloadArtifactUrl(revision: number, platform: Platform, type: ArtifactType): string {
+    const archiveMap = type === 'driver-bin' ? PlatformDriverArchiveMap : PlatformBrowserArchiveMap;
     return cloudStorageArchiveUrl
       .replace('{platform}', PlatformSnapshotNameMap[platform])
       .replace('{revision}', `${revision}`)

bazel/browsers/update-script/find-revision-chromium.ts

@@ -28,7 +28,7 @@
 import {createHash} from 'crypto';
 import fetch from 'node-fetch';
 import {Spinner} from '../../../ng-dev/utils/spinner';
-import {ArchiveType} from './browser';
+import {ArtifactType} from './browser-artifact';
 import {Chromium} from './chromium';
 import {Platform} from './platform';
 
@@ -145,7 +145,7 @@ async function isRevisionAvailableForPlatform(
   platform: Platform,
 ): Promise<boolean> {
   // Look for the `driver` archive as this is smaller and faster to check.
-  const response = await fetch(Chromium.getDownloadArchiveUrl(revision, platform, 'driver-bin'));
+  const response = await fetch(Chromium.getDownloadArtifactUrl(revision, platform, 'driver-bin'));
   return response.ok && response.status === 200;
 }
 
@@ -179,9 +179,9 @@ async function getHeadChromiumRevision(): Promise<number> {
 async function getSha256ChecksumForPlatform(
   browser: Chromium,
   platform: Platform,
-  archive: ArchiveType,
+  artifactType: ArtifactType,
 ): Promise<string> {
-  const response = await fetch(browser.getDownloadUrl(platform, archive));
+  const response = await fetch(browser.getDownloadUrl(platform, artifactType));
   const binaryContent = await response.buffer();
   return createHash('sha256').update(binaryContent).digest('hex');
 }

bazel/browsers/update-script/firefox.ts

@@ -6,7 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Browser, ArchiveType} from './browser';
+import {ArtifactType, BrowserArtifact} from './browser-artifact';
+import {Browser} from './browser';
 import {Platform} from './platform';
 
 const downloadLinuxUrls = {
@@ -32,7 +33,7 @@ const downloadMacOsArm64Urls = {
 
 /** Class providing necessary information for the firefox browser. */
 export class Firefox implements Browser<string> {
-  name = 'chromium';
+  name = 'firefox';
 
   constructor(public revision: string, public driverVersion: string) {}
 
@@ -44,14 +45,15 @@ export class Firefox implements Browser<string> {
     );
   }
 
-  getDownloadUrl(platform: Platform, archiveType: ArchiveType): string {
+  getArtifact(platform: Platform, archiveType: ArtifactType): BrowserArtifact {
     const urlSet = this._getUrlSetForPlatform(platform);
     const baseUrl = urlSet[archiveType];
+    const downloadUrl = baseUrl.replace(/\{version}/g, this.revision);
 
-    return baseUrl.replace(/\{version}/g, this.revision);
+    return new BrowserArtifact(this, archiveType, downloadUrl);
   }
 
-  private _getUrlSetForPlatform(platform: Platform): Record<ArchiveType, string> {
+  private _getUrlSetForPlatform(platform: Platform): Record<ArtifactType, string> {
     switch (platform) {
       case Platform.LINUX_X64:
         return downloadLinuxUrls;
@@ -60,7 +62,7 @@ export class Firefox implements Browser<string> {
       case Platform.MAC_ARM64:
         return downloadMacOsArm64Urls;
       default:
-        throw Error('Unexpected platform without Firefox support.');
+        throw Error(`Unexpected platform "${platform}" without Firefox support.`);
     }
   }
 }

bazel/browsers/update-script/index.ts

@@ -35,16 +35,16 @@ async function main() {
           (args) => uploadBrowserArtifactsToMirror(new Storage(), new Chromium(args.revision)),
         )
         .command(
-          'firefox <version> <driver-version>',
+          'firefox <browser-version> <driver-version>',
           'Push Firefox artifacts',
           (args) =>
             args
-              .positional('version', {type: 'string', demandOption: true})
+              .positional('browserVersion', {type: 'string', demandOption: true})
               .positional('driverVersion', {type: 'string', demandOption: true}),
           (args) =>
             uploadBrowserArtifactsToMirror(
               new Storage(),
-              new Firefox(args.version, args.driverVersion),
+              new Firefox(args.browserVersion, args.driverVersion),
             ),
         ),
     )

bazel/browsers/update-script/upload-mirror.ts

@@ -7,11 +7,12 @@
  */
 
 import {Bucket, Storage, File} from '@google-cloud/storage';
-import {Browser, ArchiveType} from './browser';
+import {Browser} from './browser';
 import {Platform} from './platform';
 import {createTmpDir, downloadFileThroughStreaming} from './utils';
 
 import * as path from 'path';
+import {ArtifactType, BrowserArtifact} from './browser-artifact';
 
 /** Name of the Google Cloud Storage bucket for the browser mirror. */
 const MIRROR_BUCKET_NAME = 'dev-infra-mirror';
@@ -21,17 +22,25 @@ export function getMirrorDirectoryForBrowserInstance<T>(browser: Browser<T>): st
   return `${browser.name}/${browser.revision}`;
 }
 
-/** Uploads a browser platform artifact to the browser mirror. */
+/** Gets the destination file path for a given browser artifact. */
+export function getDestinationFilePath(artifact: BrowserArtifact, platform: Platform): string {
+  const versionMirrorDir = getMirrorDirectoryForBrowserInstance(artifact.browser);
+  return `${versionMirrorDir}/${platform}/${artifact.type}.${artifact.extension}`;
+}
+
+/**
+ * Uploads a browser platform artifact to the browser mirror.
+ *
+ * @throws {Error} An error if the artifact already exists in the mirror.
+ */
 export async function uploadArtifactToMirror(
   bucket: Bucket,
-  browser: Browser<unknown>,
+  artifact: BrowserArtifact,
   platform: Platform,
-  type: ArchiveType,
   sourceFile: string,
 ): Promise<File> {
-  const versionMirrorDir = getMirrorDirectoryForBrowserInstance(browser);
   const [file] = await bucket.upload(sourceFile, {
-    destination: `${versionMirrorDir}/${platform}/${type}.zip`,
+    destination: getDestinationFilePath(artifact, platform),
     public: true,
   });
 
@@ -45,62 +54,67 @@ export async function uploadArtifactToMirror(
  */
 export async function uploadBrowserArtifactsToMirror(storage: Storage, browser: Browser<unknown>) {
   const bucket = storage.bucket(MIRROR_BUCKET_NAME);
-  const versionMirrorDir = getMirrorDirectoryForBrowserInstance(browser);
-
-  // Note that the `File#exists` method returns the following: `[boolean]`.
-  // https://googleapis.dev/nodejs/storage/latest/global.html#FileExistsResponse.
-  if ((await bucket.file(versionMirrorDir).exists())[0]) {
-    throw Error('Revision is already in the mirror. Remove the artifacts if you want to retry.');
-  }
-
   const tmpDir = await createTmpDir({template: `${browser.name}-${browser.revision}-XXXXXX`});
   const downloadTasks: Promise<{
     platform: Platform;
     filePath: string;
-    type: ArchiveType;
+    artifact: BrowserArtifact;
   }>[] = [];
 
   for (const platform of Object.values(Platform)) {
-    const driverArchiveUrl = browser.getDownloadUrl(platform, 'driver-bin');
-    const browserArchiveUrl = browser.getDownloadUrl(platform, 'browser-bin');
-    const driverTmpPath = path.join(tmpDir, 'driver.bin');
-    const browserTmpPath = path.join(tmpDir, 'browser.bin');
+    if (!browser.supports(platform)) {
+      continue;
+    }
+
+    const driverArtifact = browser.getArtifact(platform, 'driver-bin');
+    const browserArtifact = browser.getArtifact(platform, 'browser-bin');
+    const driverTmpPath = path.join(tmpDir, `${platform}-driver.${driverArtifact.extension}`);
+    const browserTmpPath = path.join(tmpDir, `${platform}-browser.${browserArtifact.extension}`);
+
+    // We use the driver artifact (which is usually much smaller) to run a quick
+    // sanity check upstream to ensure that the artifact does not yet exist upstream.
+    const testDestinationFile = getDestinationFilePath(driverArtifact, platform);
+    // Note that we cannot check directly for the directory to exist since GCP does
+    // not support this. Hence we need to run this check for the actual files instead.
+    if ((await bucket.file(testDestinationFile).exists())[0]) {
+      throw Error('Revision is already in the mirror. Remove the artifacts if you want to retry.');
+    }
 
     downloadTasks.push(
-      downloadFileThroughStreaming(browserArchiveUrl, browserTmpPath)
+      downloadFileThroughStreaming(browserArtifact.downloadUrl, browserTmpPath)
         .then(() => console.info(`✅ Downloaded: ${browser.name} - ${platform} browser.`))
         .then(() => ({
           platform,
           filePath: browserTmpPath,
-          type: 'browser-bin',
+          artifact: browserArtifact,
         })),
     );
 
     downloadTasks.push(
-      downloadFileThroughStreaming(driverArchiveUrl, driverTmpPath)
+      downloadFileThroughStreaming(driverArtifact.downloadUrl, driverTmpPath)
         .then(() => console.info(`✅ Downloaded: ${browser.name} - ${platform} driver.`))
         .then(() => ({
           platform,
           filePath: driverTmpPath,
-          type: 'driver-bin',
+          artifact: driverArtifact,
         })),
     );
   }
 
   const tasks = await Promise.all(downloadTasks);
-  const uploadTasks: Promise<{platform: Platform; type: ArchiveType; file: File}>[] = [];
+  const uploadTasks: Promise<{platform: Platform; artifact: BrowserArtifact; file: File}>[] = [];
 
   console.info();
   console.info('Fetched all browser artifacts. Now uploading to mirror.');
   console.info();
 
-  for (const {platform, filePath, type} of tasks) {
+  for (const {platform, filePath, artifact} of tasks) {
     uploadTasks.push(
-      uploadArtifactToMirror(bucket, browser, platform, type, filePath).then((file) => {
-        console.log(`✅ Uploaded: ${platform} ${type}`);
+      uploadArtifactToMirror(bucket, artifact, platform, filePath).then((file) => {
+        console.log(`✅ Uploaded: ${platform} ${artifact.type}`);
         console.log(`  -> ${file.publicUrl()}`);
 
-        return {platform, file, type};
+        return {platform, file, artifact};
       }),
     );
   }