From c2392de71a8f7abc092a00452eac63dd24b34e88 Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Wed, 13 Mar 2024 15:10:27 -0700 Subject: [PATCH] fix: attempting to wrap all `hdiutil`'s in a common retry mechanism for CI stability (#8135) --- .changeset/fast-maps-dress.md | 11 +++++++++++ packages/builder-util/src/util.ts | 8 ++++---- packages/dmg-builder/src/dmg.ts | 13 ++++++------- packages/dmg-builder/src/dmgUtil.ts | 10 +++------- packages/dmg-builder/src/hdiuil.ts | 15 +++++++++++++++ 5 files changed, 39 insertions(+), 18 deletions(-) create mode 100644 .changeset/fast-maps-dress.md create mode 100644 packages/dmg-builder/src/hdiuil.ts diff --git a/.changeset/fast-maps-dress.md b/.changeset/fast-maps-dress.md new file mode 100644 index 00000000000..4471b4fd0bb --- /dev/null +++ b/.changeset/fast-maps-dress.md @@ -0,0 +1,11 @@ +--- +"app-builder-lib": patch +"builder-util": patch +"dmg-builder": patch +"electron-builder": patch +"electron-builder-squirrel-windows": patch +"electron-publish": patch +"electron-updater": patch +--- + +fix: unstable hdiutil retry mechanism diff --git a/packages/builder-util/src/util.ts b/packages/builder-util/src/util.ts index f89fc610854..e6175d4d8a8 100644 --- a/packages/builder-util/src/util.ts +++ b/packages/builder-util/src/util.ts @@ -404,14 +404,14 @@ export async function executeAppBuilder( } } -export async function retry(task: () => Promise, retriesLeft: number, interval: number, backoff = 0, attempt = 0): Promise { +export async function retry(task: () => Promise, retryCount: number, interval: number, backoff = 0, attempt = 0, shouldRetry?: (e: any) => boolean): Promise { try { return await task() } catch (error: any) { - log.info(`Above command failed, retrying ${retriesLeft} more times`) - if (retriesLeft > 0) { + log.info(`Above command failed, retrying ${retryCount} more times`) + if ((shouldRetry?.(error) ?? true) && retryCount > 0) { await new Promise(resolve => setTimeout(resolve, interval + backoff * attempt)) - return await retry(task, retriesLeft - 1, interval, backoff, attempt + 1) + return await retry(task, retryCount - 1, interval, backoff, attempt + 1, shouldRetry) } else { throw error } diff --git a/packages/dmg-builder/src/dmg.ts b/packages/dmg-builder/src/dmg.ts index bdedad28367..3504693b705 100644 --- a/packages/dmg-builder/src/dmg.ts +++ b/packages/dmg-builder/src/dmg.ts @@ -4,7 +4,7 @@ import MacPackager from "app-builder-lib/out/macPackager" import { createBlockmap } from "app-builder-lib/out/targets/differentialUpdateInfoBuilder" import { executeAppBuilderAsJson } from "app-builder-lib/out/util/appBuilder" import { sanitizeFileName } from "app-builder-lib/out/util/filename" -import { Arch, AsyncTaskManager, exec, getArchSuffix, InvalidConfigurationError, isEmptyOrSpaces, log, spawn, retry } from "builder-util" +import { Arch, AsyncTaskManager, exec, getArchSuffix, InvalidConfigurationError, isEmptyOrSpaces, log } from "builder-util" import { CancellationToken } from "builder-util-runtime" import { copyDir, copyFile, exists, statOrNull } from "builder-util/out/fs" import { stat } from "fs-extra" @@ -13,6 +13,7 @@ import { TmpDir } from "temp-file" import { addLicenseToDmg } from "./dmgLicense" import { attachAndExecute, computeBackground, detach, getDmgVendorPath } from "./dmgUtil" import { release as getOsRelease } from "os" +import { hdiUtil } from "./hdiuil" export class DmgTarget extends Target { readonly options: DmgOptions = this.packager.config.dmg || Object.create(null) @@ -51,7 +52,7 @@ export class DmgTarget extends Target { const backgroundFile = specification.background == null ? null : await transformBackgroundFileIfNeed(specification.background, packager.info.tempDirManager) const finalSize = await computeAssetSize(packager.info.cancellationToken, tempDmg, specification, backgroundFile) const expandingFinalSize = finalSize * 0.1 + finalSize - await exec("hdiutil", ["resize", "-size", expandingFinalSize.toString(), tempDmg]) + await hdiUtil(["resize", "-size", expandingFinalSize.toString(), tempDmg]) const volumePath = path.join("/Volumes", volumeName) if (await exists(volumePath)) { @@ -68,9 +69,9 @@ export class DmgTarget extends Target { if (specification.format === "UDZO") { args.push("-imagekey", `zlib-level=${process.env.ELECTRON_BUILDER_COMPRESSION_LEVEL || "9"}`) } - await spawn("hdiutil", addLogLevel(args)) + await hdiUtil(addLogLevel(args)) if (this.options.internetEnabled && parseInt(getOsRelease().split(".")[0], 10) < 19) { - await exec("hdiutil", addLogLevel(["internet-enable"]).concat(artifactPath)) + await hdiUtil(addLogLevel(["internet-enable"]).concat(artifactPath)) } const licenseData = await addLicenseToDmg(packager, artifactPath) @@ -210,9 +211,7 @@ async function createStageDmg(tempDmg: string, appPath: string, volumeName: stri } imageArgs.push("-fs", ...filesystem) imageArgs.push(tempDmg) - // The reason for retrying up to ten times is that hdiutil create in some cases fail to unmount due to "resource busy". - // https://github.com/electron-userland/electron-builder/issues/5431 - await retry(() => spawn("hdiutil", imageArgs), 5, 1000) + await hdiUtil(imageArgs) return tempDmg } diff --git a/packages/dmg-builder/src/dmgUtil.ts b/packages/dmg-builder/src/dmgUtil.ts index 1d50476329e..9c57e879916 100644 --- a/packages/dmg-builder/src/dmgUtil.ts +++ b/packages/dmg-builder/src/dmgUtil.ts @@ -1,7 +1,7 @@ -import { exec, retry } from "builder-util" import { PlatformPackager } from "app-builder-lib" import { executeFinally } from "builder-util/out/promise" import * as path from "path" +import { hdiUtil } from "./hdiuil" export { DmgTarget } from "./dmg" @@ -23,7 +23,7 @@ export async function attachAndExecute(dmgPath: string, readWrite: boolean, task } args.push(dmgPath) - const attachResult = await exec("hdiutil", args) + const attachResult = await hdiUtil(args) const deviceResult = attachResult == null ? null : /^(\/dev\/\w+)/.exec(attachResult) const device = deviceResult == null || deviceResult.length !== 2 ? null : deviceResult[1] if (device == null) { @@ -34,11 +34,7 @@ export async function attachAndExecute(dmgPath: string, readWrite: boolean, task } export async function detach(name: string) { - try { - await exec("hdiutil", ["detach", "-quiet", name]) - } catch (e: any) { - await retry(() => exec("hdiutil", ["detach", "-force", "-debug", name]), 5, 1000, 500) - } + return hdiUtil(["detach", "-quiet", name]) } export async function computeBackground(packager: PlatformPackager): Promise { diff --git a/packages/dmg-builder/src/hdiuil.ts b/packages/dmg-builder/src/hdiuil.ts new file mode 100644 index 00000000000..74bf0b2828c --- /dev/null +++ b/packages/dmg-builder/src/hdiuil.ts @@ -0,0 +1,15 @@ +import { exec, log, retry } from "builder-util" + +export async function hdiUtil(args: string[]) { + return retry( + () => exec("hdiutil", args), + 5, + 1000, + 2000, + 0, + (error: any) => { + log.error({ args, code: error.code, error: (error.message || error).toString() }, "unable to execute hdiutil") + return true + } + ) +}