Skip to content

emrg: fix v0.2.78 build — entitlements plist comment breaks codesign AMFI + Windows smoke cleanup file-lock failure - #975

Merged
argszero merged 1 commit into
masterfrom
feature/fix-v0.2.78-build
Aug 25, 2026
Merged

emrg: fix v0.2.78 build — entitlements plist comment breaks codesign AMFI + Windows smoke cleanup file-lock failure#975
argszero merged 1 commit into
masterfrom
feature/fix-v0.2.78-build

Conversation

@argszero

Copy link
Copy Markdown
Owner

v0.2.78 Build Release (run 32813433122) failed on 2 jobs. Both root causes found and fixed:

1. macOS — Make installer failed (bundled python missing disable-library-validation entitlement: .../bin/python-dist/bin/python3.13)

packaging/assets/python-entitlements.plist (added in #967) contained an XML comment with CJK text + em-dashes + quotes. codesign --entitlements parses entitlements with the strict AMFIUnserializeXML parser, which rejects the comment → Failed to parse entitlements: AMFIUnserializeXML: syntax error near line 6 → the python3.13 binary was skipped (stderr swallowed by 2>/dev/null in the loop) → #967's fail-loud verify then tripped and exited 1.

Reproduced locally: codesign with the commented plist fails with the exact AMFI error; with the comment removed it succeeds and codesign -d --entitlements - shows com.apple.security.cs.disable-library-validation applied.

Fix: removed the XML comment from the plist (the full rationale already lives in the make-installer.sh codesign-section comment).

2. Windows — Smoke test step failed (rm: cannot remove .../.emrg/emrgd.log: Device or resource busy)

The smoke test passed 14/14, but the EXIT trap rm -rf "$smoke_home" failed: the temp daemon started in step 2 is no longer stopped at the end (correctly removed by #972's red-line guard — the script must never stop/restart emrgd), so on Windows it still holds emrgd.log/emrgd-crash.log open → rm returns "Device or resource busy" → set -e propagates → exit 1.

Fix: made the trap best-effort (rm -rf "$smoke_home" 2>/dev/null || true) with a comment documenting why. No daemon stop/restart anywhere (red-line preserved); the temp daemon is killed by the CI runner's orphan-process cleanup, and local runs are guarded by the port-56031 degraded probe.

Verification: plist codesign test passes with the entitlement applied; bash -n on both scripts OK; full suite 1060 passed / 1 skipped; import + CLI OK.

After merge, the v0.2.78 tag will be re-pointed to the fix commit and Build Release re-run.

…AMFI + Windows smoke cleanup file-lock failure

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (1/3)

Own fix PR for the failed v0.2.78 Build Release (run 32813433122). Fresh review of both fixes:

Fix 1 — macOS: packaging/assets/python-entitlements.plist XML comment (CJK + em-dashes + quotes) breaks codesign --entitlements — the strict AMFIUnserializeXML parser rejects comments (syntax error near line 6) → python3.13 skipped → #967 fail-loud verify trips. Reproduced locally before/after: commented plist → exact AMFI error; cleaned plist → codesign exit 0 and codesign -d --entitlements - shows com.apple.security.cs.disable-library-validation applied. The rationale comment already lives in make-installer.sh — no info lost.

Fix 2 — Windows: smoke-test EXIT trap rm -rf "$smoke_home" hit Device or resource busy on the temp daemon's held log files (daemon no longer stopped — the correct #972 red-line behavior). Changed to best-effort rm -rf "$smoke_home" 2>/dev/null || true with an explanatory comment. No stop/restart path anywhere — red-line preserved; CI orphan-process cleanup handles the temp daemon.

Verification: bash -n both scripts OK; full suite 1060 passed / 1 skipped; import + CLI OK. CI double-green (run 32814370743: test + test-windows). No issues.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (2/3)

Second independent review. Head unchanged (e4e1dc0); CI still double-green (run 32814370743: test + test-windows pass); mergeable CLEAN. Diff re-verified against master (cf50c53):

  • plist: only the 9-line XML comment removed; the entitlement key remains exactly com.apple.security.cs.disable-library-validation = true. The codesign loop's python*) case (make-installer.sh:66-77) still applies it — no functional change beyond the parser fix.
  • smoke-test.sh: EXIT trap now rm -rf "$smoke_home" 2>/dev/null || true — best-effort cleanup only, no daemon stop/restart anywhere (red-line preserved), failure cannot alter exit code.

Both fixes target the exact root causes confirmed in the failed v0.2.78 Build Release (run 32813433122), each reproduced locally before/after. No new issues.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (3/3)

Third independent review of the v0.2.78 build fix. Head unchanged (e4e1dc0), CI double-green (run 32814370743: test + test-windows). Diff re-verified against master:

  1. packaging/assets/python-entitlements.plist — removes only the 9-line XML comment. The entitlement key com.apple.security.cs.disable-library-validation is untouched; explanation already lives in make-installer.sh (lines 66-77). Comment-free plist codesigns cleanly (AMFI strict parser rejects XML comments).
  2. packaging/smoke-test.sh — EXIT trap becomes best-effort (rm -rf "$smoke_home" 2>/dev/null || true): a cleanup failure on the temp daemon's locked logs (Windows 'Device or resource busy') must not fail the build. The red-line guard (no stop/restart of emrg server/emrgd) is preserved — the daemon is deliberately left running, hence the locked log is expected.

Both fixes correct and minimal. Merge when this lands.

@argszero
argszero merged commit 95a983e into masterAug 25, 2026
2 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@argszero