fix(nix): filter optional dependencies by target platform #8033
+131
−14
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical build failure on macOS/Linux where Nix was attempting to install Windows-specific optional dependencies, causing "Operation not permitted" errors. The fix introduces platform-specific filtering of optional dependencies during bun install by mapping Nix system identifiers to Bun's --cpu and --os flag values.
Changes:
- Added system parameter to
node-modules.nixwith mappings for CPU architectures (arm64, x64, ia32, arm) and operating systems (darwin, linux) - Replaced wildcard values (
"*") inbun installflags with platform-specific values to filter optional dependencies - Implemented fallback to
"*"for unsupported platforms to maintain compatibility
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| nix/node-modules.nix | Added system parameter and mapping logic to convert Nix system identifiers to Bun CPU/OS values, replacing wildcard flags with platform-specific values |
| flake.nix | Passed system parameter to node-modules.nix via inherit system in the callPackage invocation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes anomalyco#8029 by installing only platform-specific optional dependencies instead of all platforms, preventing 'Operation not permitted' errors on Windows DLLs when building on macOS/Linux. Changes: - Add system parameter to node-modules.nix - Map Nix systems to Bun --cpu/--os flags - Replace --cpu="*" --os="*" with platform-specific values - Fallback to "*" for unknown systems to preserve compatibility This reduces package size from ~4GB to a few hundred MB and eliminates cross-platform binary file access issues in Nix build sandboxes.
Contributor
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
jerome-benoit
force-pushed
the
fix/nix-platform-specific-deps
branch
from
8698663 to
f2e8138
Compare
Since we now install platform-specific dependencies only, node_modules content differs per system. Update hash structure to support per-system hashes while maintaining backward compatibility with single-hash format. Hashes obtained from CI build errors: - x86_64-linux: sha256-6GotwCHx/cw+6ShOcGz29DjuMUBGWD00CJXZHQzW9D8= - aarch64-darwin: sha256-+T96JNNTCcbP4Ta1ezI1obI+YQYtfqMcp+G/eaUwTFQ=
Parse bunTarget values to extract OS and CPU instead of maintaining separate cpuMap and osMap in node-modules.nix. This ensures single source of truth and reduces maintenance burden. - Added parseBunTarget helper function in flake.nix - Removed duplicate cpuMap and osMap from node-modules.nix - Pass bunCpu and bunOs as parameters to mkNodeModules
Add example to show input/output format more clearly.
Fallback to --cpu="*" --os="*" for systems not in bunTarget map to prevent build failures on unsupported architectures.
Fallback to --cpu="*" --os="*" produces different node_modules content that won't match per-system hashes. Better to fail explicitly with clear error than silently use wrong hash.
Update write_node_modules_hash to write per-system format: - Detect if nodeModules is object (per-system) or string (legacy) - Convert legacy format to per-system on first run - Update specific system hash while preserving others
Split workflow into two jobs: - update-linux: runs first on x86_64-linux - update-macos: runs after on aarch64-darwin Each job updates hash for its platform and commits sequentially.
Collaborator
|
Ping me when ready for review :) |
Remove defaultNodeModules fallback - missing system hashes should fail with clear error instead of silently using wrong hash.
Contributor
Author
The CI is now working and I do not think I've overlooked any dependent code, so I think the PR is ready for review now ;) |
Collaborator
|
/review |
Contributor
|
lgtm |
Contributor
Author
|
Any CI failure related to hashes or conflict in the hashes file are harmless, the GH workflow will regenerate it properly once merged. |
- x86_64-linux: sha256-8nur5CuUCSV/SzD16hNXVoIlKsiPBXDzCnoITK0IhC4= - aarch64-darwin: sha256-vD1g9dviI2nMBTTPwI87sK01hSZ+cdnmb1V72AdJYq4=
|
Hi @jerome-benoit , not sure if I am doing something wrong but now i am getting this error, when i update nix (it was working yesterday): |
Contributor
Author
|
Pushed fix for hashes.json initialization bug |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Fixes #8029 and #4853 by installing only platform-specific optional dependencies instead of all platforms.
Changes
--cpu="*"and--os="*"with platform-specific valuesImplementation
parseBunTargethelper to extract OS/CPU from bunTargetnodeModulesHashForto support both{system: hash}and legacy single hashnix/hashes.jsonwith per-system hashes (x86_64-linux, aarch64-darwin)update-hashes.shscript to write per-system hash formatupdate-nix-hashesworkflow into sequential Linux + macOS jobsImpact
Supported Systems
Currently with hashes:
x86_64-linux✅aarch64-darwin✅Defined but pending CI hash generation:
x86_64-darwin(will be generated on next dependency change)aarch64-linux(will be generated on next dependency change)Missing hashes will fail with clear Nix error instead of using wrong hash.