From 001593d71e2585c282fd828cf40ad9491caa9b91 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 5 Aug 2026 23:03:35 +0900 Subject: [PATCH 1/5] fix(cli): keep the CodePush bundle in an absolute output directory An absolute --output-path produced an absolute bundle directory, which the './' prefix turned into a path below the current working directory. --- cli/functions/makeCodePushBundle.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/functions/makeCodePushBundle.ts b/cli/functions/makeCodePushBundle.ts index ec948bd42..c21b9c18f 100644 --- a/cli/functions/makeCodePushBundle.ts +++ b/cli/functions/makeCodePushBundle.ts @@ -15,8 +15,10 @@ export async function makeCodePushBundle(contentsPath: string, bundleDirectory: const packageHash = await generatePackageHashFromDirectory(contentsPath, path.join(contentsPath, '..')); - shell.mkdir('-p', `./${bundleDirectory}`); - shell.mv(updateContentsZipPath, `./${bundleDirectory}/${packageHash}`); + // Joined rather than interpolated so an absolute bundle directory - which an + // absolute --output-path produces - is not turned into a path below the cwd. + shell.mkdir('-p', bundleDirectory); + shell.mv(updateContentsZipPath, path.join(bundleDirectory, packageHash)); return { // To allow the "release" command to get the file and hash value from the result of the "bundle" command, From c5eb9f7390443b87c2bfa39d830621704a412b46 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 5 Aug 2026 23:03:50 +0900 Subject: [PATCH 2/5] feat(cli): generate platform binary patch artifacts `bundle` and `release` accept --binary-bundle-path, the JS bundle of the target binary. With it, `release` publishes two artifacts per platform: the full bundle named after its packageHash, and `-patch.zip`, which carries the target bundle only as a patch against the binary's bundle plus a `codepush-binary-patch.json` manifest. Every other file is copied unchanged, so applying the patch and dropping the two patch-only files reproduces the full contents byte for byte - and therefore the same packageHash. The Hermes compilation is aligned with the base bundle through `-base-bytecode` when the app's own compiler advertises the flag, which is what keeps the patch small. A compiler without the flag only warns; a compilation that fails with it fails the release, since the base is then wrong. Both archive sizes and the saving are printed before anything is uploaded, and the full bundle is uploaded before the patch. A failed upload of either leaves the release history untouched. Carrying the patch URL in the release history is deliberately not part of this change. --- cli/README.ko.md | 14 + cli/README.md | 15 + cli/commands/bundleCommand/bundleCodePush.ts | 44 +- cli/commands/bundleCommand/index.ts | 7 + cli/commands/releaseCommand/index.ts | 7 + cli/commands/releaseCommand/release.test.ts | 355 ++++++++++++++++ cli/commands/releaseCommand/release.ts | 149 ++++++- cli/functions/makeBinaryPatchBundle.test.ts | 402 ++++++++++++++++++ cli/functions/makeBinaryPatchBundle.ts | 316 ++++++++++++++ .../runHermesEmitBinaryCommand.test.ts | 124 ++++++ cli/functions/runHermesEmitBinaryCommand.ts | 58 ++- 11 files changed, 1470 insertions(+), 21 deletions(-) create mode 100644 cli/commands/releaseCommand/release.test.ts create mode 100644 cli/functions/makeBinaryPatchBundle.test.ts create mode 100644 cli/functions/makeBinaryPatchBundle.ts create mode 100644 cli/functions/runHermesEmitBinaryCommand.test.ts diff --git a/cli/README.ko.md b/cli/README.ko.md index 32e3277e8..de283925c 100644 --- a/cli/README.ko.md +++ b/cli/README.ko.md @@ -66,12 +66,16 @@ npx code-push bundle [options] | `-b, --bundle-name ` | 번들 파일 이름 | `main.jsbundle` (iOS) / `index.android.bundle` (Android) | | `--output-bundle-dir ` | 번들 출력 디렉토리 이름 | `bundleOutput` | | `--output-metro-dir ` | Hermes 컴파일 전 Metro JS 번들과 소스맵을 복사할 디렉토리 | — | +| `--binary-bundle-path ` | 대상 바이너리에 포함된 JS 번들 경로. Hermes 컴파일을 이 번들에 정렬하고, binary patch base로 기록합니다 | — | **예시:** ```bash # Android용 번들 생성 (커스텀 엔트리 파일) npx code-push bundle -p android -e index.js + +# 바이너리에 포함된 JS 번들에 정렬하여 번들 생성 +npx code-push bundle -p android --binary-bundle-path ./binary/index.android.bundle ``` --- @@ -103,6 +107,13 @@ npx code-push release [options] | `--skip-cleanup ` | 출력 디렉토리 정리 건너뛰기 | `false` | | `--output-bundle-dir ` | 번들 출력 디렉토리 이름 | `bundleOutput` | | `--output-metro-dir ` | Hermes 컴파일 전 Metro JS 번들과 소스맵을 복사할 디렉토리 | — | +| `--binary-bundle-path ` | 대상 바이너리에 포함된 JS 번들 경로. 이 번들에 대한 binary patch 번들을 함께 배포하고, Hermes 컴파일을 이 번들에 정렬합니다 | — | + +`--binary-bundle-path`를 사용하면 플랫폼별로 두 개의 artifact를 업로드합니다. `packageHash` +이름의 full 번들과, 바이너리에 포함된 번들과의 차이만 담은 `-patch.zip` patch +번들입니다. patch 번들에는 업데이트 복원 방법을 담은 `codepush-binary-patch.json` manifest가 +포함되어, patch를 적용하면 full 번들과 동일한 `packageHash`가 됩니다. 두 artifact의 크기와 +절감량은 업로드 전에 출력됩니다. **예시:** @@ -121,6 +132,9 @@ npx code-push release -b 1.0.0 -v 1.0.1 -i staging # 번들링 건너뛰기 (기존 번들 재사용) npx code-push release -b 1.0.0 -v 1.0.2 --skip-bundle true --hash-calc true + +# full 번들과 바이너리 번들에 대한 binary patch를 함께 배포 +npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle ``` --- diff --git a/cli/README.md b/cli/README.md index 1d95c09cd..cb41e4e90 100644 --- a/cli/README.md +++ b/cli/README.md @@ -66,10 +66,14 @@ npx code-push bundle [options] | `-b, --bundle-name ` | Bundle file name | `main.jsbundle` (iOS) / `index.android.bundle` (Android) | | `--output-bundle-dir ` | Directory name for the bundle output | `bundleOutput` | | `--output-metro-dir ` | Directory to copy Metro JS bundle and sourcemap before Hermes compilation | — | +| `--binary-bundle-path ` | JS bundle of the target binary. Aligns the Hermes compilation with it and records it as the binary patch base | — | ```bash # Bundle for Android with a custom entry file npx code-push bundle -p android -e index.js + +# Bundle aligned with the JS bundle shipped in the binary +npx code-push bundle -p android --binary-bundle-path ./binary/index.android.bundle ``` --- @@ -101,6 +105,14 @@ npx code-push release [options] | `--skip-cleanup ` | Skip output directory cleanup | `false` | | `--output-bundle-dir ` | Bundle output directory name | `bundleOutput` | | `--output-metro-dir ` | Directory to copy Metro JS bundle and sourcemap before Hermes compilation | — | +| `--binary-bundle-path ` | JS bundle of the target binary. Releases an additional binary patch bundle against it, and aligns the Hermes compilation with it | — | + +With `--binary-bundle-path`, the release uploads two artifacts per platform: the full +bundle named after its `packageHash`, and a patch bundle named `-patch.zip` +that carries only the difference from the bundle inside the binary. The patch bundle +holds a `codepush-binary-patch.json` manifest describing how to rebuild the update, so +applying it yields the same `packageHash` as the full bundle. Both sizes and the saving +are printed before either artifact is uploaded. ```bash # Standard iOS release @@ -117,6 +129,9 @@ npx code-push release -b 1.0.0 -v 1.0.1 -i staging # Reuse an existing bundle npx code-push release -b 1.0.0 -v 1.0.2 --skip-bundle true --hash-calc true + +# Release a full bundle and a binary patch against the bundle in the binary +npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle ``` --- diff --git a/cli/commands/bundleCommand/bundleCodePush.ts b/cli/commands/bundleCommand/bundleCodePush.ts index 3caf95b71..3b80c53ee 100644 --- a/cli/commands/bundleCommand/bundleCodePush.ts +++ b/cli/commands/bundleCommand/bundleCodePush.ts @@ -4,12 +4,32 @@ import { prepareToBundleJS } from "../../functions/prepareToBundleJS.js"; import { runReactNativeBundleCommand } from "../../functions/runReactNativeBundleCommand.js"; import { runExpoBundleCommand } from "../../functions/runExpoBundleCommand.js"; import { getReactTempDir } from "../../functions/getReactTempDir.js"; -import { runHermesEmitBinaryCommand } from "../../functions/runHermesEmitBinaryCommand.js"; +import { resolveBaseBytecodeHermesFlags, runHermesEmitBinaryCommand } from "../../functions/runHermesEmitBinaryCommand.js"; import { makeCodePushBundle } from "../../functions/makeCodePushBundle.js"; +import { hashBundleFile, writeBinaryPatchBaseRecord } from "../../functions/makeBinaryPatchBundle.js"; import { ROOT_OUTPUT_DIR, ENTRY_FILE } from "../../constant.js"; +export type CodePushBundleResult = { + /** CodePush bundle file name (equals to packageHash) */ + bundleFileName: string; + /** Directory holding the files that were packed into the CodePush bundle file */ + contentsPath: string; + /** JS bundle file name inside the contents directory */ + jsBundleName: string; +}; + /** - * @return {Promise} CodePush bundle file name (equals to packageHash) + * JS bundle file name react-native writes, which is also the name the app looks for + * inside an update, so it has to be decided the same way everywhere. + */ +export function resolveJsBundleName(platform: 'ios' | 'android', jsBundleName?: string): string { + const DEFAULT_JS_BUNDLE_NAME = platform === 'ios' ? 'main.jsbundle' : 'index.android.bundle'; + return jsBundleName || DEFAULT_JS_BUNDLE_NAME; +} + +/** + * @param baseBundlePath {string} JS bundle from the target binary. When given, the compilation is aligned with it and the base is recorded for a later `release`. + * @return {Promise} CodePush bundle file name (equals to packageHash) and the contents it was made of */ export async function bundleCodePush( framework: 'expo' | undefined, @@ -19,14 +39,14 @@ export async function bundleCodePush( jsBundleName: string, // JS bundle file name (not CodePush bundle file) bundleDirectory: string, // CodePush bundle output directory outputMetroDir?: string, -): Promise { + baseBundlePath?: string, +): Promise { if (fs.existsSync(outputRootPath)) { fs.rmSync(outputRootPath, { recursive: true }); } const OUTPUT_CONTENT_PATH = `${outputRootPath}/CodePush`; - const DEFAULT_JS_BUNDLE_NAME = platform === 'ios' ? 'main.jsbundle' : 'index.android.bundle'; - const _jsBundleName = jsBundleName || DEFAULT_JS_BUNDLE_NAME; // react-native JS bundle output name + const _jsBundleName = resolveJsBundleName(platform, jsBundleName); // react-native JS bundle output name const SOURCEMAP_OUTPUT = `${outputRootPath}/${_jsBundleName}.map`; prepareToBundleJS({ deleteDirs: [outputRootPath, getReactTempDir()], makeDir: OUTPUT_CONTENT_PATH }); @@ -57,13 +77,25 @@ export async function bundleCodePush( _jsBundleName, OUTPUT_CONTENT_PATH, SOURCEMAP_OUTPUT, + baseBundlePath ? resolveBaseBytecodeHermesFlags(baseBundlePath) : [], ); console.log('log: Hermes compilation complete'); const { bundleFileName: codePushBundleFileName } = await makeCodePushBundle(OUTPUT_CONTENT_PATH, bundleDirectory); console.log(`log: CodePush bundle created (file path: ./${bundleDirectory}/${codePushBundleFileName})`); - return codePushBundleFileName; + if (baseBundlePath) { + // Written after the bundle file, and to the output root instead of the update + // contents, so recording the base cannot change what was just packed or its hash. + const recordPath = writeBinaryPatchBaseRecord(outputRootPath, hashBundleFile(baseBundlePath)); + console.log(`log: Binary patch base recorded (file path: ${recordPath})`); + } + + return { + bundleFileName: codePushBundleFileName, + contentsPath: OUTPUT_CONTENT_PATH, + jsBundleName: _jsBundleName, + }; } function copyMetroOutputsIfNeeded( diff --git a/cli/commands/bundleCommand/index.ts b/cli/commands/bundleCommand/index.ts index e1b8ffaf6..33cc9ed7c 100644 --- a/cli/commands/bundleCommand/index.ts +++ b/cli/commands/bundleCommand/index.ts @@ -1,5 +1,6 @@ import { program, Option } from "commander"; import { bundleCodePush } from "./bundleCodePush.js"; +import { resolveBinaryBundlePathOption } from "../../functions/makeBinaryPatchBundle.js"; import { OUTPUT_BUNDLE_DIR, ROOT_OUTPUT_DIR, ENTRY_FILE } from "../../constant.js"; type Options = { @@ -10,6 +11,7 @@ type Options = { bundleName: string; outputBundleDir: string; outputMetroDir?: string; + binaryBundlePath?: string; } program.command('bundle') @@ -21,7 +23,11 @@ program.command('bundle') .option('-b, --bundle-name ', 'bundle file name (default-ios: "main.jsbundle" / default-android: "index.android.bundle")') .option('--output-metro-dir ', 'name of directory to copy the Metro JS bundle and sourcemap before Hermes compilation') .option('--output-bundle-dir ', 'name of directory containing the bundle file created by the "bundle" command', OUTPUT_BUNDLE_DIR) + .option('--binary-bundle-path ', 'path to the JS bundle of the target binary. Aligns the Hermes compilation with it and records it as the binary patch base.') .action((options: Options) => { + // Resolved before bundling so a wrong path fails now rather than after a build. + const baseBundlePath = resolveBinaryBundlePathOption(options.binaryBundlePath); + bundleCodePush( options.framework, options.platform, @@ -30,5 +36,6 @@ program.command('bundle') options.bundleName, `${options.outputPath}/${options.outputBundleDir}`, options.outputMetroDir, + baseBundlePath, ) }); diff --git a/cli/commands/releaseCommand/index.ts b/cli/commands/releaseCommand/index.ts index ec7e7b77e..8714f9b0a 100644 --- a/cli/commands/releaseCommand/index.ts +++ b/cli/commands/releaseCommand/index.ts @@ -1,6 +1,7 @@ import { program, Option } from "commander"; import { findAndReadConfigFile } from "../../utils/fsUtils.js"; import { release } from "./release.js"; +import { resolveBinaryBundlePathOption } from "../../functions/makeBinaryPatchBundle.js"; import { OUTPUT_BUNDLE_DIR, CONFIG_FILE_NAME, ROOT_OUTPUT_DIR, ENTRY_FILE } from "../../constant.js"; type Options = { @@ -21,6 +22,7 @@ type Options = { outputBundleDir: string; outputMetroDir?: string; hashCalc?: boolean; + binaryBundlePath?: string; } program.command('release') @@ -42,6 +44,7 @@ program.command('release') .option('--skip-cleanup ', 'skip cleanup process', parseBoolean, false) .option('--output-metro-dir ', 'name of directory to copy the Metro JS bundle and sourcemap before Hermes compilation') .option('--output-bundle-dir ', 'name of directory containing the bundle file created by the "bundle" command', OUTPUT_BUNDLE_DIR) + .option('--binary-bundle-path ', 'path to the JS bundle of the target binary. Releases an additional binary patch bundle against it, and aligns the Hermes compilation with it.') .action(async (options: Options) => { const config = findAndReadConfigFile(process.cwd(), options.config); @@ -55,6 +58,9 @@ program.command('release') process.exit(1); } + // Resolved before bundling so a wrong path fails now rather than after a build. + const baseBundlePath = resolveBinaryBundlePathOption(options.binaryBundlePath); + await release( config.bundleUploader, config.getReleaseHistory, @@ -75,6 +81,7 @@ program.command('release') `${options.outputPath}/${options.outputBundleDir}`, options.outputMetroDir, options.hashCalc, + baseBundlePath, ) console.log('🚀 Release completed.') diff --git a/cli/commands/releaseCommand/release.test.ts b/cli/commands/releaseCommand/release.test.ts new file mode 100644 index 000000000..c07d54228 --- /dev/null +++ b/cli/commands/releaseCommand/release.test.ts @@ -0,0 +1,355 @@ +import fs from "fs"; +import os from "os"; +import path from "path"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, jest } from "@jest/globals"; +import { release } from "./release.js"; +import { makeCodePushBundle } from "../../functions/makeCodePushBundle.js"; +import { + BINARY_PATCH_ARCHIVE_SUFFIX, + BINARY_PATCH_MANIFEST_NAME, + hashBundleFile, + writeBinaryPatchBaseRecord, +} from "../../functions/makeBinaryPatchBundle.js"; +import { applyPatch } from "../../utils/binaryPatch.js"; +import { generatePackageHashFromDirectory } from "../../utils/hash-utils.js"; +import { unzip } from "../../utils/unzip.js"; +import type { ReleaseHistoryInterface } from "../../../typings/react-native-code-push.d.ts"; + +/** + * Covers the release flow around the binary patch option with `--skip-bundle`, which is + * the one path that reaches every decision - which artifacts exist, in which order they + * are uploaded, what the history ends up saying - without running the bundler. + */ + +/** Filled in by `bundle`, so `release --skip-bundle` starts from a real archive. */ +const CONTENTS_DIR_NAME = 'CodePush'; +const OUTPUT_DIR_NAME = 'build'; +const BUNDLE_OUTPUT_DIR_NAME = 'bundleOutput'; + +const BINARY_VERSION = '9.9.9'; +const APP_VERSION = '9.9.10'; + +function findRepoRoot(): string { + let dir = process.cwd(); + for (;;) { + if (fs.existsSync(path.join(dir, "scripts", "binary-patch", "build-hdiffpatch.sh"))) { + return dir; + } + const parent = path.dirname(dir); + if (parent === dir) { + throw new Error(`cannot locate the repository root from ${process.cwd()}`); + } + dir = parent; + } +} + +const repoRoot = findRepoRoot(); +const fixtureDir = path.join(repoRoot, "cli", "fixtures", "binary-patch"); +const baseFixture = path.join(fixtureDir, "base.bundle"); +const targetFixture = path.join(fixtureDir, "target.bundle"); + +let workDir: string; + +type StagedBundle = { + outputPath: string; + bundleDirectory: string; + bundleFileName: string; +}; + +/** + * Leaves an output directory in the state a finished `bundle` run leaves it in: the + * bundle file, and no update contents - the contents are the bundler's scratch space, + * and `--skip-bundle` cannot assume they survived. + */ +async function stageBundleOutput( + caseName: string, + files: Record = { 'main.jsbundle': fs.readFileSync(targetFixture) }, +): Promise { + const caseDir = fs.mkdtempSync(path.join(workDir, `${caseName}-`)); + const outputPath = path.join(caseDir, OUTPUT_DIR_NAME); + const contentsPath = path.join(outputPath, CONTENTS_DIR_NAME); + + for (const [relativePath, content] of Object.entries(files)) { + const filePath = path.join(contentsPath, relativePath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + } + + const bundleDirectory = path.join(outputPath, BUNDLE_OUTPUT_DIR_NAME); + const { bundleFileName } = await makeCodePushBundle(contentsPath, bundleDirectory); + fs.rmSync(contentsPath, { recursive: true, force: true }); + + return { outputPath, bundleDirectory, bundleFileName }; +} + +class ProcessExitError extends Error { + constructor(readonly code: number) { + super(`process.exit(${code})`); + } +} + +type Uploads = { filePath: string, downloadUrl: string, logCountBeforeUpload: number }[]; + +function recordingUploader(uploads: Uploads, failOn?: (filePath: string) => boolean) { + return async (filePath: string) => { + if (failOn?.(filePath)) { + throw new Error(`upload rejected: ${path.basename(filePath)}`); + } + const downloadUrl = `https://cdn.example.com/${path.basename(filePath)}`; + uploads.push({ filePath, downloadUrl, logCountBeforeUpload: logs.length }); + return { downloadUrl }; + }; +} + +function historyStore() { + const saved: ReleaseHistoryInterface[] = []; + return { + saved, + getReleaseHistory: async () => ({}) as ReleaseHistoryInterface, + setReleaseHistory: async ( + _binaryVersion: string, + _jsonFilePath: string, + releaseInfo: ReleaseHistoryInterface, + ) => { + saved.push(releaseInfo); + }, + }; +} + +type ReleaseOverrides = { + binaryBundlePath?: string; + platform?: 'ios' | 'android'; + jsBundleName?: string; + skipCleanup?: boolean; + uploadFailsFor?: (filePath: string) => boolean; +}; + +async function runRelease(staged: StagedBundle, overrides: ReleaseOverrides = {}) { + const uploads: Uploads = []; + const history = historyStore(); + + await release( + recordingUploader(uploads, overrides.uploadFailsFor), + history.getReleaseHistory, + history.setReleaseHistory, + BINARY_VERSION, + APP_VERSION, + undefined, + overrides.platform ?? 'ios', + undefined, + staged.outputPath, + 'index.ts', + overrides.jsBundleName ?? '', + false, + true, + undefined, + true, // skipBundle + overrides.skipCleanup ?? true, + staged.bundleDirectory, + undefined, + undefined, + overrides.binaryBundlePath, + ); + + return { uploads, releaseHistories: history.saved }; +} + +let logs: string[]; + +beforeAll(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), "codepush-release-")); +}); + +afterAll(() => { + fs.rmSync(workDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + logs = []; + const collect = (...args: unknown[]) => { + logs.push(args.map(String).join(' ')); + }; + jest.spyOn(console, 'log').mockImplementation(collect); + jest.spyOn(console, 'warn').mockImplementation(collect); + jest.spyOn(console, 'error').mockImplementation(collect); + jest.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new ProcessExitError(code ?? 0); + }) as never); +}); + +afterEach(() => { + jest.restoreAllMocks(); + // addToReleaseHistory writes its JSON next to the invocation, and only removes it + // when the history was stored successfully. + fs.rmSync(path.resolve(process.cwd(), `${BINARY_VERSION}.json`), { force: true }); +}); + +describe("release without --binary-bundle-path", () => { + it("uploads the full bundle only, exactly as before binary patches existed", async () => { + const staged = await stageBundleOutput("full-only"); + + const { uploads, releaseHistories } = await runRelease(staged); + + expect(uploads).toHaveLength(1); + expect(uploads[0].filePath).toBe(`${staged.bundleDirectory}/${staged.bundleFileName}`); + expect(fs.readdirSync(staged.bundleDirectory)).toEqual([staged.bundleFileName]); + expect(releaseHistories[0][APP_VERSION]).toEqual({ + enabled: true, + mandatory: false, + downloadUrl: uploads[0].downloadUrl, + packageHash: staged.bundleFileName, + }); + }); +}); + +describe("release --skip-bundle --binary-bundle-path", () => { + it("patches the bundle that is already in the output directory and uploads both artifacts", async () => { + const staged = await stageBundleOutput("skip-bundle"); + + const { uploads, releaseHistories } = await runRelease(staged, { binaryBundlePath: baseFixture }); + + const patchFileName = `${staged.bundleFileName}${BINARY_PATCH_ARCHIVE_SUFFIX}`; + // The full archive goes first: the patch is an optimisation on top of it. + expect(uploads.map(({ filePath }) => path.basename(filePath))).toEqual([staged.bundleFileName, patchFileName]); + expect(fs.existsSync(path.join(staged.bundleDirectory, patchFileName))).toBe(true); + + // Carrying the patch URL in the release history is a separate concern; for now + // the history keeps describing the full bundle only. + expect(releaseHistories[0][APP_VERSION].downloadUrl).toBe(uploads[0].downloadUrl); + expect(releaseHistories[0][APP_VERSION].packageHash).toBe(staged.bundleFileName); + }); + + it("prints the size summary before uploading anything", async () => { + const staged = await stageBundleOutput("summary"); + + const { uploads } = await runRelease(staged, { binaryBundlePath: baseFixture }); + + const summaryIndex = logs.findIndex((line) => line.startsWith('Binary patch summary (ios)')); + expect(summaryIndex).toBeGreaterThanOrEqual(0); + expect(logs[summaryIndex]).toContain('Saved:'); + // Printed while the release can still be stopped, so before the first upload. + expect(uploads).toHaveLength(2); + expect(summaryIndex).toBeLessThan(uploads[0].logCountBeforeUpload); + }); + + it("uses the platform's own bundle name when picking the target to patch", async () => { + const staged = await stageBundleOutput("android", { + 'index.android.bundle': fs.readFileSync(targetFixture), + }); + + await runRelease(staged, { binaryBundlePath: baseFixture, platform: 'android' }); + + expect(logs.some((line) => line.startsWith('Binary patch summary (android)'))).toBe(true); + }); + + it("fails with an actionable error when the released bundle holds no matching JS bundle", async () => { + const staged = await stageBundleOutput("no-target", { + 'index.android.bundle': fs.readFileSync(targetFixture), + }); + + await expect(runRelease(staged, { binaryBundlePath: baseFixture, platform: 'ios' })).rejects.toThrow( + /main\.jsbundle.*--js-bundle-name/s, + ); + }); + + it("warns when the bundle was compiled against a different base bundle", async () => { + const staged = await stageBundleOutput("mismatch"); + writeBinaryPatchBaseRecord(staged.outputPath, 'f'.repeat(64)); + + await runRelease(staged, { binaryBundlePath: baseFixture }); + + expect(logs.filter((line) => line.startsWith('warn:')).join('\n')).toMatch(/compiled against base bundle/); + }); + + it("does not warn when the recorded base bundle is the one being patched against", async () => { + const staged = await stageBundleOutput("match"); + // The record the `bundle` command writes holds the hash of this same file. + writeBinaryPatchBaseRecord(staged.outputPath, hashBundleFile(baseFixture)); + + await runRelease(staged, { binaryBundlePath: baseFixture }); + + expect(logs.filter((line) => line.startsWith('warn:'))).toEqual([]); + }); + + it("keeps the release history untouched when the patch archive cannot be uploaded", async () => { + const staged = await stageBundleOutput("patch-upload-failure"); + + const uploads: Uploads = []; + const history = historyStore(); + + await expect( + release( + recordingUploader(uploads, (filePath) => filePath.endsWith(BINARY_PATCH_ARCHIVE_SUFFIX)), + history.getReleaseHistory, + history.setReleaseHistory, + BINARY_VERSION, + APP_VERSION, + undefined, + 'ios', + undefined, + staged.outputPath, + 'index.ts', + '', + false, + true, + undefined, + true, + true, + staged.bundleDirectory, + undefined, + undefined, + baseFixture, + ), + ).rejects.toThrow(ProcessExitError); + + expect(uploads.map(({ filePath }) => path.basename(filePath))).toEqual([staged.bundleFileName]); + expect(history.saved).toEqual([]); + }); + + it("leaves no working directories behind and cleans up the output when asked", async () => { + const staged = await stageBundleOutput("cleanup"); + + await runRelease(staged, { binaryBundlePath: baseFixture, skipCleanup: true }); + + expect(fs.readdirSync(staged.outputPath)).toEqual([BUNDLE_OUTPUT_DIR_NAME]); + + await runRelease(staged, { binaryBundlePath: baseFixture, skipCleanup: false }); + + expect(fs.existsSync(staged.outputPath)).toBe(false); + }); + + it("still finds the bundle to release when a previous run left its patch archive behind", async () => { + const staged = await stageBundleOutput("leftover-patch"); + fs.writeFileSync(path.join(staged.bundleDirectory, `stale${BINARY_PATCH_ARCHIVE_SUFFIX}`), 'stale'); + + const { uploads } = await runRelease(staged, { binaryBundlePath: baseFixture }); + + expect(path.basename(uploads[0].filePath)).toBe(staged.bundleFileName); + }); + + it("ships a patch archive a client can turn back into the released bundle", async () => { + const staged = await stageBundleOutput("manifest", { + 'main.jsbundle': fs.readFileSync(targetFixture), + 'assets/logo.png': Buffer.from('logo-bytes'), + }); + + await runRelease(staged, { binaryBundlePath: baseFixture }); + + const extractRoot = path.join(staged.outputPath, 'extracted'); + fs.mkdirSync(extractRoot, { recursive: true }); + await unzip(path.join(staged.bundleDirectory, `${staged.bundleFileName}${BINARY_PATCH_ARCHIVE_SUFFIX}`), extractRoot); + + const contents = path.join(extractRoot, CONTENTS_DIR_NAME); + const manifest = JSON.parse(fs.readFileSync(path.join(contents, BINARY_PATCH_MANIFEST_NAME), 'utf8')) as { + bundlePath: string, + patchFile: string, + }; + expect(manifest.bundlePath).toBe('main.jsbundle'); + + applyPatch(baseFixture, path.join(contents, manifest.patchFile), path.join(contents, manifest.bundlePath)); + fs.rmSync(path.join(contents, manifest.patchFile)); + fs.rmSync(path.join(contents, BINARY_PATCH_MANIFEST_NAME)); + + expect(await generatePackageHashFromDirectory(contents, extractRoot)).toBe(staged.bundleFileName); + }); +}); diff --git a/cli/commands/releaseCommand/release.ts b/cli/commands/releaseCommand/release.ts index 521dfb596..f1a602c66 100644 --- a/cli/commands/releaseCommand/release.ts +++ b/cli/commands/releaseCommand/release.ts @@ -1,10 +1,19 @@ import fs from "fs"; import path from "path"; -import { bundleCodePush } from "../bundleCommand/bundleCodePush.js"; +import { bundleCodePush, resolveJsBundleName } from "../bundleCommand/bundleCodePush.js"; import { addToReleaseHistory } from "./addToReleaseHistory.js"; import type { CliConfigInterface } from "../../../typings/react-native-code-push.d.ts"; import { generatePackageHashFromDirectory } from "../../utils/hash-utils.js"; import { unzip } from "../../utils/unzip.js"; +import { + BINARY_PATCH_ARCHIVE_SUFFIX, + extractCodePushBundleContents, + formatBinaryPatchSummary, + hashBundleFile, + makeBinaryPatchBundle, + readBinaryPatchBaseRecord, + type BinaryPatchBundle, +} from "../../functions/makeBinaryPatchBundle.js"; export async function release( bundleUploader: CliConfigInterface['bundleUploader'], @@ -26,10 +35,12 @@ export async function release( bundleDirectory: string, outputMetroDir?: string, hashCalc?: boolean, + baseBundlePath?: string, ): Promise { - const bundleFileName = skipBundle - ? readBundleFileNameFrom(bundleDirectory) - : await bundleCodePush(framework, platform, outputPath, entryFile, jsBundleName, bundleDirectory, outputMetroDir); + const codePushBundle = skipBundle + ? null + : await bundleCodePush(framework, platform, outputPath, entryFile, jsBundleName, bundleDirectory, outputMetroDir, baseBundlePath); + const bundleFileName = codePushBundle?.bundleFileName ?? readBundleFileNameFrom(bundleDirectory); const bundleFilePath = `${bundleDirectory}/${bundleFileName}`; const packageHash = await (() => { @@ -40,15 +51,26 @@ export async function release( return bundleFileName; })(); - const downloadUrl = await (async () => { - try { - const { downloadUrl } = await bundleUploader(bundleFilePath, platform, identifier); - return downloadUrl - } catch (error) { - console.error('Failed to upload the bundle file. Exiting the program.\n', error) - process.exit(1) - } - })(); + const binaryPatch = baseBundlePath + ? await makeBinaryPatchArtifact({ + baseBundlePath, + contentsPath: codePushBundle?.contentsPath, + bundleFilePath, + jsBundleName: codePushBundle?.jsBundleName ?? resolveJsBundleName(platform, jsBundleName), + bundleDirectory, + packageHash, + outputPath, + platform, + }) + : null; + + // Every artifact is uploaded before the release history is touched, so a failed + // upload leaves the history describing only updates that can actually be downloaded. + const downloadUrl = await uploadArtifact(bundleUploader, bundleFilePath, platform, identifier, 'bundle'); + if (binaryPatch) { + const patchDownloadUrl = await uploadArtifact(bundleUploader, binaryPatch.patchBundleFilePath, platform, identifier, 'binary patch bundle'); + console.log(`log: Binary patch archive uploaded (download url: ${patchDownloadUrl})`); + } await addToReleaseHistory( appVersion, @@ -74,7 +96,9 @@ function cleanUpOutputs(dir: string) { } function readBundleFileNameFrom(bundleDirectory: string): string { - const files = fs.readdirSync(bundleDirectory); + // A previous release of the same bundle may have left its patch archive here, and + // that archive is derived from the bundle file rather than a candidate for release. + const files = fs.readdirSync(bundleDirectory).filter((file) => !file.endsWith(BINARY_PATCH_ARCHIVE_SUFFIX)); if (files.length !== 1) { console.error('The bundlePath must contain only one file.'); process.exit(1); @@ -101,3 +125,100 @@ async function calcHashFromBundleFile(bundleFilePath: string): Promise { fs.rmSync(tempDir, { recursive: true, force: true }); } } + +/** + * Builds the binary patch artifact of this release and reports what it saves before + * anything is uploaded, so an unexpectedly large patch can still be stopped. + * + * @param contentsPath {string | undefined} Update contents of a bundle that was just built. Absent with `--skip-bundle`, where the bundle file being released is unpacked instead, so the patch describes exactly the bytes that go out. + */ +async function makeBinaryPatchArtifact({ + baseBundlePath, + contentsPath, + bundleFilePath, + jsBundleName, + bundleDirectory, + packageHash, + outputPath, + platform, +}: { + baseBundlePath: string; + contentsPath: string | undefined; + bundleFilePath: string; + jsBundleName: string; + bundleDirectory: string; + packageHash: string; + outputPath: string; + platform: 'ios' | 'android'; +}): Promise { + warnOnBaseBundleMismatch(outputPath, baseBundlePath); + + let patchContentsPath = contentsPath; + let extractDir: string | null = null; + if (patchContentsPath === undefined) { + const extracted = await extractCodePushBundleContents(bundleFilePath, outputPath); + patchContentsPath = extracted.contentsPath; + extractDir = extracted.extractDir; + } + + try { + const binaryPatch = await makeBinaryPatchBundle({ + contentsPath: patchContentsPath, + baseBundlePath, + bundleRelativePath: jsBundleName, + bundleDirectory, + packageHash, + }); + + console.log(formatBinaryPatchSummary({ + platform, + baseBundleHash: binaryPatch.manifest.baseBundleHash, + targetBundleHash: binaryPatch.manifest.targetBundleHash, + fullArchiveSize: fs.statSync(bundleFilePath).size, + patchArchiveSize: fs.statSync(binaryPatch.patchBundleFilePath).size, + })); + + return binaryPatch; + } finally { + if (extractDir) { + fs.rmSync(extractDir, { recursive: true, force: true }); + } + } +} + +/** + * The bundle being released may have been compiled by an earlier `bundle` run against a + * different base. The patch stays valid - it is always computed against the base given + * here - but the bytecode alignment that keeps it small is lost, which is worth saying + * out loud instead of leaving it to be noticed in the size summary. + */ +function warnOnBaseBundleMismatch(outputPath: string, baseBundlePath: string): void { + const record = readBinaryPatchBaseRecord(outputPath); + if (!record) { + return; + } + + const baseBundleHash = hashBundleFile(baseBundlePath); + if (record.baseBundleHash !== baseBundleHash) { + console.warn( + `warn: The bundle was compiled against base bundle ${record.baseBundleHash}, but this release patches against ${baseBundleHash}. ` + + 'The patch is valid but larger than an aligned one.', + ); + } +} + +async function uploadArtifact( + bundleUploader: CliConfigInterface['bundleUploader'], + filePath: string, + platform: 'ios' | 'android', + identifier: string | undefined, + artifactName: string, +): Promise { + try { + const { downloadUrl } = await bundleUploader(filePath, platform, identifier); + return downloadUrl + } catch (error) { + console.error(`Failed to upload the ${artifactName} file. Exiting the program.\n`, error) + process.exit(1) + } +} diff --git a/cli/functions/makeBinaryPatchBundle.test.ts b/cli/functions/makeBinaryPatchBundle.test.ts new file mode 100644 index 000000000..5fc52b976 --- /dev/null +++ b/cli/functions/makeBinaryPatchBundle.test.ts @@ -0,0 +1,402 @@ +import { spawnSync } from "child_process"; +import crypto from "crypto"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from "@jest/globals"; +import { + BINARY_PATCH_ARCHIVE_SUFFIX, + BINARY_PATCH_BASE_RECORD_NAME, + BINARY_PATCH_MANIFEST_NAME, + extractCodePushBundleContents, + formatBinaryPatchSummary, + makeBinaryPatchBundle, + readBinaryPatchBaseRecord, + resolveBaseBundlePath, + writeBinaryPatchBaseRecord, +} from "./makeBinaryPatchBundle.js"; +import { makeCodePushBundle } from "./makeCodePushBundle.js"; +import { applyPatch, BINARY_PATCH_ALGORITHM, BINARY_PATCH_FORMAT_VERSION, resolveBinaryPatchTool } from "../utils/binaryPatch.js"; +import { generatePackageHashFromDirectory } from "../utils/hash-utils.js"; +import { unzip } from "../utils/unzip.js"; + +/** + * Exercises the patch artifact against real archives, real hdiffz output and real + * hashes: the artifact only has value if a client can rebuild the exact contents of + * the full archive from it, and only real bytes prove that. + */ + +/** Cloning and building hdiffz/hpatchz from source takes a while on a cold machine. */ +const BUILD_TOOLS_TIMEOUT_MS = 10 * 60 * 1000; + +/** The directory a CodePush archive keeps its contents in, for legacy reasons. */ +const CONTENTS_DIR_NAME = 'CodePush'; + +function findRepoRoot(): string { + let dir = process.cwd(); + for (;;) { + if (fs.existsSync(path.join(dir, "scripts", "binary-patch", "build-hdiffpatch.sh"))) { + return dir; + } + const parent = path.dirname(dir); + if (parent === dir) { + throw new Error(`cannot locate the repository root from ${process.cwd()}`); + } + dir = parent; + } +} + +const repoRoot = findRepoRoot(); +const fixtureDir = path.join(repoRoot, "cli", "fixtures", "binary-patch"); +const baseFixture = path.join(fixtureDir, "base.bundle"); +const targetFixture = path.join(fixtureDir, "target.bundle"); + +let workDir: string; + +/** + * Builds hdiffz/hpatchz when they are missing so a clean checkout - and CI - can run + * this suite without a manual setup step, the same way the codec suite does. + */ +function ensureBinaryPatchTools(): void { + try { + resolveBinaryPatchTool("hdiffz"); + resolveBinaryPatchTool("hpatchz"); + return; + } catch { + // Not built yet; build them once below. + } + const script = path.join(repoRoot, "scripts", "binary-patch", "build-hdiffpatch.sh"); + const result = spawnSync(script, { encoding: "utf8", timeout: BUILD_TOOLS_TIMEOUT_MS }); + if (result.status !== 0) { + throw new Error(`${script} failed:\n${result.stdout ?? ""}${result.stderr ?? ""}`); + } +} + +function sha256(filePath: string): string { + return crypto.createHash("sha256").update(fs.readFileSync(filePath)).digest("hex"); +} + +/** A unique directory per test so a failed run never leaks state into the next one. */ +function makeCaseDir(name: string): string { + const caseDir = fs.mkdtempSync(path.join(workDir, `${name}-`)); + return caseDir; +} + +/** + * Writes an update contents directory shaped like the one the bundler produces: the + * JS bundle next to the assets that ship with it. + */ +function writeUpdateContents(caseDir: string, files: Record): string { + const contentsPath = path.join(caseDir, CONTENTS_DIR_NAME); + for (const [relativePath, content] of Object.entries(files)) { + const filePath = path.join(contentsPath, relativePath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + } + return contentsPath; +} + +function defaultContents(caseDir: string, bundleName = 'main.jsbundle'): string { + return writeUpdateContents(caseDir, { + [bundleName]: fs.readFileSync(targetFixture), + 'assets/logo.png': Buffer.from('logo-bytes'), + }); +} + +async function unzipTo(archivePath: string, destination: string): Promise { + fs.mkdirSync(destination, { recursive: true }); + await unzip(archivePath, destination); + return path.join(destination, CONTENTS_DIR_NAME); +} + +beforeAll(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), "codepush-patch-bundle-")); + ensureBinaryPatchTools(); +}, BUILD_TOOLS_TIMEOUT_MS); + +afterAll(() => { + fs.rmSync(workDir, { recursive: true, force: true }); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe("resolveBaseBundlePath", () => { + it("returns an absolute path for an existing file", () => { + expect(resolveBaseBundlePath(path.relative(process.cwd(), baseFixture))).toBe(baseFixture); + }); + + it("fails with a message naming the option when the file does not exist", () => { + const missing = path.join(makeCaseDir("missing-base"), "no-such.bundle"); + + expect(() => resolveBaseBundlePath(missing)).toThrow(/--binary-bundle-path/); + expect(() => resolveBaseBundlePath(missing)).toThrow(/does not exist/); + }); + + it("fails when the path is a directory", () => { + expect(() => resolveBaseBundlePath(fixtureDir)).toThrow(/is not a file/); + }); +}); + +describe("makeBinaryPatchBundle", () => { + it("writes a patch archive next to the full archive, named after the package hash", async () => { + const caseDir = makeCaseDir("names"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + const { bundleFileName: packageHash } = await makeCodePushBundle(contentsPath, bundleDirectory); + + const { patchBundleFilePath } = await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash, + }); + + expect(path.basename(patchBundleFilePath)).toBe(`${packageHash}${BINARY_PATCH_ARCHIVE_SUFFIX}`); + expect(fs.existsSync(patchBundleFilePath)).toBe(true); + // The full archive stays exactly as it was uploaded before patches existed. + expect(fs.readdirSync(bundleDirectory).sort()).toEqual( + [packageHash, `${packageHash}${BINARY_PATCH_ARCHIVE_SUFFIX}`].sort(), + ); + }); + + it("pins the manifest to the codec contract and to the target bundle it describes", async () => { + const caseDir = makeCaseDir("manifest"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + + const { patchBundleFilePath, manifest } = await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash: 'package-hash', + }); + + expect(manifest).toEqual({ + formatVersion: BINARY_PATCH_FORMAT_VERSION, + algorithm: BINARY_PATCH_ALGORITHM, + bundlePath: 'main.jsbundle', + patchFile: 'main.jsbundle.patch', + baseBundleHash: sha256(baseFixture), + targetBundleHash: sha256(targetFixture), + targetBundleSize: fs.statSync(targetFixture).size, + }); + + const extracted = await unzipTo(patchBundleFilePath, path.join(caseDir, "extracted")); + expect(JSON.parse(fs.readFileSync(path.join(extracted, BINARY_PATCH_MANIFEST_NAME), 'utf8'))).toEqual(manifest); + }); + + it("replaces the target bundle with its patch and keeps every other file", async () => { + const caseDir = makeCaseDir("layout"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + + const { patchBundleFilePath } = await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash: 'package-hash', + }); + + const extracted = await unzipTo(patchBundleFilePath, path.join(caseDir, "extracted")); + expect(fs.existsSync(path.join(extracted, 'main.jsbundle'))).toBe(false); + expect(fs.existsSync(path.join(extracted, 'main.jsbundle.patch'))).toBe(true); + expect(fs.readFileSync(path.join(extracted, 'assets/logo.png'), 'utf8')).toBe('logo-bytes'); + }); + + it("restores the exact package hash of the full archive from the patch archive", async () => { + const caseDir = makeCaseDir("round-trip"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + const { bundleFileName: packageHash } = await makeCodePushBundle(contentsPath, bundleDirectory); + + const { patchBundleFilePath, manifest } = await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash, + }); + + // Replay what a client does: apply the patch onto the bundle from the binary, + // then drop the patch artifacts that are not part of the update contents. + const extractRoot = path.join(caseDir, "extracted"); + const extracted = await unzipTo(patchBundleFilePath, extractRoot); + applyPatch(baseFixture, path.join(extracted, manifest.patchFile), path.join(extracted, manifest.bundlePath)); + fs.rmSync(path.join(extracted, manifest.patchFile)); + fs.rmSync(path.join(extracted, BINARY_PATCH_MANIFEST_NAME)); + + expect(sha256(path.join(extracted, manifest.bundlePath))).toBe(sha256(targetFixture)); + expect(await generatePackageHashFromDirectory(extracted, extractRoot)).toBe(packageHash); + }); + + it("patches only the bundle named by the caller, leaving another platform's bundle untouched", async () => { + const caseDir = makeCaseDir("android"); + const contentsPath = writeUpdateContents(caseDir, { + 'index.android.bundle': fs.readFileSync(targetFixture), + 'main.jsbundle': Buffer.from('ios-bundle-bytes'), + }); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + + const { patchBundleFilePath, manifest } = await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'index.android.bundle', + bundleDirectory, + packageHash: 'package-hash', + }); + + expect(manifest.bundlePath).toBe('index.android.bundle'); + expect(manifest.patchFile).toBe('index.android.bundle.patch'); + + const extracted = await unzipTo(patchBundleFilePath, path.join(caseDir, "extracted")); + expect(fs.existsSync(path.join(extracted, 'index.android.bundle'))).toBe(false); + expect(fs.readFileSync(path.join(extracted, 'main.jsbundle'), 'utf8')).toBe('ios-bundle-bytes'); + }); + + it("warns when the base and the target bundle are the same bytes", async () => { + const caseDir = makeCaseDir("identical"); + const contentsPath = writeUpdateContents(caseDir, { 'main.jsbundle': fs.readFileSync(baseFixture) }); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory: path.join(caseDir, "bundleOutput"), + packageHash: 'package-hash', + }); + + expect(warn.mock.calls.join('\n')).toMatch(/identical/); + }); + + it("fails with an actionable error when the target bundle is not in the contents", async () => { + const caseDir = makeCaseDir("no-target"); + const contentsPath = writeUpdateContents(caseDir, { 'index.android.bundle': Buffer.from('android') }); + + await expect( + makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory: path.join(caseDir, "bundleOutput"), + packageHash: 'package-hash', + }), + ).rejects.toThrow(/main\.jsbundle.*--js-bundle-name/s); + }); + + it("leaves no working directory behind, whether it succeeds or fails", async () => { + const caseDir = makeCaseDir("cleanup"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + const entriesBefore = fs.readdirSync(caseDir).sort(); + + await makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: baseFixture, + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash: 'package-hash', + }); + + expect(fs.readdirSync(caseDir).sort()).toEqual([...entriesBefore, 'bundleOutput'].sort()); + + await expect( + makeBinaryPatchBundle({ + contentsPath, + baseBundlePath: path.join(caseDir, 'no-such-base.bundle'), + bundleRelativePath: 'main.jsbundle', + bundleDirectory, + packageHash: 'package-hash', + }), + ).rejects.toThrow(); + + expect(fs.readdirSync(caseDir).sort()).toEqual([...entriesBefore, 'bundleOutput'].sort()); + }); +}); + +describe("binary patch base record", () => { + it("is written outside the update contents, so it cannot change the package hash", async () => { + const caseDir = makeCaseDir("record"); + const contentsPath = defaultContents(caseDir); + const hashBefore = await generatePackageHashFromDirectory(contentsPath, caseDir); + + const recordPath = writeBinaryPatchBaseRecord(caseDir, sha256(baseFixture)); + + expect(path.relative(contentsPath, recordPath).startsWith('..')).toBe(true); + expect(path.basename(recordPath)).toBe(BINARY_PATCH_BASE_RECORD_NAME); + expect(await generatePackageHashFromDirectory(contentsPath, caseDir)).toBe(hashBefore); + expect(readBinaryPatchBaseRecord(caseDir)).toEqual({ baseBundleHash: sha256(baseFixture) }); + }); + + it("reads as absent when there is no record or the record is unreadable", () => { + const caseDir = makeCaseDir("record-missing"); + + expect(readBinaryPatchBaseRecord(caseDir)).toBeNull(); + + fs.writeFileSync(path.join(caseDir, BINARY_PATCH_BASE_RECORD_NAME), 'not json'); + expect(readBinaryPatchBaseRecord(caseDir)).toBeNull(); + }); +}); + +describe("extractCodePushBundleContents", () => { + it("unpacks the contents of an already built bundle file so a patch can be built from it", async () => { + const caseDir = makeCaseDir("extract"); + const contentsPath = defaultContents(caseDir); + const bundleDirectory = path.join(caseDir, "bundleOutput"); + const { bundleFileName } = await makeCodePushBundle(contentsPath, bundleDirectory); + + const { extractDir, contentsPath: extractedContents } = await extractCodePushBundleContents( + path.join(bundleDirectory, bundleFileName), + caseDir, + ); + + expect(await generatePackageHashFromDirectory(extractedContents, path.dirname(extractedContents))).toBe(bundleFileName); + expect(fs.existsSync(path.join(extractedContents, 'main.jsbundle'))).toBe(true); + + fs.rmSync(extractDir, { recursive: true, force: true }); + }); +}); + +describe("formatBinaryPatchSummary", () => { + it("reports both archive sizes and what the patch saves", () => { + const baseBundleHash = 'a'.repeat(64); + const targetBundleHash = 'b'.repeat(64); + + const summary = formatBinaryPatchSummary({ + platform: 'ios', + baseBundleHash, + targetBundleHash, + fullArchiveSize: 19_293_798, + patchArchiveSize: 3_250_586, + }); + + expect(summary).toBe( + [ + 'Binary patch summary (ios)', + `Base bundle SHA-256: ${baseBundleHash}`, + `Target bundle SHA-256: ${targetBundleHash}`, + 'Full archive: 18.4 MB', + 'Patch archive: 3.1 MB', + 'Saved: 15.3 MB (83.2%)', + ].join('\n'), + ); + }); + + it("reports a negative saving when the patch is larger than the full archive", () => { + const summary = formatBinaryPatchSummary({ + platform: 'android', + baseBundleHash: 'a'.repeat(64), + targetBundleHash: 'b'.repeat(64), + fullArchiveSize: 1_000, + patchArchiveSize: 1_500, + }); + + expect(summary).toContain('Binary patch summary (android)'); + expect(summary).toContain('Saved: -500 B (-50.0%)'); + }); +}); diff --git a/cli/functions/makeBinaryPatchBundle.ts b/cli/functions/makeBinaryPatchBundle.ts new file mode 100644 index 000000000..2ac32b997 --- /dev/null +++ b/cli/functions/makeBinaryPatchBundle.ts @@ -0,0 +1,316 @@ +/** + * Builds the binary patch artifact that ships alongside a full CodePush update. + * + * A patch archive is the full archive with the JS bundle swapped for a patch against + * the bundle that is already inside the app binary, plus a manifest describing how to + * rebuild it. Every other file (assets and so on) is copied over untouched, so a + * client that applies the patch and drops the two patch-only files holds byte-for-byte + * the same contents as the full archive - and therefore computes the same + * `packageHash`. That is what lets one release serve both artifacts. + * + * The archive is produced from an already assembled contents directory rather than + * from the bundler, so it is independent of Metro and Hermes: whoever prepared the + * contents (a fresh bundle run, or an existing bundle file unpacked for + * `--skip-bundle`) gets the same artifact. + */ + +import crypto from "crypto"; +import fs from "fs"; +import path from "path"; +import shell from "shelljs"; +import { + BINARY_PATCH_ALGORITHM, + BINARY_PATCH_FORMAT_VERSION, + generatePatch, +} from "../utils/binaryPatch.js"; +import { unzip } from "../utils/unzip.js"; +import { zip } from "../utils/zip.js"; + +/** Manifest file a client reads to decide whether it can apply the patch, and how. */ +export const BINARY_PATCH_MANIFEST_NAME = 'codepush-binary-patch.json'; + +/** Appended to the full archive name so the two artifacts of a release stay paired. */ +export const BINARY_PATCH_ARCHIVE_SUFFIX = '-patch.zip'; + +/** + * Record the `bundle` command leaves in the output root - outside the update contents, + * so it never reaches the archive - to say which base bundle the JS bundle was + * compiled against. A later `release` compares it with the base it was given. + */ +export const BINARY_PATCH_BASE_RECORD_NAME = 'binary-patch-base.json'; + +const TEMP_PATCH_CONTENTS_DIR_NAME = 'temp_contents_for_binary_patch'; +const TEMP_EXTRACTED_CONTENTS_DIR_NAME = 'temp_contents_from_bundle_file'; + +const HASH_ALGORITHM = 'sha256'; + +export type BinaryPatchManifest = { + formatVersion: number; + algorithm: string; + /** Target bundle path, relative to the update contents root. */ + bundlePath: string; + /** Patch file path, relative to the update contents root. */ + patchFile: string; + baseBundleHash: string; + targetBundleHash: string; + targetBundleSize: number; +}; + +export type BinaryPatchBundle = { + patchBundleFilePath: string; + manifest: BinaryPatchManifest; +}; + +export type BinaryPatchBaseRecord = { + baseBundleHash: string; +}; + +/** SHA-256 of a single file's bytes, which is what the manifest records. */ +export function hashBundleFile(filePath: string): string { + return crypto.createHash(HASH_ALGORITHM).update(fs.readFileSync(filePath)).digest('hex'); +} + +/** + * Resolves the `--binary-bundle-path` option to an absolute path, rejecting anything + * that is not an existing file. Called before the bundler runs so a typo surfaces + * before minutes of bundling rather than after. + */ +export function resolveBaseBundlePath(binaryBundlePath: string): string { + const resolved = path.resolve(binaryBundlePath); + + let stats: fs.Stats; + try { + stats = fs.statSync(resolved); + } catch { + throw new Error(`--binary-bundle-path "${binaryBundlePath}" does not exist.`); + } + if (!stats.isFile()) { + throw new Error(`--binary-bundle-path "${binaryBundlePath}" is not a file.`); + } + + return resolved; +} + +/** + * Same as `resolveBaseBundlePath`, but reports the problem the way the other CLI + * options do. Returns `undefined` when the option was not passed at all. + */ +export function resolveBinaryBundlePathOption(binaryBundlePath: string | undefined): string | undefined { + if (!binaryBundlePath) { + return undefined; + } + + try { + return resolveBaseBundlePath(binaryBundlePath); + } catch (error) { + console.error(error instanceof Error ? error.message : error); + process.exit(1); + } +} + +/** + * Creates the patch archive for an update that has already been assembled. + * + * @param contentsPath {string} Directory holding the update contents that went into the full archive + * @param baseBundlePath {string} JS bundle from the target binary, which the patch is computed against + * @param bundleRelativePath {string} Target bundle path inside `contentsPath` + * @param bundleDirectory {string} Directory the full archive was written to + * @param packageHash {string} Package hash of the full archive, which names both artifacts + */ +export async function makeBinaryPatchBundle({ + contentsPath, + baseBundlePath, + bundleRelativePath, + bundleDirectory, + packageHash, +}: { + contentsPath: string; + baseBundlePath: string; + bundleRelativePath: string; + bundleDirectory: string; + packageHash: string; +}): Promise { + const resolvedContentsPath = path.resolve(contentsPath); + const targetBundlePath = path.join(resolvedContentsPath, bundleRelativePath); + if (!isFile(targetBundlePath)) { + throw new Error( + `Target bundle "${bundleRelativePath}" was not found in the update contents ("${resolvedContentsPath}"). ` + + `Pass -j/--js-bundle-name if the bundle file has a different name.`, + ); + } + const resolvedBaseBundlePath = resolveBaseBundlePath(baseBundlePath); + + const baseBundleHash = hashBundleFile(resolvedBaseBundlePath); + const targetBundleHash = hashBundleFile(targetBundlePath); + if (baseBundleHash === targetBundleHash) { + console.warn( + 'warn: The base bundle and the target bundle are identical, so the update changes nothing. Releasing anyway.', + ); + } + + // Kept next to the update contents rather than in the system temp directory so a + // crash leaves the leftovers where the rest of the build output is cleaned up. + const tempRoot = path.join(path.dirname(resolvedContentsPath), TEMP_PATCH_CONTENTS_DIR_NAME); + // The archive root directory name is part of the contents layout, so the patch + // archive has to reuse the name the full archive used. + const patchContentsPath = path.join(tempRoot, path.basename(resolvedContentsPath)); + + fs.rmSync(tempRoot, { recursive: true, force: true }); + + try { + fs.cpSync(resolvedContentsPath, patchContentsPath, { recursive: true }); + fs.rmSync(path.join(patchContentsPath, bundleRelativePath)); + + const patchRelativePath = `${bundleRelativePath}.patch`; + generatePatch(resolvedBaseBundlePath, targetBundlePath, path.join(patchContentsPath, patchRelativePath)); + + const manifest: BinaryPatchManifest = { + formatVersion: BINARY_PATCH_FORMAT_VERSION, + algorithm: BINARY_PATCH_ALGORITHM, + bundlePath: bundleRelativePath, + patchFile: patchRelativePath, + baseBundleHash, + targetBundleHash, + targetBundleSize: fs.statSync(targetBundlePath).size, + }; + fs.writeFileSync(path.join(patchContentsPath, BINARY_PATCH_MANIFEST_NAME), JSON.stringify(manifest, null, 2)); + + const patchArchiveZipPath = await zip(patchContentsPath); + const patchBundleFilePath = path.join(bundleDirectory, `${packageHash}${BINARY_PATCH_ARCHIVE_SUFFIX}`); + shell.mkdir('-p', bundleDirectory); + shell.mv(patchArchiveZipPath, patchBundleFilePath); + + return { patchBundleFilePath, manifest }; + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +} + +/** + * Unpacks an already built CodePush bundle file, so `release --skip-bundle` can patch + * exactly the contents that were packed instead of bundling them again. + * + * @return The extraction directory, which the caller owns and must remove, and the + * update contents root inside it. + */ +export async function extractCodePushBundleContents( + bundleFilePath: string, + parentDirectory: string, +): Promise<{ extractDir: string, contentsPath: string }> { + const extractDir = path.resolve(path.join(parentDirectory, TEMP_EXTRACTED_CONTENTS_DIR_NAME)); + + fs.rmSync(extractDir, { recursive: true, force: true }); + fs.mkdirSync(extractDir, { recursive: true }); + try { + await unzip(path.resolve(bundleFilePath), extractDir); + } catch (error) { + fs.rmSync(extractDir, { recursive: true, force: true }); + throw error; + } + + return { extractDir, contentsPath: resolveExtractedContentsPath(extractDir) }; +} + +/** + * A CodePush archive wraps its files in a single directory, so that directory - not + * the extraction directory - is the contents root. + */ +function resolveExtractedContentsPath(extractDir: string): string { + const entries = fs.readdirSync(extractDir, { withFileTypes: true }); + if (entries.length === 1 && entries[0].isDirectory()) { + return path.join(extractDir, entries[0].name); + } + return extractDir; +} + +/** Records which base bundle the JS bundle in `outputRootPath` was compiled against. */ +export function writeBinaryPatchBaseRecord(outputRootPath: string, baseBundleHash: string): string { + const recordPath = path.join(outputRootPath, BINARY_PATCH_BASE_RECORD_NAME); + const record: BinaryPatchBaseRecord = { baseBundleHash }; + + fs.mkdirSync(outputRootPath, { recursive: true }); + fs.writeFileSync(recordPath, JSON.stringify(record, null, 2)); + + return recordPath; +} + +/** + * Reads the record left by a previous `bundle` run. An unreadable record is treated as + * no record: it only feeds a warning, and must never be able to fail a release. + */ +export function readBinaryPatchBaseRecord(outputRootPath: string): BinaryPatchBaseRecord | null { + const recordPath = path.join(outputRootPath, BINARY_PATCH_BASE_RECORD_NAME); + + try { + const record = JSON.parse(fs.readFileSync(recordPath, 'utf8')) as Partial; + if (typeof record.baseBundleHash !== 'string') { + return null; + } + return { baseBundleHash: record.baseBundleHash }; + } catch { + return null; + } +} + +/** + * The operator-facing summary, printed before the artifacts are uploaded so the size + * of what is about to be published is visible while it can still be stopped. A patch + * larger than the full archive is reported as a negative saving and left in place; + * whether that is worth shipping is a judgement call. + */ +export function formatBinaryPatchSummary({ + platform, + baseBundleHash, + targetBundleHash, + fullArchiveSize, + patchArchiveSize, +}: { + platform: 'ios' | 'android'; + baseBundleHash: string; + targetBundleHash: string; + fullArchiveSize: number; + patchArchiveSize: number; +}): string { + const savedBytes = fullArchiveSize - patchArchiveSize; + const savedRatio = fullArchiveSize > 0 ? savedBytes / fullArchiveSize : 0; + + const sizes = [fullArchiveSize, patchArchiveSize, savedBytes].map(formatBytes); + const sizeWidth = Math.max(...sizes.map((size) => size.length)); + const [fullSize, patchSize, savedSize] = sizes.map((size) => size.padStart(sizeWidth)); + + const label = (text: string) => text.padEnd(23); + + return [ + `Binary patch summary (${platform})`, + `${label('Base bundle SHA-256:')}${baseBundleHash}`, + `${label('Target bundle SHA-256:')}${targetBundleHash}`, + `${label('Full archive:')}${fullSize}`, + `${label('Patch archive:')}${patchSize}`, + `${label('Saved:')}${savedSize} (${(savedRatio * 100).toFixed(1)}%)`, + ].join('\n'); +} + +function formatBytes(bytes: number): string { + const units = ['KB', 'MB', 'GB', 'TB']; + + if (Math.abs(bytes) < 1024) { + return `${bytes} B`; + } + + let value = bytes / 1024; + let unitIndex = 0; + while (Math.abs(value) >= 1024 && unitIndex < units.length - 1) { + value /= 1024; + unitIndex++; + } + + return `${value.toFixed(1)} ${units[unitIndex]}`; +} + +function isFile(filePath: string): boolean { + try { + return fs.statSync(filePath).isFile(); + } catch { + return false; + } +} diff --git a/cli/functions/runHermesEmitBinaryCommand.test.ts b/cli/functions/runHermesEmitBinaryCommand.test.ts new file mode 100644 index 000000000..d3d60ba19 --- /dev/null +++ b/cli/functions/runHermesEmitBinaryCommand.test.ts @@ -0,0 +1,124 @@ +import fs from "fs"; +import os from "os"; +import path from "path"; +import { afterAll, afterEach, beforeAll, describe, expect, it, jest } from "@jest/globals"; +import { + helpOutputSupportsBaseBytecode, + resolveBaseBytecodeHermesFlags, +} from "./runHermesEmitBinaryCommand.js"; + +/** + * The base bytecode flag only exists in recent Hermes compilers, and the compiler that + * matters is the one inside the app being released - so the flag is decided by asking + * that binary, not by a version check. + */ + +/** Excerpt of `hermesc --help` from a compiler that has the flag. */ +const HELP_WITH_BASE_BYTECODE = `OVERVIEW: Hermes driver + +USAGE: hermesc [options] + +OPTIONS: + -Wno-direct-eval - Disable Warning when attempting a direct (local) eval + -base-bytecode= - input base bytecode for delta optimizing mode + -bytecode-output-manifest= - Name of the manifest file generated when compiling multiple segments to bytecode + -commonjs - Use CommonJS modules +`; + +/** The same excerpt from a compiler that predates the flag. */ +const HELP_WITHOUT_BASE_BYTECODE = `OVERVIEW: Hermes driver + +USAGE: hermesc [options] + +OPTIONS: + -Wno-direct-eval - Disable Warning when attempting a direct (local) eval + -bytecode-output-manifest= - Name of the manifest file generated when compiling multiple segments to bytecode + -commonjs - Use CommonJS modules +`; + +function findRepoRoot(): string { + let dir = process.cwd(); + for (;;) { + if (fs.existsSync(path.join(dir, "node_modules", "react-native"))) { + return dir; + } + const parent = path.dirname(dir); + if (parent === dir) { + throw new Error(`cannot locate the repository root from ${process.cwd()}`); + } + dir = parent; + } +} + +function hermesOSBinDirName(): string { + switch (process.platform) { + case 'win32': + return 'win64-bin'; + case 'darwin': + return 'osx-bin'; + default: + return 'linux64-bin'; + } +} + +const repoRoot = findRepoRoot(); +const baseBundlePath = path.join(repoRoot, "cli", "fixtures", "binary-patch", "base.bundle"); + +let workDir: string; + +/** A project whose Hermes compiler reports a help text without the flag. */ +function writeProjectWithFakeHermesc(helpOutput: string): string { + const projectRoot = fs.mkdtempSync(path.join(workDir, "project-")); + const hermescDir = path.join(projectRoot, "node_modules", "react-native", "sdks", "hermesc", hermesOSBinDirName()); + + fs.mkdirSync(hermescDir, { recursive: true }); + const hermescPath = path.join(hermescDir, "hermesc"); + fs.writeFileSync(hermescPath, `#!/bin/sh\ncat <<'HELP'\n${helpOutput}HELP\n`); + fs.chmodSync(hermescPath, 0o755); + + return projectRoot; +} + +beforeAll(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), "codepush-hermes-flags-")); +}); + +afterAll(() => { + fs.rmSync(workDir, { recursive: true, force: true }); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe("helpOutputSupportsBaseBytecode", () => { + it("detects the flag in the help output of a compiler that has it", () => { + expect(helpOutputSupportsBaseBytecode(HELP_WITH_BASE_BYTECODE)).toBe(true); + }); + + it("reports no support when the help output does not list the flag", () => { + expect(helpOutputSupportsBaseBytecode(HELP_WITHOUT_BASE_BYTECODE)).toBe(false); + }); +}); + +describe("resolveBaseBytecodeHermesFlags", () => { + it("aligns the compilation with the base bundle when the compiler supports it", () => { + expect(resolveBaseBytecodeHermesFlags(baseBundlePath, repoRoot)).toEqual(['-base-bytecode', baseBundlePath]); + }); + + const itUnlessWindows = process.platform === 'win32' ? it.skip : it; + + itUnlessWindows("warns and compiles without the flag when the compiler does not support it", () => { + const projectRoot = writeProjectWithFakeHermesc(HELP_WITHOUT_BASE_BYTECODE); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + expect(resolveBaseBytecodeHermesFlags(baseBundlePath, projectRoot)).toEqual([]); + expect(warn.mock.calls.join('\n')).toMatch(/-base-bytecode/); + }); + + itUnlessWindows("uses the flag when the app's own compiler advertises it", () => { + const projectRoot = writeProjectWithFakeHermesc(HELP_WITH_BASE_BYTECODE); + + expect(resolveBaseBytecodeHermesFlags(baseBundlePath, projectRoot)).toEqual(['-base-bytecode', baseBundlePath]); + }); +}); diff --git a/cli/functions/runHermesEmitBinaryCommand.ts b/cli/functions/runHermesEmitBinaryCommand.ts index 05eed6f5b..072f56bee 100644 --- a/cli/functions/runHermesEmitBinaryCommand.ts +++ b/cli/functions/runHermesEmitBinaryCommand.ts @@ -8,6 +8,12 @@ import path from "path"; import { createRequire } from "node:module"; import shell from "shelljs"; +/** + * Tells Hermes to lay the compiled bytecode out like an existing bundle's, which keeps + * the binary patch between the two small. Older compilers do not have the flag. + */ +const BASE_BYTECODE_FLAG = '-base-bytecode'; + /** * Run Hermes compile CLI command * @@ -43,7 +49,16 @@ export async function runHermesEmitBinaryCommand( const hermesCommand = getHermesCommand(projectRoot); const disableAllWarningsArg = '-w'; - shell.exec(`${hermesCommand} ${hermesArgs.join(' ')} ${disableAllWarningsArg}`); + const compileResult = shell.exec(`${hermesCommand} ${hermesArgs.join(' ')} ${disableAllWarningsArg}`); + if (compileResult.code !== 0) { + // Reported here rather than left to the missing .hbc file, so a rejected + // flag - a base bundle that is not Hermes bytecode, for instance - fails + // the release instead of silently producing an unaligned bundle. + const extraFlagsHint = extraHermesFlags.length > 0 + ? ` Additional options: ${extraHermesFlags.join(' ')}` + : ''; + throw new Error(`"hermesc" command failed (exitCode=${compileResult.code}).${extraFlagsHint}`); + } // Copy HBC bundle to overwrite JS bundle const source = path.join(outputPath, bundleName + '.hbc'); @@ -109,6 +124,47 @@ export async function runHermesEmitBinaryCommand( }); } +/** + * Builds the flags that align a compilation with the bundle already inside the app + * binary, so the binary patch between them stays small. + * + * When the app's Hermes compiler predates the flag the release still goes ahead + * without it: the patch is then computed against unaligned bytecode and is larger, but + * it is still a valid patch. A compiler that does accept the flag and fails is a + * different matter and fails the release, because the base input is then wrong. + * + * @param baseBundlePath {string} JS bundle from the target binary to align against + * @param projectRoot {string} Root directory of the target app project, used to locate its Hermes compiler + * @return {string[]} Flags to pass to `runHermesEmitBinaryCommand` as `extraHermesFlags` + */ +export function resolveBaseBytecodeHermesFlags(baseBundlePath: string, projectRoot: string = process.cwd()): string[] { + if (!hermesSupportsBaseBytecode(projectRoot)) { + console.warn( + `warn: The Hermes compiler of this app does not support "${BASE_BYTECODE_FLAG}". ` + + 'Compiling without it, which makes the binary patch larger.', + ); + return []; + } + + return [BASE_BYTECODE_FLAG, baseBundlePath]; +} + +/** Whether a `hermesc --help` output advertises the base bytecode flag. */ +export function helpOutputSupportsBaseBytecode(helpOutput: string): boolean { + return helpOutput.includes(BASE_BYTECODE_FLAG); +} + +function hermesSupportsBaseBytecode(projectRoot: string): boolean { + const hermesCommand = getHermesCommand(projectRoot); + const result = childProcess.spawnSync(hermesCommand, ['--help'], { encoding: 'utf8' }); + + if (result.error) { + throw new Error(`failed to run "${hermesCommand} --help": ${result.error.message}`); + } + + return helpOutputSupportsBaseBytecode(`${result.stdout ?? ''}${result.stderr ?? ''}`); +} + function getHermesCommand(projectRoot: string): string { const fileExists = (file: string): boolean => { try { From 2ed5bb26ef08d3f44c3fb853f08613a4d58907d4 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Wed, 5 Aug 2026 23:18:15 +0900 Subject: [PATCH 3/5] fix(cli): pass --js-bundle-name through to release The release action read `options.bundleName`, but commander stores the `-j, --js-bundle-name` flag as `options.jsBundleName`, so a custom JS bundle name never reached `release()` and the platform default was always used. Optionality is now expressed in the types instead of asserted away, so "not given" cannot pass for a name again. --- cli/commands/bundleCommand/bundleCodePush.ts | 2 +- cli/commands/bundleCommand/index.ts | 3 +- cli/commands/releaseCommand/index.test.ts | 83 ++++++++++++++++++++ cli/commands/releaseCommand/index.ts | 5 +- cli/commands/releaseCommand/release.test.ts | 40 +++++++++- cli/commands/releaseCommand/release.ts | 2 +- 6 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 cli/commands/releaseCommand/index.test.ts diff --git a/cli/commands/bundleCommand/bundleCodePush.ts b/cli/commands/bundleCommand/bundleCodePush.ts index 3b80c53ee..b73292675 100644 --- a/cli/commands/bundleCommand/bundleCodePush.ts +++ b/cli/commands/bundleCommand/bundleCodePush.ts @@ -36,7 +36,7 @@ export async function bundleCodePush( platform: 'ios' | 'android' = 'ios', outputRootPath: string = ROOT_OUTPUT_DIR, entryFile: string = ENTRY_FILE, - jsBundleName: string, // JS bundle file name (not CodePush bundle file) + jsBundleName: string | undefined, // JS bundle file name (not CodePush bundle file) bundleDirectory: string, // CodePush bundle output directory outputMetroDir?: string, baseBundlePath?: string, diff --git a/cli/commands/bundleCommand/index.ts b/cli/commands/bundleCommand/index.ts index 33cc9ed7c..9c33178d1 100644 --- a/cli/commands/bundleCommand/index.ts +++ b/cli/commands/bundleCommand/index.ts @@ -8,7 +8,8 @@ type Options = { platform: 'ios' | 'android'; outputPath: string; entryFile: string; - bundleName: string; + // Commander derives this from the "-b, --bundle-name" flag. + bundleName?: string; outputBundleDir: string; outputMetroDir?: string; binaryBundlePath?: string; diff --git a/cli/commands/releaseCommand/index.test.ts b/cli/commands/releaseCommand/index.test.ts new file mode 100644 index 000000000..a268df9b1 --- /dev/null +++ b/cli/commands/releaseCommand/index.test.ts @@ -0,0 +1,83 @@ +import path from "path"; +import { afterEach, beforeEach, describe, expect, it, jest } from "@jest/globals"; + +/** + * Checks the command definition against the arguments it forwards, which is where an + * option name can silently drift: commander derives the name it stores from the flag, + * so reading a differently named property yields `undefined` for every release rather + * than an error, and every downstream default looks as if it had been chosen. + */ + +jest.mock("./release.js"); +jest.mock("../../utils/fsUtils.js", () => ({ + findAndReadConfigFile: () => ({ + bundleUploader: async () => ({ downloadUrl: 'https://cdn.example.com/bundle' }), + getReleaseHistory: async () => ({}), + setReleaseHistory: async () => undefined, + }), +})); + +/** + * `release()` takes positional arguments; these are the positions this suite asserts on. + */ +const ARG_INDEX = { + platform: 6, + outputPath: 8, + entryFile: 9, + jsBundleName: 10, + skipBundle: 14, + bundleDirectory: 16, + baseBundlePath: 19, +} as const; + +async function runReleaseCommand(args: string[]): Promise { + const { release } = await import("./release.js"); + const { program } = await import("commander"); + await import("./index.js"); + + await program.parseAsync(['release', ...args], { from: 'user' }); + + const releaseMock = jest.mocked(release); + expect(releaseMock).toHaveBeenCalledTimes(1); + return releaseMock.mock.calls[0]; +} + +beforeEach(() => { + jest.resetModules(); + jest.spyOn(console, 'log').mockImplementation(() => {}); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +/** Relative to the CLI workspace root, which is where the test runner starts. */ +const BASE_BUNDLE_FIXTURE = 'fixtures/binary-patch/base.bundle'; + +describe("release command options", () => { + it("passes the JS bundle name from -j through to the release", async () => { + const args = await runReleaseCommand([ + '-b', '1.0.0', + '-v', '1.0.1', + '-p', 'android', + '-j', 'custom.jsbundle', + '--skip-bundle', 'true', + '--binary-bundle-path', BASE_BUNDLE_FIXTURE, + ]); + + expect(args[ARG_INDEX.jsBundleName]).toBe('custom.jsbundle'); + expect(args[ARG_INDEX.platform]).toBe('android'); + expect(args[ARG_INDEX.skipBundle]).toBe(true); + expect(args[ARG_INDEX.baseBundlePath]).toBe(path.resolve(BASE_BUNDLE_FIXTURE)); + }); + + it("leaves the JS bundle name unset when -j is not given, so the platform default applies", async () => { + const args = await runReleaseCommand(['-b', '1.0.0', '-v', '1.0.1']); + + expect(args[ARG_INDEX.jsBundleName]).toBeUndefined(); + expect(args[ARG_INDEX.baseBundlePath]).toBeUndefined(); + expect(args[ARG_INDEX.outputPath]).toBe('build'); + expect(args[ARG_INDEX.entryFile]).toBe('index.ts'); + expect(args[ARG_INDEX.bundleDirectory]).toBe('build/bundleOutput'); + }); +}); diff --git a/cli/commands/releaseCommand/index.ts b/cli/commands/releaseCommand/index.ts index 8714f9b0a..cb0ea2e4c 100644 --- a/cli/commands/releaseCommand/index.ts +++ b/cli/commands/releaseCommand/index.ts @@ -13,7 +13,8 @@ type Options = { config: string; outputPath: string; entryFile: string; - bundleName: string; + // Commander derives this from the "-j, --js-bundle-name" flag. + jsBundleName?: string; mandatory: boolean; enable: boolean; rollout?: number; @@ -72,7 +73,7 @@ program.command('release') options.identifier, options.outputPath, options.entryFile, - options.bundleName, + options.jsBundleName, options.mandatory, options.enable, options.rollout, diff --git a/cli/commands/releaseCommand/release.test.ts b/cli/commands/releaseCommand/release.test.ts index c07d54228..f2bf7d0ae 100644 --- a/cli/commands/releaseCommand/release.test.ts +++ b/cli/commands/releaseCommand/release.test.ts @@ -139,7 +139,8 @@ async function runRelease(staged: StagedBundle, overrides: ReleaseOverrides = {} undefined, staged.outputPath, 'index.ts', - overrides.jsBundleName ?? '', + // Left unset unless the case is about -j, the way commander leaves it. + overrides.jsBundleName, false, true, undefined, @@ -242,6 +243,43 @@ describe("release --skip-bundle --binary-bundle-path", () => { expect(logs.some((line) => line.startsWith('Binary patch summary (android)'))).toBe(true); }); + it("patches a bundle that was built with a custom JS bundle name", async () => { + const staged = await stageBundleOutput("custom-name", { + 'custom.jsbundle': fs.readFileSync(targetFixture), + }); + + const { uploads } = await runRelease(staged, { + binaryBundlePath: baseFixture, + jsBundleName: 'custom.jsbundle', + }); + + expect(uploads.map(({ filePath }) => path.basename(filePath))).toEqual([ + staged.bundleFileName, + `${staged.bundleFileName}${BINARY_PATCH_ARCHIVE_SUFFIX}`, + ]); + + const extractRoot = path.join(staged.outputPath, 'extracted'); + fs.mkdirSync(extractRoot, { recursive: true }); + await unzip(path.join(staged.bundleDirectory, `${staged.bundleFileName}${BINARY_PATCH_ARCHIVE_SUFFIX}`), extractRoot); + const manifest = JSON.parse( + fs.readFileSync(path.join(extractRoot, CONTENTS_DIR_NAME, BINARY_PATCH_MANIFEST_NAME), 'utf8'), + ) as { bundlePath: string, patchFile: string }; + + expect(manifest.bundlePath).toBe('custom.jsbundle'); + expect(manifest.patchFile).toBe('custom.jsbundle.patch'); + }); + + it("fails without -j when the released bundle uses a custom JS bundle name", async () => { + const staged = await stageBundleOutput("custom-name-without-option", { + 'custom.jsbundle': fs.readFileSync(targetFixture), + }); + + // The error names -j because passing it is what makes this release work. + await expect(runRelease(staged, { binaryBundlePath: baseFixture })).rejects.toThrow( + /main\.jsbundle.*--js-bundle-name/s, + ); + }); + it("fails with an actionable error when the released bundle holds no matching JS bundle", async () => { const staged = await stageBundleOutput("no-target", { 'index.android.bundle': fs.readFileSync(targetFixture), diff --git a/cli/commands/releaseCommand/release.ts b/cli/commands/releaseCommand/release.ts index f1a602c66..b1dd68c82 100644 --- a/cli/commands/releaseCommand/release.ts +++ b/cli/commands/releaseCommand/release.ts @@ -26,7 +26,7 @@ export async function release( identifier: string | undefined, outputPath: string, entryFile: string, - jsBundleName: string, + jsBundleName: string | undefined, mandatory: boolean, enable: boolean, rollout: number | undefined, From bb6c517f6400dba8b75fbee71af2eb10b50fe1c9 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Thu, 6 Aug 2026 13:52:03 +0900 Subject: [PATCH 4/5] fix(cli): provision hdiffz tools once in jest global setup The suites that generate real patches each built the tools in their own `beforeAll`, so a suite without that hook - the release flow suite - failed on a machine with no `.hdiffpatch-tools` yet, and two workers could run the same build at the same time. A jest global setup builds them once before any worker starts, which also removes the duplicated helper from the two suites that had one. --- cli/functions/makeBinaryPatchBundle.test.ts | 29 ++------------ cli/jest.config.cjs | 3 ++ cli/jest.globalSetup.ts | 43 +++++++++++++++++++++ cli/utils/binaryPatch.test.ts | 26 ++----------- 4 files changed, 52 insertions(+), 49 deletions(-) create mode 100644 cli/jest.globalSetup.ts diff --git a/cli/functions/makeBinaryPatchBundle.test.ts b/cli/functions/makeBinaryPatchBundle.test.ts index 5fc52b976..d5b6894fd 100644 --- a/cli/functions/makeBinaryPatchBundle.test.ts +++ b/cli/functions/makeBinaryPatchBundle.test.ts @@ -1,4 +1,3 @@ -import { spawnSync } from "child_process"; import crypto from "crypto"; import fs from "fs"; import os from "os"; @@ -16,7 +15,7 @@ import { writeBinaryPatchBaseRecord, } from "./makeBinaryPatchBundle.js"; import { makeCodePushBundle } from "./makeCodePushBundle.js"; -import { applyPatch, BINARY_PATCH_ALGORITHM, BINARY_PATCH_FORMAT_VERSION, resolveBinaryPatchTool } from "../utils/binaryPatch.js"; +import { applyPatch, BINARY_PATCH_ALGORITHM, BINARY_PATCH_FORMAT_VERSION } from "../utils/binaryPatch.js"; import { generatePackageHashFromDirectory } from "../utils/hash-utils.js"; import { unzip } from "../utils/unzip.js"; @@ -26,9 +25,6 @@ import { unzip } from "../utils/unzip.js"; * the full archive from it, and only real bytes prove that. */ -/** Cloning and building hdiffz/hpatchz from source takes a while on a cold machine. */ -const BUILD_TOOLS_TIMEOUT_MS = 10 * 60 * 1000; - /** The directory a CodePush archive keeps its contents in, for legacy reasons. */ const CONTENTS_DIR_NAME = 'CodePush'; @@ -53,25 +49,6 @@ const targetFixture = path.join(fixtureDir, "target.bundle"); let workDir: string; -/** - * Builds hdiffz/hpatchz when they are missing so a clean checkout - and CI - can run - * this suite without a manual setup step, the same way the codec suite does. - */ -function ensureBinaryPatchTools(): void { - try { - resolveBinaryPatchTool("hdiffz"); - resolveBinaryPatchTool("hpatchz"); - return; - } catch { - // Not built yet; build them once below. - } - const script = path.join(repoRoot, "scripts", "binary-patch", "build-hdiffpatch.sh"); - const result = spawnSync(script, { encoding: "utf8", timeout: BUILD_TOOLS_TIMEOUT_MS }); - if (result.status !== 0) { - throw new Error(`${script} failed:\n${result.stdout ?? ""}${result.stderr ?? ""}`); - } -} - function sha256(filePath: string): string { return crypto.createHash("sha256").update(fs.readFileSync(filePath)).digest("hex"); } @@ -110,9 +87,9 @@ async function unzipTo(archivePath: string, destination: string): Promise { + // hdiffz/hpatchz are provisioned for the whole run by the jest global setup. workDir = fs.mkdtempSync(path.join(os.tmpdir(), "codepush-patch-bundle-")); - ensureBinaryPatchTools(); -}, BUILD_TOOLS_TIMEOUT_MS); +}); afterAll(() => { fs.rmSync(workDir, { recursive: true, force: true }); diff --git a/cli/jest.config.cjs b/cli/jest.config.cjs index c34069e55..019a78427 100644 --- a/cli/jest.config.cjs +++ b/cli/jest.config.cjs @@ -3,6 +3,9 @@ module.exports = { rootDir: __dirname, testEnvironment: 'node', resolver: '/jest.resolver.cjs', + // Provisions hdiffz/hpatchz for the suites that generate real patches, once for the + // whole run instead of once per suite. + globalSetup: '/jest.globalSetup.ts', transform: { '^.+\\.(t|j)s$': ['babel-jest', { configFile: '../babel.config.js' }], }, diff --git a/cli/jest.globalSetup.ts b/cli/jest.globalSetup.ts new file mode 100644 index 000000000..90978f710 --- /dev/null +++ b/cli/jest.globalSetup.ts @@ -0,0 +1,43 @@ +import { spawnSync } from "child_process"; +import fs from "fs"; +import path from "path"; + +/** + * Builds hdiffz/hpatchz once, before any test worker starts. + * + * Several suites generate and apply real patches, which is the only way to prove the + * committed patch format is what every applier understands. Provisioning the tools here + * rather than in each suite keeps a clean checkout - and CI - free of a manual setup + * step, without letting two workers run the same build at the same time. + * + * The build script installs into `HDIFFPATCH_TOOLS_DIR` when it is set and into + * `/.hdiffpatch-tools` otherwise, which is where the CLI looks for the tools. It + * is a no-op when they are already there, so the check below only decides whether to + * warn that this run has a multi-minute build ahead of it. + * + * Note that this module is loaded outside the jest module resolver, so it cannot import + * from the CLI sources. + */ + +const REPO_ROOT = path.join(__dirname, '..'); +const BUILD_SCRIPT_PATH = path.join(REPO_ROOT, 'scripts', 'binary-patch', 'build-hdiffpatch.sh'); +const DEFAULT_TOOLS_DIR = path.join(REPO_ROOT, '.hdiffpatch-tools'); + +export default function ensureBinaryPatchTools(): void { + const toolsDir = process.env.HDIFFPATCH_TOOLS_DIR || DEFAULT_TOOLS_DIR; + const alreadyInstalled = ['hdiffz', 'hpatchz'].every((tool) => fs.existsSync(path.join(toolsDir, tool))); + + if (!alreadyInstalled) { + // Announced because a first build clones and compiles HDiffPatch, which takes + // minutes; a silent wait would look like a hang. + console.log(`Building hdiffz/hpatchz into ${toolsDir} (first run only)`); + } + + const result = spawnSync(BUILD_SCRIPT_PATH, { encoding: 'utf8' }); + if (result.error) { + throw new Error(`failed to run ${BUILD_SCRIPT_PATH}: ${result.error.message}`); + } + if (result.status !== 0) { + throw new Error(`${BUILD_SCRIPT_PATH} failed:\n${result.stdout ?? ''}${result.stderr ?? ''}`); + } +} diff --git a/cli/utils/binaryPatch.test.ts b/cli/utils/binaryPatch.test.ts index 6d68b1b43..e3a95e33c 100644 --- a/cli/utils/binaryPatch.test.ts +++ b/cli/utils/binaryPatch.test.ts @@ -15,8 +15,7 @@ import { applyPatch, generatePatch, resolveBinaryPatchTool } from "./binaryPatch * committed patch format really is what every applier understands. */ -/** Cloning and building hdiffz/hpatchz from source takes a while on a cold machine. */ -const BUILD_TOOLS_TIMEOUT_MS = 10 * 60 * 1000; +/** Building the applier from source takes a while on a cold machine. */ const BUILD_APPLIER_TIMEOUT_MS = 5 * 60 * 1000; function findRepoRoot(): string { @@ -61,25 +60,6 @@ function writeMutatedCopy(sourcePath: string, name: string, mutate: (bytes: Buff return destination; } -/** - * Builds hdiffz/hpatchz when they are missing so a clean checkout - and CI - can run - * this suite without a manual setup step. - */ -function ensureBinaryPatchTools(): void { - try { - resolveBinaryPatchTool("hdiffz"); - resolveBinaryPatchTool("hpatchz"); - return; - } catch { - // Not built yet; build them once below. - } - const script = path.join(repoRoot, "scripts", "binary-patch", "build-hdiffpatch.sh"); - const result = spawnSync(script, { encoding: "utf8", timeout: BUILD_TOOLS_TIMEOUT_MS }); - if (result.status !== 0) { - throw new Error(`${script} failed:\n${result.stdout ?? ""}${result.stderr ?? ""}`); - } -} - function cCompiler(): string { return process.env.CC || "cc"; } @@ -89,9 +69,9 @@ function hasCCompiler(): boolean { } beforeAll(() => { + // hdiffz/hpatchz are provisioned for the whole run by the jest global setup. workDir = fs.mkdtempSync(path.join(os.tmpdir(), "codepush-binary-patch-")); - ensureBinaryPatchTools(); -}, BUILD_TOOLS_TIMEOUT_MS); +}); afterAll(() => { fs.rmSync(workDir, { recursive: true, force: true }); From 7b2564ea917a3ec9ca0263497041f1b43eaaad5f Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Thu, 6 Aug 2026 18:26:36 +0900 Subject: [PATCH 5/5] feat(cli): add --on-oversized-patch policy A patch is only worth publishing when it is smaller than the archive it replaces, and the CLI runs unattended in CI, so what happens otherwise is decided up front instead of being left to whoever reads the summary. `skip`, the default, warns, records the skip in the summary and releases the full bundle alone. `fail` stops the release before either artifact is uploaded, so the release history stays untouched. Equal sizes count as oversized: a patch that saves nothing still costs a client an extra download and an apply step. --- cli/README.ko.md | 9 +++ cli/README.md | 10 +++ cli/commands/releaseCommand/index.test.ts | 39 +++++++++- cli/commands/releaseCommand/index.ts | 12 ++- cli/commands/releaseCommand/release.test.ts | 84 +++++++++++++++++++++ cli/commands/releaseCommand/release.ts | 43 +++++++++-- cli/functions/makeBinaryPatchBundle.test.ts | 31 ++++++++ cli/functions/makeBinaryPatchBundle.ts | 44 +++++++++-- 8 files changed, 258 insertions(+), 14 deletions(-) diff --git a/cli/README.ko.md b/cli/README.ko.md index de283925c..5c3075af0 100644 --- a/cli/README.ko.md +++ b/cli/README.ko.md @@ -108,6 +108,7 @@ npx code-push release [options] | `--output-bundle-dir ` | 번들 출력 디렉토리 이름 | `bundleOutput` | | `--output-metro-dir ` | Hermes 컴파일 전 Metro JS 번들과 소스맵을 복사할 디렉토리 | — | | `--binary-bundle-path ` | 대상 바이너리에 포함된 JS 번들 경로. 이 번들에 대한 binary patch 번들을 함께 배포하고, Hermes 컴파일을 이 번들에 정렬합니다 | — | +| `--on-oversized-patch ` | patch 번들이 full 번들보다 작지 않을 때의 동작: `skip`은 full 번들만 배포하고, `fail`은 업로드 전에 릴리스를 중단합니다 | `skip` | `--binary-bundle-path`를 사용하면 플랫폼별로 두 개의 artifact를 업로드합니다. `packageHash` 이름의 full 번들과, 바이너리에 포함된 번들과의 차이만 담은 `-patch.zip` patch @@ -115,6 +116,11 @@ npx code-push release [options] 포함되어, patch를 적용하면 full 번들과 동일한 `packageHash`가 됩니다. 두 artifact의 크기와 절감량은 업로드 전에 출력됩니다. +patch는 대체하려는 archive보다 작을 때만 배포할 가치가 있습니다. CLI는 사용자에게 묻지 +않으므로, patch 크기가 full 이상일 때의 동작을 `--on-oversized-patch`로 미리 정합니다. +기본값 `skip`은 경고를 남기고 요약에 skip 사실을 명시한 뒤 full 번들만 배포하며, `fail`은 +어떤 업로드도 시작하기 전에 릴리스를 실패시키고 릴리스 히스토리를 변경하지 않습니다. + **예시:** ```bash @@ -135,6 +141,9 @@ npx code-push release -b 1.0.0 -v 1.0.2 --skip-bundle true --hash-calc true # full 번들과 바이너리 번들에 대한 binary patch를 함께 배포 npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle + +# 동일하지만, patch가 더 작지 않으면 릴리스를 실패시킴 +npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle --on-oversized-patch fail ``` --- diff --git a/cli/README.md b/cli/README.md index cb41e4e90..a2949e0ac 100644 --- a/cli/README.md +++ b/cli/README.md @@ -106,6 +106,7 @@ npx code-push release [options] | `--output-bundle-dir ` | Bundle output directory name | `bundleOutput` | | `--output-metro-dir ` | Directory to copy Metro JS bundle and sourcemap before Hermes compilation | — | | `--binary-bundle-path ` | JS bundle of the target binary. Releases an additional binary patch bundle against it, and aligns the Hermes compilation with it | — | +| `--on-oversized-patch ` | What to do when the patch bundle is not smaller than the full bundle: `skip` releases the full bundle only, `fail` stops the release before any upload | `skip` | With `--binary-bundle-path`, the release uploads two artifacts per platform: the full bundle named after its `packageHash`, and a patch bundle named `-patch.zip` @@ -114,6 +115,12 @@ holds a `codepush-binary-patch.json` manifest describing how to rebuild the upda applying it yields the same `packageHash` as the full bundle. Both sizes and the saving are printed before either artifact is uploaded. +A patch is only worth publishing when it is smaller than the archive it replaces. The CLI +never prompts, so `--on-oversized-patch` decides in advance what happens when the patch +comes out the same size or larger: `skip` (the default) logs a warning, notes the skip in +the summary and releases the full bundle alone, while `fail` stops the release before +anything is uploaded and leaves the release history untouched. + ```bash # Standard iOS release npx code-push release -b 1.0.0 -v 1.0.1 -p ios @@ -132,6 +139,9 @@ npx code-push release -b 1.0.0 -v 1.0.2 --skip-bundle true --hash-calc true # Release a full bundle and a binary patch against the bundle in the binary npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle + +# Same, but fail the release if the patch does not come out smaller +npx code-push release -b 1.0.0 -v 1.0.1 -p ios --binary-bundle-path ./binary/main.jsbundle --on-oversized-patch fail ``` --- diff --git a/cli/commands/releaseCommand/index.test.ts b/cli/commands/releaseCommand/index.test.ts index a268df9b1..3c544112e 100644 --- a/cli/commands/releaseCommand/index.test.ts +++ b/cli/commands/releaseCommand/index.test.ts @@ -28,14 +28,29 @@ const ARG_INDEX = { skipBundle: 14, bundleDirectory: 16, baseBundlePath: 19, + onOversizedPatch: 20, } as const; -async function runReleaseCommand(args: string[]): Promise { - const { release } = await import("./release.js"); +/** + * Parses a `release` invocation against the real command definition. Commander is asked + * to throw instead of exiting, and to keep its diagnostics to itself, so a rejected + * option can be asserted on without ending the worker or the output. + */ +async function parseReleaseCommand(args: string[]): Promise { const { program } = await import("commander"); await import("./index.js"); + const releaseCommand = program.commands.find((command) => command.name() === 'release'); + releaseCommand?.exitOverride(); + releaseCommand?.configureOutput({ writeErr: () => {} }); + await program.parseAsync(['release', ...args], { from: 'user' }); +} + +async function runReleaseCommand(args: string[]): Promise { + const { release } = await import("./release.js"); + + await parseReleaseCommand(args); const releaseMock = jest.mocked(release); expect(releaseMock).toHaveBeenCalledTimes(1); @@ -80,4 +95,24 @@ describe("release command options", () => { expect(args[ARG_INDEX.entryFile]).toBe('index.ts'); expect(args[ARG_INDEX.bundleDirectory]).toBe('build/bundleOutput'); }); + + it("defaults the oversized patch policy to skipping the patch", async () => { + const args = await runReleaseCommand(['-b', '1.0.0', '-v', '1.0.1']); + + expect(args[ARG_INDEX.onOversizedPatch]).toBe('skip'); + }); + + it("passes the chosen oversized patch policy through to the release", async () => { + const args = await runReleaseCommand(['-b', '1.0.0', '-v', '1.0.1', '--on-oversized-patch', 'fail']); + + expect(args[ARG_INDEX.onOversizedPatch]).toBe('fail'); + }); + + it("rejects an oversized patch policy it does not know", async () => { + await expect(parseReleaseCommand(['-b', '1.0.0', '-v', '1.0.1', '--on-oversized-patch', 'ask'])) + .rejects.toThrow(/--on-oversized-patch.*'ask'.*skip, fail/s); + + const { release } = await import("./release.js"); + expect(jest.mocked(release)).not.toHaveBeenCalled(); + }); }); diff --git a/cli/commands/releaseCommand/index.ts b/cli/commands/releaseCommand/index.ts index cb0ea2e4c..f265f371d 100644 --- a/cli/commands/releaseCommand/index.ts +++ b/cli/commands/releaseCommand/index.ts @@ -1,7 +1,12 @@ import { program, Option } from "commander"; import { findAndReadConfigFile } from "../../utils/fsUtils.js"; import { release } from "./release.js"; -import { resolveBinaryBundlePathOption } from "../../functions/makeBinaryPatchBundle.js"; +import { + DEFAULT_OVERSIZED_PATCH_POLICY, + OVERSIZED_PATCH_POLICIES, + resolveBinaryBundlePathOption, + type OversizedPatchPolicy, +} from "../../functions/makeBinaryPatchBundle.js"; import { OUTPUT_BUNDLE_DIR, CONFIG_FILE_NAME, ROOT_OUTPUT_DIR, ENTRY_FILE } from "../../constant.js"; type Options = { @@ -24,6 +29,7 @@ type Options = { outputMetroDir?: string; hashCalc?: boolean; binaryBundlePath?: string; + onOversizedPatch: OversizedPatchPolicy; } program.command('release') @@ -46,6 +52,9 @@ program.command('release') .option('--output-metro-dir ', 'name of directory to copy the Metro JS bundle and sourcemap before Hermes compilation') .option('--output-bundle-dir ', 'name of directory containing the bundle file created by the "bundle" command', OUTPUT_BUNDLE_DIR) .option('--binary-bundle-path ', 'path to the JS bundle of the target binary. Releases an additional binary patch bundle against it, and aligns the Hermes compilation with it.') + .addOption(new Option('--on-oversized-patch ', 'what to do when the binary patch bundle is not smaller than the full bundle: "skip" releases the full bundle only, "fail" stops the release before any upload') + .choices(OVERSIZED_PATCH_POLICIES) + .default(DEFAULT_OVERSIZED_PATCH_POLICY)) .action(async (options: Options) => { const config = findAndReadConfigFile(process.cwd(), options.config); @@ -83,6 +92,7 @@ program.command('release') options.outputMetroDir, options.hashCalc, baseBundlePath, + options.onOversizedPatch, ) console.log('🚀 Release completed.') diff --git a/cli/commands/releaseCommand/release.test.ts b/cli/commands/releaseCommand/release.test.ts index f2bf7d0ae..da94c39cf 100644 --- a/cli/commands/releaseCommand/release.test.ts +++ b/cli/commands/releaseCommand/release.test.ts @@ -9,6 +9,7 @@ import { BINARY_PATCH_MANIFEST_NAME, hashBundleFile, writeBinaryPatchBaseRecord, + type OversizedPatchPolicy, } from "../../functions/makeBinaryPatchBundle.js"; import { applyPatch } from "../../utils/binaryPatch.js"; import { generatePackageHashFromDirectory } from "../../utils/hash-utils.js"; @@ -122,6 +123,7 @@ type ReleaseOverrides = { jsBundleName?: string; skipCleanup?: boolean; uploadFailsFor?: (filePath: string) => boolean; + onOversizedPatch?: OversizedPatchPolicy; }; async function runRelease(staged: StagedBundle, overrides: ReleaseOverrides = {}) { @@ -150,6 +152,7 @@ async function runRelease(staged: StagedBundle, overrides: ReleaseOverrides = {} undefined, undefined, overrides.binaryBundlePath, + overrides.onOversizedPatch, ); return { uploads, releaseHistories: history.saved }; @@ -365,6 +368,16 @@ describe("release --skip-bundle --binary-bundle-path", () => { expect(path.basename(uploads[0].filePath)).toBe(staged.bundleFileName); }); + it("uploads the patch when it is smaller than the full bundle, without warning about its size", async () => { + const staged = await stageBundleOutput("worthwhile-patch"); + + const { uploads } = await runRelease(staged, { binaryBundlePath: baseFixture }); + + expect(uploads).toHaveLength(2); + expect(logs.filter((line) => line.startsWith('warn:'))).toEqual([]); + expect(logs.join('\n')).not.toContain('Patch skipped:'); + }); + it("ships a patch archive a client can turn back into the released bundle", async () => { const staged = await stageBundleOutput("manifest", { 'main.jsbundle': fs.readFileSync(targetFixture), @@ -391,3 +404,74 @@ describe("release --skip-bundle --binary-bundle-path", () => { expect(await generatePackageHashFromDirectory(contents, extractRoot)).toBe(staged.bundleFileName); }); }); + +/** + * A patch is only worth publishing when it is smaller than the archive it replaces. The + * CLI runs unattended, so `--on-oversized-patch` decides what happens when it is not. + * + * An update whose bundle is a few bytes long produces one: the patch container and the + * manifest that describes it together outweigh the whole full archive. + */ +describe("release --on-oversized-patch", () => { + const tinyContents = () => ({ 'main.jsbundle': Buffer.from('tiny') }); + + it("skips the patch by default and releases the full bundle alone", async () => { + const staged = await stageBundleOutput("oversized-skip", tinyContents()); + + const { uploads, releaseHistories } = await runRelease(staged, { binaryBundlePath: baseFixture }); + + expect(uploads.map(({ filePath }) => path.basename(filePath))).toEqual([staged.bundleFileName]); + expect(logs.filter((line) => line.startsWith('warn:')).join('\n')).toMatch(/not smaller than the full archive/); + expect(logs.join('\n')).toContain('Patch skipped:'); + expect(releaseHistories[0][APP_VERSION].downloadUrl).toBe(uploads[0].downloadUrl); + }); + + it("fails before any upload when the policy is fail", async () => { + const staged = await stageBundleOutput("oversized-fail", tinyContents()); + + const uploads: Uploads = []; + const history = historyStore(); + + await expect( + release( + recordingUploader(uploads), + history.getReleaseHistory, + history.setReleaseHistory, + BINARY_VERSION, + APP_VERSION, + undefined, + 'ios', + undefined, + staged.outputPath, + 'index.ts', + undefined, + false, + true, + undefined, + true, + true, + staged.bundleDirectory, + undefined, + undefined, + baseFixture, + 'fail', + ), + ).rejects.toThrow(/not smaller than the full archive/); + + expect(uploads).toEqual([]); + expect(history.saved).toEqual([]); + // The temp directories are still cleaned up on the way out. + expect(fs.readdirSync(staged.outputPath)).toEqual([BUNDLE_OUTPUT_DIR_NAME]); + }); + + it("uploads a worthwhile patch even when the policy is fail", async () => { + const staged = await stageBundleOutput("worthwhile-under-fail"); + + const { uploads } = await runRelease(staged, { + binaryBundlePath: baseFixture, + onOversizedPatch: 'fail', + }); + + expect(uploads).toHaveLength(2); + }); +}); diff --git a/cli/commands/releaseCommand/release.ts b/cli/commands/releaseCommand/release.ts index b1dd68c82..896e6ae54 100644 --- a/cli/commands/releaseCommand/release.ts +++ b/cli/commands/releaseCommand/release.ts @@ -7,12 +7,15 @@ import { generatePackageHashFromDirectory } from "../../utils/hash-utils.js"; import { unzip } from "../../utils/unzip.js"; import { BINARY_PATCH_ARCHIVE_SUFFIX, + DEFAULT_OVERSIZED_PATCH_POLICY, extractCodePushBundleContents, formatBinaryPatchSummary, hashBundleFile, + isPatchArchiveOversized, makeBinaryPatchBundle, readBinaryPatchBaseRecord, type BinaryPatchBundle, + type OversizedPatchPolicy, } from "../../functions/makeBinaryPatchBundle.js"; export async function release( @@ -36,6 +39,7 @@ export async function release( outputMetroDir?: string, hashCalc?: boolean, baseBundlePath?: string, + onOversizedPatch: OversizedPatchPolicy = DEFAULT_OVERSIZED_PATCH_POLICY, ): Promise { const codePushBundle = skipBundle ? null @@ -61,6 +65,7 @@ export async function release( packageHash, outputPath, platform, + onOversizedPatch, }) : null; @@ -127,10 +132,16 @@ async function calcHashFromBundleFile(bundleFilePath: string): Promise { } /** - * Builds the binary patch artifact of this release and reports what it saves before - * anything is uploaded, so an unexpectedly large patch can still be stopped. + * Builds the binary patch artifact of this release and reports what it saves, before + * anything is uploaded. + * + * A patch that is not smaller than the full archive is not worth publishing, and the + * CLI cannot ask: it runs unattended. `onOversizedPatch` decides instead - `skip` + * releases the full bundle alone, `fail` stops the release while nothing has been + * uploaded yet. * * @param contentsPath {string | undefined} Update contents of a bundle that was just built. Absent with `--skip-bundle`, where the bundle file being released is unpacked instead, so the patch describes exactly the bytes that go out. + * @return {Promise} The artifact to upload, or `null` when the patch was skipped. */ async function makeBinaryPatchArtifact({ baseBundlePath, @@ -141,6 +152,7 @@ async function makeBinaryPatchArtifact({ packageHash, outputPath, platform, + onOversizedPatch, }: { baseBundlePath: string; contentsPath: string | undefined; @@ -150,7 +162,8 @@ async function makeBinaryPatchArtifact({ packageHash: string; outputPath: string; platform: 'ios' | 'android'; -}): Promise { + onOversizedPatch: OversizedPatchPolicy; +}): Promise { warnOnBaseBundleMismatch(outputPath, baseBundlePath); let patchContentsPath = contentsPath; @@ -170,14 +183,34 @@ async function makeBinaryPatchArtifact({ packageHash, }); + const fullArchiveSize = fs.statSync(bundleFilePath).size; + const patchArchiveSize = fs.statSync(binaryPatch.patchBundleFilePath).size; + const oversized = isPatchArchiveOversized(fullArchiveSize, patchArchiveSize); + console.log(formatBinaryPatchSummary({ platform, baseBundleHash: binaryPatch.manifest.baseBundleHash, targetBundleHash: binaryPatch.manifest.targetBundleHash, - fullArchiveSize: fs.statSync(bundleFilePath).size, - patchArchiveSize: fs.statSync(binaryPatch.patchBundleFilePath).size, + fullArchiveSize, + patchArchiveSize, + patchSkipped: oversized && onOversizedPatch === 'skip', })); + if (oversized) { + if (onOversizedPatch === 'fail') { + throw new Error( + `The binary patch archive (${patchArchiveSize} bytes) is not smaller than the full archive (${fullArchiveSize} bytes), ` + + 'and --on-oversized-patch is set to "fail". Nothing was uploaded.', + ); + } + + console.warn( + `warn: The binary patch archive (${patchArchiveSize} bytes) is not smaller than the full archive (${fullArchiveSize} bytes). ` + + 'Releasing the full bundle only, without a binary patch.', + ); + return null; + } + return binaryPatch; } finally { if (extractDir) { diff --git a/cli/functions/makeBinaryPatchBundle.test.ts b/cli/functions/makeBinaryPatchBundle.test.ts index d5b6894fd..bfab29a83 100644 --- a/cli/functions/makeBinaryPatchBundle.test.ts +++ b/cli/functions/makeBinaryPatchBundle.test.ts @@ -9,6 +9,7 @@ import { BINARY_PATCH_MANIFEST_NAME, extractCodePushBundleContents, formatBinaryPatchSummary, + isPatchArchiveOversized, makeBinaryPatchBundle, readBinaryPatchBaseRecord, resolveBaseBundlePath, @@ -339,6 +340,20 @@ describe("extractCodePushBundleContents", () => { }); }); +describe("isPatchArchiveOversized", () => { + it("keeps a patch that is smaller than the full archive", () => { + expect(isPatchArchiveOversized(100, 99)).toBe(false); + }); + + it("rejects a patch of exactly the same size, which saves a client nothing", () => { + expect(isPatchArchiveOversized(100, 100)).toBe(true); + }); + + it("rejects a patch that is larger than the full archive", () => { + expect(isPatchArchiveOversized(100, 101)).toBe(true); + }); +}); + describe("formatBinaryPatchSummary", () => { it("reports both archive sizes and what the patch saves", () => { const baseBundleHash = 'a'.repeat(64); @@ -375,5 +390,21 @@ describe("formatBinaryPatchSummary", () => { expect(summary).toContain('Binary patch summary (android)'); expect(summary).toContain('Saved: -500 B (-50.0%)'); + expect(summary).not.toContain('Patch skipped:'); + }); + + it("states that the patch was skipped, and why, when it is not being released", () => { + const summary = formatBinaryPatchSummary({ + platform: 'android', + baseBundleHash: 'a'.repeat(64), + targetBundleHash: 'b'.repeat(64), + fullArchiveSize: 1_000, + patchArchiveSize: 1_500, + patchSkipped: true, + }); + + expect(summary.split('\n').at(-1)).toBe( + 'Patch skipped: not smaller than the full archive; releasing the full bundle only (--on-oversized-patch skip)', + ); }); }); diff --git a/cli/functions/makeBinaryPatchBundle.ts b/cli/functions/makeBinaryPatchBundle.ts index 2ac32b997..1f7b4e6c6 100644 --- a/cli/functions/makeBinaryPatchBundle.ts +++ b/cli/functions/makeBinaryPatchBundle.ts @@ -253,10 +253,32 @@ export function readBinaryPatchBaseRecord(outputRootPath: string): BinaryPatchBa } /** - * The operator-facing summary, printed before the artifacts are uploaded so the size - * of what is about to be published is visible while it can still be stopped. A patch - * larger than the full archive is reported as a negative saving and left in place; - * whether that is worth shipping is a judgement call. + * What a release does with a patch archive that did not turn out smaller than the full + * archive. The CLI runs unattended in CI, so the answer is a policy chosen up front + * rather than a decision the summary invites someone to make. + */ +export type OversizedPatchPolicy = 'skip' | 'fail'; + +/** Accepted `--on-oversized-patch` values, in the order the help text lists them. */ +export const OVERSIZED_PATCH_POLICIES: OversizedPatchPolicy[] = ['skip', 'fail']; + +export const DEFAULT_OVERSIZED_PATCH_POLICY: OversizedPatchPolicy = 'skip'; + +/** + * Whether the patch archive is worth publishing next to the full archive. + * + * Equal sizes count as oversized: a patch that saves nothing still costs a client the + * download plus an apply step, so it is never the better artifact of the two. + */ +export function isPatchArchiveOversized(fullArchiveSize: number, patchArchiveSize: number): boolean { + return patchArchiveSize >= fullArchiveSize; +} + +/** + * The operator-facing summary, printed before the artifacts are uploaded so the size of + * what is about to be published is on the record. When the patch is not smaller than the + * full archive and the release is going ahead without it, the summary says so rather + * than leaving a negative saving to be interpreted. */ export function formatBinaryPatchSummary({ platform, @@ -264,12 +286,14 @@ export function formatBinaryPatchSummary({ targetBundleHash, fullArchiveSize, patchArchiveSize, + patchSkipped = false, }: { platform: 'ios' | 'android'; baseBundleHash: string; targetBundleHash: string; fullArchiveSize: number; patchArchiveSize: number; + patchSkipped?: boolean; }): string { const savedBytes = fullArchiveSize - patchArchiveSize; const savedRatio = fullArchiveSize > 0 ? savedBytes / fullArchiveSize : 0; @@ -280,14 +304,22 @@ export function formatBinaryPatchSummary({ const label = (text: string) => text.padEnd(23); - return [ + const lines = [ `Binary patch summary (${platform})`, `${label('Base bundle SHA-256:')}${baseBundleHash}`, `${label('Target bundle SHA-256:')}${targetBundleHash}`, `${label('Full archive:')}${fullSize}`, `${label('Patch archive:')}${patchSize}`, `${label('Saved:')}${savedSize} (${(savedRatio * 100).toFixed(1)}%)`, - ].join('\n'); + ]; + + if (patchSkipped) { + lines.push( + `${label('Patch skipped:')}not smaller than the full archive; releasing the full bundle only (--on-oversized-patch skip)`, + ); + } + + return lines.join('\n'); } function formatBytes(bytes: number): string {