fix(electron): retry DMG detach using the disk device

Author: ibuler

.github/workflows/build.yml

@@ -127,8 +127,8 @@ jobs:
       - name: Install dependencies
         run: pnpm install --frozen-lockfile
 
-      - name: Validate Windows packaging configuration
-        if: startsWith(matrix.os, 'windows')
+      - name: Validate packaging configuration
+        if: startsWith(matrix.os, 'windows') || startsWith(matrix.os, 'macos')
         run: pnpm --dir electron exec node --import tsx --test tests/forge-artifacts.test.ts
 
       - name: Sync version

electron/tests/forge-artifacts.test.ts

@@ -5,13 +5,58 @@ import { mkdir, mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promis
 import os from "node:os";
 import path from "node:path";
 import test from "node:test";
+import { promisify } from "node:util";
 import { createJiti } from "jiti";
 import { MakerNsis } from "../maker-nsis";
 import { MakerWix } from "@electron-forge/maker-wix";
 
 // Load the configuration with the same TypeScript loader used by Electron Forge.
 const configPromise = createJiti(import.meta.url).import<ForgeConfig>("../forge.config.ts", { default: true });
 
+test(
+  "DMG cleanup retries the device after the volume has been unmounted",
+  { skip: process.platform !== "darwin" },
+  async (context) => {
+    const require = createRequire(import.meta.url);
+    const installerRequire = createRequire(require.resolve("electron-installer-dmg"));
+    const appdmgRequire = createRequire(installerRequire.resolve("appdmg"));
+    const appdmg = installerRequire("appdmg");
+    const Pipeline = appdmgRequire("./lib/pipeline");
+    const util = appdmgRequire("./lib/util");
+
+    // Exercise the installed dependency's mount/cleanup steps without mounting disks.
+    context.mock.method(Pipeline.prototype, "run", function () {
+      return this;
+    });
+    for (const stdout of [
+      "/dev/disk4\tApple_partition_scheme\n/dev/disk4s1\tApple_HFS\t/Volumes/JumpServer Test\n",
+      "/dev/disk4\tGUID_partition_scheme\n/dev/disk4s2\tApple_APFS\n/dev/disk5\tEF57347C-0000-11AA-AA11-00306543ECAC\n/dev/disk5s1\t41504653-0000-11AA-AA11-00306543ECAC\t/Volumes/JumpServer Test\n"
+    ]) {
+      const detachTargets: string[] = [];
+      context.mock.method(util, "sh", (_command, args, callback) => {
+        if (args[0] === "attach") return callback(null, { stdout });
+        assert.equal(args[0], "detach");
+        detachTargets.push(args[1]);
+        if (detachTargets.length === 1) {
+          return callback(Object.assign(new Error("Resource busy after unmount"), { code: 16 }));
+        }
+        if (args[1].startsWith("/Volumes/")) {
+          return callback(Object.assign(new Error("No such file or directory"), { code: 2 }));
+        }
+        callback(null);
+      });
+      const pipeline = appdmg({ target: "unused.dmg", basepath: ".", specification: {} });
+      await promisify(pipeline.steps.find((step) => step.title === "Mounting temporary image").fn)();
+      await promisify(pipeline.steps.find((step) => step.title === "Unmounting temporary image").fn)();
+      assert.deepEqual(detachTargets, ["/dev/disk4", "/dev/disk4"]);
+    }
+
+    const denied = Object.assign(new Error("Permission denied"), { code: 13 });
+    context.mock.method(util, "sh", (_command, _args, callback) => callback(denied));
+    await assert.rejects(promisify(appdmgRequire("./lib/hdiutil").detach)("/dev/disk4"), denied);
+  }
+);
+
 test("NSIS builder resolves the download cache API required on clean machines", () => {
   const require = createRequire(import.meta.url);
   const builderRequire = createRequire(require.resolve("app-builder-lib"));

patches/appdmg@0.6.6.patch

@@ -0,0 +1,36 @@
+diff --git a/lib/appdmg.js b/lib/appdmg.js
+index 77748bfcdaf55ea9597f65b03c9953c63ffa50dc..ade23ea0f311a18d88b4344362430b64e10a78eb 100644
+--- a/lib/appdmg.js
++++ b/lib/appdmg.js
+@@ -223,11 +223,13 @@ module.exports = exports = function (options) {
+    **/
+ 
+   pipeline.addStep('Mounting temporary image', function (next) {
+-    hdiutil.attach(global.temporaryImagePath, function (err, temporaryMountPath) {
++    hdiutil.attach(global.temporaryImagePath, function (err, temporaryMountPath, temporaryDevice) {
+       if (err) return next(err)
+ 
+       pipeline.addCleanupStep('unmount-temporary-image', 'Unmounting temporary image', function (next) {
+-        hdiutil.detach(temporaryMountPath, next)
++        // A busy detach can unmount the volume before failing. Retry the device,
++        // since the mount path may no longer exist: LinusU/node-appdmg#228.
++        hdiutil.detach(temporaryDevice, next)
+       })
+ 
+       global.temporaryMountPath = temporaryMountPath
+diff --git a/lib/hdiutil.js b/lib/hdiutil.js
+index a85a3af77883bfd25480db3426a491c4c6ad1632..7f072ba9f999a45d0450b864eea33364426d7a16 100644
+--- a/lib/hdiutil.js
++++ b/lib/hdiutil.js
+@@ -56,7 +56,10 @@ exports.attach = function (path, cb) {
+     const m = /\s+(\/Volumes\/.+)$/m.exec(res.stdout)
+     if (m === null) return cb(new Error('Failed to mount image'))
+ 
+-    cb(null, m[1])
++    const device = /^(\/dev\/disk\d+)\s/m.exec(res.stdout)
++    if (device === null) return cb(new Error('Failed to find mounted image device'))
++
++    cb(null, m[1], device[1])
+   })
+ }
+ 

pnpm-lock.yaml

@@ -32,6 +32,7 @@ allowBuilds:
   vue-demi: true
 
 patchedDependencies:
+  appdmg@0.6.6: patches/appdmg@0.6.6.patch
   asciinema-player@3.17.0: patches/asciinema-player@3.17.0.patch
   guacamole-common-js-jumpserver@1.1.0-c: >-
     patches/guacamole-common-js-jumpserver@1.1.0-c.patch