Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1780,6 +1780,25 @@ jobs:
exit 1
fi
mnt="$(mktemp -d)"
# Release the mount on ANY step exit — errexit mid-verify or the job's
# timeout signal must not leak the attachment (a reused runner then
# hits "Resource busy" on the next hdiutil create). Idempotent and
# error-swallowing: a failing trap during exit would mask the real
# status.
trap 'hdiutil detach "$mnt" >/dev/null 2>&1 || hdiutil detach "$mnt" -force >/dev/null 2>&1 || true' EXIT
# Force-detach any Compass attachment a prior job on this reused runner
# left behind. Narrow by design: only mount points whose basename is
# exactly "Compass" or a "Compass N" duplicate, so an unrelated volume
# (e.g. /Volumes/CompassUnrelated) is never touched. The mount path is
# cut from the tab-delimited entity line rather than taken as the last
# field, so a path containing spaces survives intact.
hdiutil info 2>/dev/null \
| sed -n 's#^/dev/[^ ]* *\(/Volumes/Compass.*\)$#\1#p' \
| while read -r stale; do
case "$(basename "$stale")" in
Compass|Compass\ [0-9]*) hdiutil detach "$stale" -force >/dev/null 2>&1 || true ;;
esac
done
hdiutil attach "$out" -mountpoint "$mnt" -nobrowse -readonly
rc=0
BINARIES=(
Expand Down Expand Up @@ -1810,7 +1829,7 @@ jobs:
rc=1
fi
done
hdiutil detach "$mnt" || hdiutil detach "$mnt" -force || true
# Detach runs via the EXIT trap installed above, unconditionally.
if [ "$rc" != 0 ]; then
exit 1
fi
Expand Down
57 changes: 56 additions & 1 deletion tools/macos-bundle/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// import.meta.main-guarded, so importing index.ts never runs it.

import { describe, expect, test } from "bun:test";
import { parseArgs, renderInfoPlist } from "./index.ts";
import { parseArgs, renderInfoPlist, staleMountPoints } from "./index.ts";

/** The canonical render inputs used across the plist cases. */
function plistOpts() {
Expand Down Expand Up @@ -273,3 +273,58 @@ describe("parseArgs — fails loud on malformed input", () => {
);
});
});

/**
* Build one `hdiutil info -plist` image block for an image at `imagePath` with
* the given mount points. The real plist is larger; these are the only keys
* staleMountPoints reads, in the order hdiutil emits them.
*/
function imageBlock(imagePath: string, mountPoints: string[]): string {
const entities = mountPoints
.map((mp) => `<dict><key>mount-point</key><string>${mp}</string></dict>`)
.join("");
return `<key>image-path</key><string>${imagePath}</string><key>system-entities</key><array>${entities}</array>`;
}

const TARGET = {
imagePath: "/tmp/compass-app-darwin-arm64.dmg",
volumeName: "Compass",
};

describe("staleMountPoints — selects only this build's leaked attachment", () => {
test("no attachments → empty", () => {
expect(staleMountPoints("<dict></dict>", TARGET)).toEqual([]);
});

test("empty input → empty, does not throw", () => {
expect(staleMountPoints("", TARGET)).toEqual([]);
});

test("malformed input → empty, does not throw", () => {
expect(staleMountPoints("not a plist <<< >>>", TARGET)).toEqual([]);
});

test("matches by image path", () => {
const info = imageBlock(TARGET.imagePath, ["/Volumes/Compass"]);
expect(staleMountPoints(info, TARGET)).toEqual(["/Volumes/Compass"]);
});

test("matches by Compass volume name even when the image path differs", () => {
const info = imageBlock("/tmp/some-other-run.dmg", [
"/private/tmp/Compass",
]);
expect(staleMountPoints(info, TARGET)).toEqual(["/private/tmp/Compass"]);
});

test("does NOT select an unrelated volume", () => {
const info = imageBlock("/tmp/unrelated.dmg", ["/Volumes/SomethingElse"]);
expect(staleMountPoints(info, TARGET)).toEqual([]);
});

test("selects only the matching block when both are attached", () => {
const info =
imageBlock("/tmp/unrelated.dmg", ["/Volumes/SomethingElse"]) +
imageBlock(TARGET.imagePath, ["/Volumes/Compass"]);
expect(staleMountPoints(info, TARGET)).toEqual(["/Volumes/Compass"]);
});
});
64 changes: 63 additions & 1 deletion tools/macos-bundle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,48 @@ function assertSidecarBasenamesDistinct(sidecars: string[]): void {
}
}

