xtask: re-sign binaries after pdump fingerprint patch on macOS#91
Conversation
Patching the pdump fingerprint modifies the executable image in-place,
which invalidates the macOS code signature. Without a valid signature
the kernel sends SIGKILL (signal 9) when the binary is executed during
the pbootstrap and pdump steps.
After copy_executable_role_images copies the patched final_bin to the
temacs and bootstrap roles, re-sign all three binaries with an ad-hoc
signature (codesign --sign -) so the kernel accepts them.
Fixes build failure on Apple Silicon:
error: command failed with status signal: 9 (SIGKILL):
neomacs-temacs --batch -l loadup --temacs=pbootstrap
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a macOS-specific build step to re-sign role binaries after patching the pdump fingerprint, which is necessary to prevent execution failures due to invalid code signatures. The review feedback highlights several improvement opportunities: ensuring the new logic respects the dry run flag, utilizing the project's existing command execution helpers for consistent logging and error handling, and improving path handling to avoid potential panics.
| // macOS: re-sign all role binaries after patching. Patching the pdump | ||
| // fingerprint modifies the executable image in-place, which invalidates | ||
| // the code signature. Without a fresh ad-hoc signature the kernel sends | ||
| // SIGKILL when the binary is executed (exit status: signal 9). | ||
| #[cfg(target_os = "macos")] | ||
| { | ||
| for bin in [&paths.temacs, &paths.bootstrap, &paths.final_bin] { | ||
| if bin.exists() { | ||
| let status = std::process::Command::new("codesign") | ||
| .args(["--force", "--sign", "-", bin.to_str().unwrap()]) | ||
| .status()?; | ||
| if !status.success() { | ||
| return Err( | ||
| format!("codesign failed on {}", bin.display()).into() | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation for re-signing binaries on macOS has a few issues that should be addressed to maintain consistency with the rest of the build pipeline:
- Dry Run Support: It does not respect the
options.dry_runflag. A dry run should print the commands that would be executed without actually running them. Currently, this block either executes the command or does nothing, without informing the user. - Consistency: The project uses a
run_commandhelper (line 2334) to manage external process execution, including logging and error reporting. Bypassing it here leads to inconsistent output and error handling. - Robustness: Using
bin.to_str().unwrap()is risky as it will panic if a path contains invalid UTF-8. UsingOsStringvia therun_commandinterface is safer and more idiomatic in Rust.
Leveraging the existing helpers will resolve these issues and ensure the build script behaves predictably.
// macOS: re-sign all role binaries after patching. Patching the pdump
// fingerprint modifies the executable image in-place, which invalidates
// the code signature. Without a fresh ad-hoc signature the kernel sends
// SIGKILL when the binary is executed (exit status: signal 9).
#[cfg(target_os = "macos")]
{
let codesign = tool_program("codesign");
for bin in [&paths.temacs, &paths.bootstrap, &paths.final_bin] {
if options.dry_run || bin.exists() {
run_command(
options,
&options.repo_root,
&codesign,
&[
OsString::from("--force"),
OsString::from("--sign"),
OsString::from("-"),
bin.as_os_str().to_os_string(),
],
&[],
)?;
}
}
}There was a problem hiding this comment.
Pull request overview
This PR updates the xtask fresh-build pipeline on macOS so patched role binaries are re-signed after the pdump fingerprint is embedded, addressing the Apple Silicon SIGKILL failure caused by invalidated code signatures.
Changes:
- Add a macOS-only re-signing step after patching/copying the role executables.
- Re-sign
temacs,bootstrap, and the final binary with ad-hoccodesign. - Keep Linux behavior unchanged via
#[cfg(target_os = "macos")].
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for bin in [&paths.temacs, &paths.bootstrap, &paths.final_bin] { | ||
| if bin.exists() { | ||
| let status = std::process::Command::new("codesign") | ||
| .args(["--force", "--sign", "-", bin.to_str().unwrap()]) | ||
| .status()?; | ||
| if !status.success() { | ||
| return Err( | ||
| format!("codesign failed on {}", bin.display()).into() | ||
| ); |
| for bin in [&paths.temacs, &paths.bootstrap, &paths.final_bin] { | ||
| if bin.exists() { | ||
| let status = std::process::Command::new("codesign") | ||
| .args(["--force", "--sign", "-", bin.to_str().unwrap()]) |
Problem
On macOS (Apple Silicon),
cargo xtask fresh-build --releasefails with:Root Cause
The
patch_primary_executable_fingerprintstep modifies the executable binary in-place to embed the pdump fingerprint. On macOS, any in-place modification of a signed binary invalidates its code signature. When the kernel subsequently tries to execute the binary, it detects the invalid signature and sends SIGKILL.Fix
After
copy_executable_role_imagescopies the patchedfinal_binto thetemacsandbootstraproles, re-sign all three binaries using an ad-hoc signature (codesign --force --sign -). Ad-hoc signing requires no certificate and is sufficient for local execution.This fix is gated behind
#[cfg(target_os = macos)]and has no effect on Linux builds.