fix(npm): resolve global bin dir under pnpm and install arm64 build on Apple Silicon - #813
Open
danielpaulus wants to merge 1 commit into
Open
fix(npm): resolve global bin dir under pnpm and install arm64 build on Apple Silicon#813danielpaulus wants to merge 1 commit into
danielpaulus wants to merge 1 commit into
Conversation
…pple Silicon getInstallationPath() assumed npm_config_prefix is always set, but pnpm does not set it, so global installs via pnpm crashed with "TypeError [ERR_INVALID_ARG_TYPE] ... Received null" from mkdirp(null) (issue #659). Resolve the install dir with fallbacks: npm_config_prefix -> npm_config_global_prefix -> PNPM_HOME (pnpm's global bin dir, used as-is without a bin/ subdir) -> `npm prefix -g`, and fail with a clear, actionable error message if none resolve. The Windows rule is preserved: the npm prefix itself is the bin dir there, no bin/ subdir. Also drop the darwin/arm64 override that installed the amd64 build (and logged "using amd64 build on M1 mac"): the published package ships a go-ios-darwin-arm64_darwin_arm64 dist dir (universal binary), so Apple Silicon Macs now get it via the normal platform/arch mapping. Fix the dead "w32" platform typo (-> "win32") in the ia32 fallback and validate the architecture on every platform. Fixes #659 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk
Owner
Author
|
/test-devices |
Contributor
Contributor
|
❌ Real-device tests failed — see run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
pnpm i -g go-iosfails on macOS (reported on an M4 MacBook Air) with:The log also claimed
using amd64 build on M1 maceven though the machine is arm64.Root cause
getInstallationPath()innpm_publish/postinstall.jsonly knew one way to find the global bin dir: thenpm_config_prefixenv var. npm always sets it, pnpm does not — so under pnpmdirstayednullandmkdirp(null)threw an opaqueTypeError.go-ios-darwin-amd64_darwin_amd64dist dir. That predates the package shipping ago-ios-darwin-arm64_darwin_arm64dist dir (both contain the lipo universal mac binary today), so it "worked" but installed via the amd64 path with a misleading log line.Fix
getInstallationPath()now resolves the install dir with fallbacks, and fails loudly and helpfully instead of throwing aTypeError:npm_config_prefix(unchanged behavior for npm installs)npm_config_global_prefix(set by newer npm versions)PNPM_HOME— pnpm's global bin dir; used as-is (it is onPATHitself, nobin/subdir)npm prefix -goutput, accepted only if the command succeeds and returns an absolute path (a failure likesh: npm: command not foundstarts with/and must not be mistaken for a path)pnpm setup, then reinstallThe Windows rule is preserved for every npm-prefix-derived path (steps 1, 2, 4): on Windows the prefix itself is the bin dir, no
bin/subdir appended.Arch handling:
go-ios-darwin-arm64_darwin_arm64via the normal platform/arch mapping (and no more "using amd64 build on M1 mac" log).process.platform === "w32"typo →"win32"in the ia32 fallback.platform === "darwin"orarch === "arm64").Options considered
getInstallationPath()(chosen): npm env vars →PNPM_HOME→npm prefix -g→ clear error. Keeps npm behavior byte-identical (npm always setsnpm_config_prefix, so step 1 wins there), fixes pnpm without special-casing the package manager, and degrades to an actionable message instead of a crash.npm_config_user_agentand branch per package manager: more moving parts and still needs a fallback for other managers (yarn, bun); the env-var chain covers the same cases with less code.package.jsonbinfield: cleanest long-term, but a much bigger packaging change (per-platform shims or optionalDependencies split) — out of scope for a bug fix on the release path.For darwin/arm64: kept using the shipped
darwin-arm64dist dir rather than continuing to redirect to amd64 — the dist dir exists in the published package (release workflow copies the universal binary into both), so the normal mapping is correct and the special case was only misleading.Test plan
node --check npm_publish/postinstall.jspasses;go build ./... && go test ./...untouched and green.dist/tree on darwin/arm64:npm_config_prefixunset,PNPM_HOMEset): binary lands directly in$PNPM_HOME(nobin/subdir), and the darwin-arm64 dist binary is selected.npm_config_prefixset): binary lands in<prefix>/binas before.npm_config_global_prefix-only andnpm prefix -g-only scenarios: both resolve correctly.npmnot onPATH): clear error message, exit code 1 (previously an opaqueTypeError).go-ios-canaryand verifies install + run on Windows/Linux/macOS): https://github.com/danielpaulus/go-ios/actions/runs/31018210948 — success (publishedgo-ios-canary@0.0.5with this postinstall.js; install verified on macOS, Linux and Windows). Note: the first attempt ofverify_install (windows-latest)hit npm registry propagation lag (ETARGETfor the version macOS/Linux had already installed from the same run); a rerun of that job passed with no code change.Related
Does not resolve #227 — that failure is a missing 32-bit
linux/armbinary indist/(only arm64/amd64 are shipped), unrelated to the pnpm prefix crash fixed here.Fixes #659
🤖 Generated with Claude Code
https://claude.ai/code/session_01J8eMENxJ1nec9CeHp4tjWk