fix: macOS binary crashes on machines without Nix#118
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds macOS release packaging steps to set executable permissions on the copied binary and rewrite Nix-store dynamic library references by scanning with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The macOS binary is built with Nix, which hardcodes library paths like /nix/store/.../libiconv.2.dylib into the binary. This means the binary only works on machines that have Nix installed. Use install_name_tool to rewrite any /nix/store/... references to /usr/lib/..., where macOS ships these libraries by default. This matches what the Linux builds already do with patchelf --remove-rpath.
fa10165 to
2a6d8d4
Compare
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
When a user installs the CLI on macOS via the curl installer, it crashes immediately:
Cause
The macOS binary is built with Nix, which hardcodes library paths like
/nix/store/.../libiconv.2.dylibinto the binary. The binary then only works on machines that have Nix installed.The Linux builds already fix this — the GNU builds use
patchelf --remove-rpath, and the musl builds are fully static. The macOS build was missing an equivalent fix.Fix
After copying the binary, use
install_name_toolto rewrite any/nix/store/...library references to/usr/lib/.... macOS shipslibiconv(and other system libraries) at/usr/lib/on every Mac, so the binary will find them there.The fix handles all Nix store references generically, not just
libiconv, so it won't break if another Nix-provided library gets pulled in later.Summary by CodeRabbit