/** Extract one image block's `mount-point` values, in document order. */
function blockMountPoints(block: string): string[] {
const mountRe = /<key>mount-point<\/key>\s*<string>([^<]*)<\/string>/g;
const mounts: string[] = [];
for (let m = mountRe.exec(block); m; m = mountRe.exec(block)) {
if (m[1]) mounts.push(m[1]);
}
return mounts;
}

/**
* Parse `hdiutil info -plist` text and return the mount points that a stale
* attachment of THIS build holds — so the caller can force-detach them before
* `hdiutil create`, which fails with "Resource busy" when an earlier run on a
* reused runner leaked the attachment of our image path or volume. Pure: it
* only reads text, so the parsing (where a bug would hide) is unit-testable.
*
* Conservative by construction: a block is selected ONLY when its image-path
* equals `imagePath` or one of its mount points is named `volumeName`, so an
* unrelated volume is never returned. Unparseable/empty input yields `[]`.
*/
export function staleMountPoints(
hdiutilInfoPlist: string,
target: { imagePath: string; volumeName: string },
): string[] {
// Each attached image is a block introduced by its <key>image-path</key>;
// splitting on that key isolates one image's system-entities per chunk.
const blocks = hdiutilInfoPlist.split("<key>image-path</key>").slice(1);
const found = new Set<string>();
for (const block of blocks) {
const imagePath = /^\s*<string>([^<]*)<\/string>/.exec(block)?.[1];
const mounts = blockMountPoints(block);
const matches =
imagePath === target.imagePath ||
mounts.some((mp) => basename(mp) === target.volumeName);
if (matches) {
for (const mp of mounts) found.add(mp);
}
}
return [...found];
}

// ── The edge (impure) ──────────────────────────────────────────────────────

/** Fail loud if a required input path does not exist (build.sh sanity posture). */
Expand All @@ -218,6 +260,24 @@ async function assertExists(path: string, what: string): Promise<void> {
}
}

/**
* Force-detach any stale attachment of our volume/image left by an earlier run
* on a reused runner, so `hdiutil create` does not hit "Resource busy". Never
* throws: a clean system (nothing to detach) and an unavailable/unparseable
* `hdiutil info` both leave the build untouched — a cleanup that reds a green
* system is worse than the leak.
*/
async function detachStaleAttachments(target: {
imagePath: string;
volumeName: string;
}): Promise<void> {
const probe = await $`hdiutil info -plist`.quiet().nothrow();
if (probe.exitCode !== 0) return;
for (const mount of staleMountPoints(probe.stdout.toString(), target)) {
await $`hdiutil detach ${mount} -force`.quiet().nothrow();
}
}

async function main(): Promise<void> {
const args = parseArgs(Bun.argv.slice(2));

Expand Down Expand Up @@ -276,8 +336,10 @@ async function main(): Promise<void> {
await $`codesign --sign - --force --deep ${appDir}`;

// Wrap the staging dir into a compressed (UDZO) .dmg. -ov overwrites an
// existing image so a re-run is idempotent.
// existing image so a re-run is idempotent. Detach any stale attachment of
// this volume first — on a reused runner a leaked mount makes create EBUSY.
await rm(args.out, { force: true });
await detachStaleAttachments({ imagePath: args.out, volumeName: "Compass" });
await $`hdiutil create -volname Compass -srcfolder ${stageRoot} -ov -format UDZO ${args.out}`;

console.log(`macos-bundle: wrote ${args.out}`);
Expand Down
Loading