fix(exe): drop darwin-x64 artifact (upstream Node SEA bug)#11455
Conversation
Node.js SEA injection produces a binary that segfaults at startup on Intel Macs. The bug is in LIEF's Mach-O surgery during --build-sea, not in signing — the canonical `node --build-sea` + `codesign --sign -` flow crashes too (nodejs/node#62893). The Node.js team has opted not to fix it (nodejs/node#60250) on the grounds that x64 macOS is being phased out; long-running test failures are tracked in nodejs/node#59553. Changes: - Remove `@pnpm/macos-x64` from `@pnpm/exe`'s optionalDependencies and from the meta-updater enforcement list, so npm no longer tries to install the broken platform package on Intel Mac. - Remove `darwin-x64` from `pnpm.app.targets` and from `narrowTargets` in build-artifacts.ts so pack-app skips the build entirely. - Remove the `darwin-x64` tarball from copy-artifacts.ts so `pnpm-darwin-x64.tar.gz` is no longer produced for the GitHub release. - Delete the `pnpm/artifacts/darwin-x64` workspace package (was the source of `@pnpm/macos-x64`). - `setup.js` now wraps the `import.meta.resolve(...)` for the platform package in try/catch and, on Intel Mac, prints a clear pointer to this issue plus workarounds (npm install -g pnpm, or pnpm 10.x) before exiting non-zero. Other unsupported platforms get a generic message in the same shape. Closes #11423.
📝 WalkthroughWalkthroughThis PR removes the darwin-x64 ( ChangesDarwin-x64 artifact removal and installer behavior
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes the darwin-x64 (@pnpm/macos-x64 / pnpm-darwin-x64.tar.gz) standalone executable artifact from @pnpm/exe distribution, due to an upstream Node.js SEA bug causing consistent crashes on Intel macOS, and adds a clearer install-time failure message for Intel Macs.
Changes:
- Remove
@pnpm/macos-x64optional dependency anddarwin-x64build target from@pnpm/exe. - Update build/release artifact generation scripts to stop producing
pnpm-darwin-x64.tar.gzand to align local/CI target matrices. - Improve
@pnpm/exepreinstall (setup.js) behavior when the platform package is missing by printing actionable guidance and exiting non-zero.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pnpm/artifacts/exe/setup.js |
Wrap platform package resolution in try/catch and emit a targeted Intel macOS error message when the platform package isn’t present. |
pnpm/artifacts/exe/scripts/build-artifacts.ts |
Remove darwin-x64 from Intel-Mac “narrow” targets and update rationale comments. |
pnpm/artifacts/exe/package.json |
Drop @pnpm/macos-x64 from optionalDependencies and remove darwin-x64 from pnpm.app.targets. |
__utils__/scripts/src/copy-artifacts.ts |
Stop producing the pnpm-darwin-x64 release tarball and document why. |
.meta-updater/src/index.ts |
Prevent meta-updater from re-introducing @pnpm/macos-x64 into enforced optional deps. |
pnpm/artifacts/darwin-x64/* |
Delete the @pnpm/macos-x64 workspace package sources. |
pnpm-lock.yaml |
Regenerate lockfile to remove the deleted workspace importer and links. |
.changeset/drop-darwin-x64-broken-sea.md |
Add changeset documenting the removal and user guidance. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on #11455: - Catch only ERR_MODULE_NOT_FOUND from import.meta.resolve and rethrow anything else, so unrelated resolver failures surface instead of being reported as "unsupported host." - When running inside the pnpm workspace itself (detected via an ancestor pnpm-workspace.yaml), skip the hard error and exit 0. Without this, a contributor on Intel macOS — where the platform package was removed in the parent commit — could no longer `pnpm install` the repo to work on unrelated parts of pnpm. - Reword the awkward "doesn't ship a binary for is darwin-x64" comment. Verified isInPnpmWorkspace returns true from inside the workspace and false from /tmp / a typical global install path. Verified import.meta.resolve throws ERR_MODULE_NOT_FOUND on a missing package.
Replace the ancestor `pnpm-workspace.yaml` walk-up with a precise suffix check on `import.meta.dirname` (must end in `pnpm/artifacts/exe`, the workspace path of @pnpm/exe). The walk-up could false-positive: a user globally installing @pnpm/exe while their cwd or install path happened to live under any unrelated pnpm workspace tree would be misidentified as a workspace contributor and silently skipped, leaving them with no working pnpm. The suffix-on-the-script's-own-directory check matches only when this script is literally the workspace's @pnpm/exe. Verified with sample paths: workspace clones (any disk location) match, global/local installs and contrived "artifacts/exe" paths without the "pnpm/" segment do not.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…h tests Address Copilot review on #11455 (round 2): - build-artifacts.ts: the comment said "On Intel Mac we only build the two baseline targets" but `buildFullMatrix` is `linux || isM1Mac`, so any non-Linux non-Apple-Silicon host (Windows, Intel Mac, etc.) uses narrowTargets. Reword to match the actual condition and keep the darwin-x64-absent rationale. - setup.test.ts: add two integration tests that stand up a minimal sandbox (setup.js + platform-pkg-name.js + a node_modules with only detect-libc symlinked from this repo) at a tempdir whose path either ends in `pnpm/artifacts/exe` (workspace shape) or doesn't (global- install shape). import.meta.resolve for the host's platform package fails in both because no @pnpm/<platform> is installed. Asserts: 1. workspace-shaped path → exit 0, no stderr 2. non-workspace path → exit 1, stderr names the missing pkg Verified the workspace-shape test fails when the path-suffix in setup.js is mutated (i.e. the test isn't a no-op). Skipped on Windows because fs.symlinkSync there needs elevated privileges and the path-suffix logic is platform-independent.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pnpm/artifacts/exe/test/setup.test.ts (1)
99-116: ⚡ Quick winMove
buildFailurePathSandboxbelow its call sites to match repo function-order convention.This helper is declared before usage. In this repo, function declarations should be placed after use (relying on hoisting).
As per coding guidelines,
**/*.{ts,tsx,js,jsx}: "declare functions after they are used (relying on hoisting)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpm/artifacts/exe/test/setup.test.ts` around lines 99 - 116, The helper function buildFailurePathSandbox is declared before it's used; per repo convention functions should be declared after their call sites (relying on hoisting). Move the entire buildFailurePathSandbox declaration so it appears after all places that call it (search for buildFailurePathSandbox usage in this test file) and ensure any required helper variables (exeDir, fs, path, os) remain in scope; no other code changes are needed besides reordering the declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pnpm/artifacts/exe/test/setup.test.ts`:
- Around line 99-116: The helper function buildFailurePathSandbox is declared
before it's used; per repo convention functions should be declared after their
call sites (relying on hoisting). Move the entire buildFailurePathSandbox
declaration so it appears after all places that call it (search for
buildFailurePathSandbox usage in this test file) and ensure any required helper
variables (exeDir, fs, path, os) remain in scope; no other code changes are
needed besides reordering the declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6587b537-8f39-4a61-b7e7-da6d2e36a640
📒 Files selected for processing (2)
pnpm/artifacts/exe/scripts/build-artifacts.tspnpm/artifacts/exe/test/setup.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- pnpm/artifacts/exe/scripts/build-artifacts.ts
## Summary Closes #11423. `pnpm-darwin-x64.tar.gz` and `@pnpm/macos-x64` are removed because the binaries they contain segfault at startup on Intel Macs and the underlying bug is upstream and unfixed. ## Why this isn't a fix in code The crash happens in `__cxx_global_var_init` with `EXC_BAD_ACCESS (code=1, address=0x3)` — the unprocessed-chain-entry tag — in dyld's chained-fixup processing. PR #11415's hypothesis was that `ldid`'s page hashes were the cause, but switching to native `codesign` in #11415 didn't fix it: the upstream minimal repro in [nodejs/node#62893](nodejs/node#62893) is `node --build-sea` + `codesign --sign -` + run, with no pnpm and no `ldid`, and it still crashes. The corruption is in LIEF's Mach-O surgery during `--build-sea` for x64 — chained-fixup chain entries get rewritten incorrectly when the SEA segment is inserted, and re-signing produces a valid signature over the broken bytes. The Node.js team is not going to fix this: - [nodejs/node#60250](nodejs/node#60250) (merged) — *"It's unlikely that anyone would invest in fixing them on x64 macOS in the near future, now that x64 macOS is being phased out."* They skipped the SEA tests on x64 macOS rather than chase the bug. - [nodejs/node#59553](nodejs/node#59553) (open) — long-running test failures on macOS x64 with the same root cause (sometimes surfacing as `unsupported thread-local, larger than 4GB`). `@yao-pkg/pkg` works around it by appending the JS payload to the file tail and using a custom-patched Node binary that reads from the tail at startup; this avoids Mach-O surgery entirely. We can't reuse pack-app for that because vanilla Node from nodejs.org doesn't read tail-appended payloads — only pkg-fetch's patched binaries do — so adopting that path would mean re-implementing pkg-fetch for one target. For now we're dropping the broken artifact rather than introducing a second build mechanism. ## Changes - **`pnpm/artifacts/exe/package.json`** — remove `@pnpm/macos-x64` from `optionalDependencies`; remove `darwin-x64` from `pnpm.app.targets`. - **`.meta-updater/src/index.ts`** — remove `@pnpm/macos-x64` from the enforced `optionalDependencies` list (otherwise `meta-updater` would put it back). - **`pnpm/artifacts/exe/scripts/build-artifacts.ts`** — drop `darwin-x64` from `narrowTargets` so dev-local builds match the published matrix; comment explains why. - **`__utils__/scripts/src/copy-artifacts.ts`** — stop creating `pnpm-darwin-x64.tar.gz` so the GitHub release page no longer ships it. - **`pnpm/artifacts/darwin-x64/`** — deleted (was the workspace source for `@pnpm/macos-x64`). - **`pnpm/artifacts/exe/setup.js`** — wraps the `import.meta.resolve('${pkgName}/package.json')` lookup in `try`/`catch`. On Intel Mac specifically, prints a clear message pointing at this issue, the upstream Node.js issue, and the two workarounds (`npm install -g pnpm` to use the system Node.js, or stay on pnpm 10.x). Other unsupported hosts get a generic message in the same shape. Exits non-zero so the install fails loudly instead of silently leaving a broken `pnpm`. - **`pnpm-lock.yaml`** — regenerated. - **`.changeset/drop-darwin-x64-broken-sea.md`** — patch bumps for `@pnpm/exe` and `pnpm` with user-facing explanation and pointers. Docs side already lists this limitation under `pack-app` Known limitations: pnpm/pnpm.io@36d962f6 / pnpm/pnpm.io@91f45632. ## Compat - Intel Mac users on existing `@pnpm/exe` (≤ 11.0.4) keep working with the (broken) old binary they already have. - `pnpm self-update` from an Intel Mac on an older `@pnpm/exe` will hit the new `setup.js` error path with a clear pointer to the workarounds. - New Intel Mac installs via `npm install -g @pnpm/exe` will fail loudly with the same pointer. - Install via `npm install -g pnpm` (the JS-only package, uses system Node) is unaffected and remains the recommended path. - The `install.sh` from `get.pnpm.io` will fail with a 404 on the missing `pnpm-darwin-x64.tar.gz`. That's a separate repo and a follow-up — happy to do that as a second PR.
|
@zkochan since nodejs/node#62893 was closed, does that mean intel support on macs will be coming back to pnpm? |
Summary
Closes #11423.
pnpm-darwin-x64.tar.gzand@pnpm/macos-x64are removed because the binaries they contain segfault at startup on Intel Macs and the underlying bug is upstream and unfixed.Why this isn't a fix in code
The crash happens in
__cxx_global_var_initwithEXC_BAD_ACCESS (code=1, address=0x3)— the unprocessed-chain-entry tag — in dyld's chained-fixup processing. PR #11415's hypothesis was thatldid's page hashes were the cause, but switching to nativecodesignin #11415 didn't fix it: the upstream minimal repro in nodejs/node#62893 isnode --build-sea+codesign --sign -+ run, with no pnpm and noldid, and it still crashes. The corruption is in LIEF's Mach-O surgery during--build-seafor x64 — chained-fixup chain entries get rewritten incorrectly when the SEA segment is inserted, and re-signing produces a valid signature over the broken bytes.The Node.js team is not going to fix this:
unsupported thread-local, larger than 4GB).@yao-pkg/pkgworks around it by appending the JS payload to the file tail and using a custom-patched Node binary that reads from the tail at startup; this avoids Mach-O surgery entirely. We can't reuse pack-app for that because vanilla Node from nodejs.org doesn't read tail-appended payloads — only pkg-fetch's patched binaries do — so adopting that path would mean re-implementing pkg-fetch for one target. For now we're dropping the broken artifact rather than introducing a second build mechanism.Changes
pnpm/artifacts/exe/package.json— remove@pnpm/macos-x64fromoptionalDependencies; removedarwin-x64frompnpm.app.targets..meta-updater/src/index.ts— remove@pnpm/macos-x64from the enforcedoptionalDependencieslist (otherwisemeta-updaterwould put it back).pnpm/artifacts/exe/scripts/build-artifacts.ts— dropdarwin-x64fromnarrowTargetsso dev-local builds match the published matrix; comment explains why.__utils__/scripts/src/copy-artifacts.ts— stop creatingpnpm-darwin-x64.tar.gzso the GitHub release page no longer ships it.pnpm/artifacts/darwin-x64/— deleted (was the workspace source for@pnpm/macos-x64).pnpm/artifacts/exe/setup.js— wraps theimport.meta.resolve('${pkgName}/package.json')lookup intry/catch. On Intel Mac specifically, prints a clear message pointing at this issue, the upstream Node.js issue, and the two workarounds (npm install -g pnpmto use the system Node.js, or stay on pnpm 10.x). Other unsupported hosts get a generic message in the same shape. Exits non-zero so the install fails loudly instead of silently leaving a brokenpnpm.pnpm-lock.yaml— regenerated..changeset/drop-darwin-x64-broken-sea.md— patch bumps for@pnpm/exeandpnpmwith user-facing explanation and pointers.Docs side already lists this limitation under
pack-appKnown limitations: pnpm/pnpm.io@36d962f6 / pnpm/pnpm.io@91f45632.Compat
@pnpm/exe(≤ 11.0.4) keep working with the (broken) old binary they already have.pnpm self-updatefrom an Intel Mac on an older@pnpm/exewill hit the newsetup.jserror path with a clear pointer to the workarounds.npm install -g @pnpm/exewill fail loudly with the same pointer.npm install -g pnpm(the JS-only package, uses system Node) is unaffected and remains the recommended path.install.shfromget.pnpm.iowill fail with a 404 on the missingpnpm-darwin-x64.tar.gz. That's a separate repo and a follow-up — happy to do that as a second PR.Test plan
pnpm installregenerates the lockfile cleanly; only the@pnpm/exeimporter's@pnpm/macos-x64link is removed.pnpm run spellcheckpasses.pnpm run lint:meta(which runsmeta-updater --testto check workspace manifest sync) passes.setup.jssyntax-checks (node --check); preinstall on Apple Silicon (darwin-arm64) still resolves the platform package and links the binary, so the new error path is correctly scoped to missing-platform-package only.npm install -g @pnpm/exefails with the new error message (no Intel hardware available locally — would appreciate a maintainer with one running this).pnpm-darwin-x64.tar.gz).Follow-ups (separate PRs)
get.pnpm.io/install.shto detectdarwin-x64and print a clear "not supported" message before attempting download.@pnpm/macos-x64placeholder so old@pnpm/exeinstalls that resolvelatestget a clear runtime message rather than the cached broken binary.References
Summary by CodeRabbit