perf: download bundletool on demand instead of vendoring it (#6097)

Author: edusperoni

lib/common/http-client.ts

@@ -1,6 +1,7 @@
 import * as _ from "lodash";
 import { EOL } from "os";
 import * as util from "util";
+import { pipeline } from "stream/promises";
 import { Server, IProxySettings, IProxyService } from "./declarations";
 import { injector } from "./yok";
 import axios from "axios";
@@ -18,12 +19,12 @@ export class HttpClient implements Server.IHttpClient {
 	constructor(
 		private $logger: ILogger,
 		private $proxyService: IProxyService,
-		private $staticConfig: Config.IStaticConfig
+		private $staticConfig: Config.IStaticConfig,
 	) {}
 
 	public async httpRequest(
 		options: any,
-		proxySettings?: IProxySettings
+		proxySettings?: IProxySettings,
 	): Promise<Server.IResponse> {
 		try {
 			const result = await this.httpRequestCore(options, proxySettings);
@@ -43,7 +44,7 @@ export class HttpClient implements Server.IHttpClient {
 				this.$logger.warn(
 					"%s Retrying request to %s...",
 					err.message,
-					options.url || options
+					options.url || options,
 				);
 				const retryResult = await this.httpRequestCore(options, proxySettings);
 				return {
@@ -59,7 +60,7 @@ export class HttpClient implements Server.IHttpClient {
 
 	private async httpRequestCore(
 		options: any,
-		proxySettings?: IProxySettings
+		proxySettings?: IProxySettings,
 	): Promise<Server.IResponse> {
 		if (_.isString(options)) {
 			options = {
@@ -79,7 +80,7 @@ export class HttpClient implements Server.IHttpClient {
 			cliProxySettings,
 			options,
 			headers,
-			requestProto
+			requestProto,
 		);
 
 		if (!headers["User-Agent"]) {
@@ -113,7 +114,11 @@ export class HttpClient implements Server.IHttpClient {
 			method: options.method,
 			proxy: false,
 			httpAgent: agent,
+			// axios picks the agent by protocol, so an https:// request ignores httpAgent
+			httpsAgent: agent,
 			data: options.body,
+			responseType: options.pipeTo ? "stream" : undefined,
+			onDownloadProgress: options.onDownloadProgress,
 		}).catch((err) => {
 			this.$logger.trace("An error occurred while sending the request:", err);
 			if (err.response) {
@@ -137,9 +142,18 @@ export class HttpClient implements Server.IHttpClient {
 		if (result) {
 			this.$logger.trace(
 				"httpRequest: Done. code = %d",
-				result.status.toString()
+				result.status.toString(),
 			);
 
+			if (options.pipeTo) {
+				await pipeline(result.data, options.pipeTo);
+
+				return {
+					response: result,
+					headers: result.headers,
+				};
+			}
+
 			return {
 				response: result,
 				body: JSON.stringify(result.data),
@@ -152,7 +166,7 @@ export class HttpClient implements Server.IHttpClient {
 		if (statusCode === HttpStatusCodes.PROXY_AUTHENTICATION_REQUIRED) {
 			const clientNameLowerCase = this.$staticConfig.CLIENT_NAME.toLowerCase();
 			this.$logger.error(
-				`You can run ${EOL}\t${clientNameLowerCase} proxy set <url> <username> <password>.${EOL}In order to supply ${clientNameLowerCase} with the credentials needed.`
+				`You can run ${EOL}\t${clientNameLowerCase} proxy set <url> <username> <password>.${EOL}In order to supply ${clientNameLowerCase} with the credentials needed.`,
 			);
 			return "Your proxy requires authentication.";
 		} else if (statusCode === HttpStatusCodes.PAYMENT_REQUIRED) {
@@ -177,7 +191,7 @@ export class HttpClient implements Server.IHttpClient {
 			} catch (parsingFailed) {
 				this.$logger.trace(
 					"Failed to get error from http request: ",
-					parsingFailed
+					parsingFailed,
 				);
 				return `The server returned unexpected response: ${body}`;
 			}
@@ -199,7 +213,7 @@ export class HttpClient implements Server.IHttpClient {
 		cliProxySettings: IProxySettings,
 		options: any,
 		headers: any,
-		requestProto: string
+		requestProto: string,
 	): Promise<void> {
 		const isLocalRequest =
 			options.host === "localhost" || options.host === "127.0.0.1";

lib/constants.ts

@@ -217,6 +217,15 @@ export const DEBUGGER_ATTACHED_EVENT_NAME = "debuggerAttached";
 export const DEBUGGER_DETACHED_EVENT_NAME = "debuggerDetached";
 export const VERSION_STRING = "version";
 export const INSPECTOR_CACHE_DIRNAME = "ios-inspector";
+export const BUNDLETOOL_CACHE_DIRNAME = "bundletool";
+export const BUNDLETOOL_VERSION = "1.18.2";
+// sha256 of bundletool-all-<BUNDLETOOL_VERSION>.jar as published on GitHub;
+// must be updated together with BUNDLETOOL_VERSION or the download is rejected
+export const BUNDLETOOL_SHA256 =
+	"378b5434cd1378bef6b2bc527b8c7f0ff2584b273830335bce54d6d0813c8584";
+export const BUNDLETOOL_RELEASES_URL =
+	"https://github.com/google/bundletool/releases/download";
+export const BUNDLETOOL_PATH_ENV_VAR = "NS_BUNDLETOOL_PATH";
 export const POST_INSTALL_COMMAND_NAME = "post-install-cli";
 const ANDROID_SIGNING_REQUIRED_MESSAGE =
 	"you need to specify all --key-store-* options.";

lib/services/android/android-bundle-tool-service.ts

@@ -1,31 +1,61 @@
-import { resolve, join } from "path";
+import { join } from "path";
 import * as _ from "lodash";
 import { hasValidAndroidSigning } from "../../common/helpers";
-import { IChildProcess, ISysInfo, IErrors } from "../../common/declarations";
+import {
+	IChildProcess,
+	IErrors,
+	IFileSystem,
+	ISettingsService,
+	ISysInfo,
+	Server,
+} from "../../common/declarations";
 import {
 	IAndroidBundleToolService,
 	IBuildApksOptions,
 	IInstallApksOptions,
 } from "../../definitions/android-bundle-tool-service";
+import { ITerminalSpinnerService } from "../../definitions/terminal-spinner-service";
+import {
+	BUNDLETOOL_CACHE_DIRNAME,
+	BUNDLETOOL_PATH_ENV_VAR,
+	BUNDLETOOL_RELEASES_URL,
+	BUNDLETOOL_SHA256,
+	BUNDLETOOL_VERSION,
+} from "../../constants";
 import { injector } from "../../common/yok";
 
 export class AndroidBundleToolService implements IAndroidBundleToolService {
+	// a cold download of ~32MB can outlast the default 10s lock, and a second
+	// process has to keep waiting rather than pull the same jar again
+	private static LOCK_OPTIONS: ILockOptions = {
+		stale: 5 * 60 * 1000,
+		retriesObj: {
+			retries: 300,
+			minTimeout: 200,
+			maxTimeout: 2000,
+			factor: 1.5,
+		},
+	};
+
 	private javaPath: string;
-	private aabToolPath: string;
+	private bundleToolPathPromise: Promise<string>;
+
 	constructor(
 		private $childProcess: IChildProcess,
 		private $sysInfo: ISysInfo,
-		private $errors: IErrors
-	) {
-		this.aabToolPath = resolve(
-			join(__dirname, "../../../vendor/aab-tool/bundletool.jar")
-		);
-	}
+		private $errors: IErrors,
+		private $fs: IFileSystem,
+		private $httpClient: Server.IHttpClient,
+		private $lockService: ILockService,
+		private $logger: ILogger,
+		private $settingsService: ISettingsService,
+		private $terminalSpinnerService: ITerminalSpinnerService,
+	) {}
 
 	public async buildApks(options: IBuildApksOptions): Promise<void> {
 		if (!hasValidAndroidSigning(options.signingData)) {
 			this.$errors.fail(
-				`Unable to build "apks" without a full signing information.`
+				`Unable to build "apks" without a full signing information.`,
 			);
 		}
 
@@ -46,7 +76,7 @@ export class AndroidBundleToolService implements IAndroidBundleToolService {
 		]);
 		if (aabToolResult.exitCode !== 0 && aabToolResult.stderr) {
 			this.$errors.fail(
-				`Unable to build "apks" from the provided "aab". Error: ${aabToolResult.stderr}`
+				`Unable to build "apks" from the provided "aab". Error: ${aabToolResult.stderr}`,
 			);
 		}
 	}
@@ -61,18 +91,18 @@ export class AndroidBundleToolService implements IAndroidBundleToolService {
 		]);
 		if (aabToolResult.exitCode !== 0 && aabToolResult.stderr) {
 			this.$errors.fail(
-				`Unable to install "apks" on device "${options.deviceId}". Error: ${aabToolResult.stderr}`
+				`Unable to install "apks" on device "${options.deviceId}". Error: ${aabToolResult.stderr}`,
 			);
 		}
 	}
 
 	private async execBundleTool(args: string[]) {
 		const javaPath = await this.getJavaPath();
-		const defaultArgs = ["-jar", this.aabToolPath];
+		const defaultArgs = ["-jar", await this.getBundleToolPath()];
 
 		const result = await this.$childProcess.trySpawnFromCloseEvent(
 			javaPath,
-			_.concat(defaultArgs, args)
+			_.concat(defaultArgs, args),
 		);
 
 		return result;
@@ -85,6 +115,130 @@ export class AndroidBundleToolService implements IAndroidBundleToolService {
 
 		return this.javaPath;
 	}
+
+	private getBundleToolPath(): Promise<string> {
+		this.bundleToolPathPromise =
+			this.bundleToolPathPromise || this.resolveBundleToolPath();
+
+		return this.bundleToolPathPromise;
+	}
+
+	private async resolveBundleToolPath(): Promise<string> {
+		const customPath = process.env[BUNDLETOOL_PATH_ENV_VAR];
+		if (customPath) {
+			if (!this.$fs.exists(customPath)) {
+				this.$errors.fail(
+					`${BUNDLETOOL_PATH_ENV_VAR} is set to "${customPath}", but no file exists there.`,
+				);
+			}
+
+			this.$logger.trace(
+				`Using bundletool from ${BUNDLETOOL_PATH_ENV_VAR}: ${customPath}`,
+			);
+			return customPath;
+		}
+
+		const cacheDir = join(
+			this.$settingsService.getProfileDir(),
+			BUNDLETOOL_CACHE_DIRNAME,
+		);
+		const jarPath = join(cacheDir, `bundletool-all-${BUNDLETOOL_VERSION}.jar`);
+		this.$fs.ensureDirectoryExists(cacheDir);
+
+		if (await this.isExpectedJar(jarPath)) {
+			return jarPath;
+		}
+
+		return this.$lockService.executeActionWithLock(
+			async () => {
+				// whoever held the lock before us may have just downloaded it
+				if (await this.isExpectedJar(jarPath)) {
+					return jarPath;
+				}
+
+				await this.downloadBundleTool(jarPath);
+				return jarPath;
+			},
+			join(cacheDir, `bundletool-${BUNDLETOOL_VERSION}.lock`),
+			AndroidBundleToolService.LOCK_OPTIONS,
+		);
+	}
+
+	private async isExpectedJar(jarPath: string): Promise<boolean> {
+		if (!this.$fs.exists(jarPath)) {
+			return false;
+		}
+
+		const shasum = await this.$fs.getFileShasum(jarPath, {
+			algorithm: "sha256",
+		});
+		if (shasum === BUNDLETOOL_SHA256) {
+			return true;
+		}
+
+		this.$logger.warn(
+			`Cached bundletool at "${jarPath}" does not match the expected checksum and will be downloaded again.`,
+		);
+		this.$fs.deleteFile(jarPath);
+		return false;
+	}
+
+	private async downloadBundleTool(jarPath: string): Promise<void> {
+		const jarName = `bundletool-all-${BUNDLETOOL_VERSION}.jar`;
+		const url = `${BUNDLETOOL_RELEASES_URL}/${BUNDLETOOL_VERSION}/${jarName}`;
+		const tempPath = `${jarPath}.download`;
+		const spinner = this.$terminalSpinnerService.createSpinner();
+
+		spinner.start(`Downloading bundletool ${BUNDLETOOL_VERSION}`);
+
+		try {
+			await this.$httpClient.httpRequest({
+				url,
+				method: "GET",
+				// identity keeps Content-Length equal to the real jar size, so the
+				// progress readout is not skewed by a re-compressed response
+				headers: { "Accept-Encoding": "identity" },
+				pipeTo: this.$fs.createWriteStream(tempPath),
+				onDownloadProgress: (progress: { loaded: number; total?: number }) => {
+					spinner.text = `Downloading bundletool ${BUNDLETOOL_VERSION} ${this.formatProgress(
+						progress,
+					)}`;
+				},
+			});
+		} catch (err) {
+			spinner.fail(`Failed to download bundletool ${BUNDLETOOL_VERSION}`);
+			this.$fs.deleteFile(tempPath);
+			this.$errors.fail(
+				`Unable to download bundletool from "${url}". ` +
+					`Set ${BUNDLETOOL_PATH_ENV_VAR} to the path of a local bundletool jar to skip the download. ` +
+					`Error: ${err.message}`,
+			);
+		}
+
+		const shasum = await this.$fs.getFileShasum(tempPath, {
+			algorithm: "sha256",
+		});
+		if (shasum !== BUNDLETOOL_SHA256) {
+			spinner.fail(`Failed to download bundletool ${BUNDLETOOL_VERSION}`);
+			this.$fs.deleteFile(tempPath);
+			this.$errors.fail(
+				`Checksum mismatch for bundletool downloaded from "${url}". ` +
+					`Expected ${BUNDLETOOL_SHA256}, got ${shasum}.`,
+			);
+		}
+
+		// rename is atomic, so a concurrent reader never sees a partial jar
+		this.$fs.rename(tempPath, jarPath);
+		spinner.succeed(`Downloaded bundletool ${BUNDLETOOL_VERSION}`);
+	}
+
+	private formatProgress(progress: { loaded: number; total?: number }): string {
+		const toMb = (bytes: number) => (bytes / 1024 / 1024).toFixed(1);
+
+		return progress.total
+			? `${toMb(progress.loaded)}/${toMb(progress.total)} MB`
+			: `${toMb(progress.loaded)} MB`;
+	}
 }
 
 injector.register("androidBundleToolService", AndroidBundleToolService);

scripts/copy-assets.js

@@ -48,7 +48,6 @@ function copyFile(sourcePath, targetPath) {
 	const source = fs.statSync(sourcePath);
 	if (fs.existsSync(targetPath)) {
 		const target = fs.statSync(targetPath);
-		// vendor/ alone is ~31MB; re-copying it on every build is pure waste
 		if (target.mtimeMs >= source.mtimeMs && target.size === source.size) {
 			skipped++;
 			return;