-
Notifications
You must be signed in to change notification settings - Fork 4
Ferric x86 iOS simulator #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for x86_64 iOS simulator builds in the Ferric package, enabling Rust Node-API modules to run on Intel-based iOS simulators. This complements the existing ARM64 simulator support.
Key Changes:
- Enabled
x86_64-apple-iosas a valid Rust target for iOS simulator builds - Updated universal library creation to combine both ARM64 and x86_64 simulator architectures into a single universal binary
- Added validation assertions and improved error messages in the library combination logic
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/ferric/src/targets.ts |
Uncommented and documented x86_64-apple-ios as a supported iOS simulator target |
packages/ferric/src/cargo.ts |
Updated XCFramework mapping to create universal simulator binaries combining ARM64 and x86_64 architectures |
packages/ferric/src/build.ts |
Refactored library combination logic to handle iOS simulator libraries separately and create universal binaries for both macOS and iOS simulator |
packages/ferric/src/program.ts |
Made the build command the default command for the ferric CLI |
packages/host/src/node/prebuilds/apple.ts |
Added input validation and improved error messages in universal library creation function |
packages/ferric/src/cargo.ts
Outdated
| "x86_64-apple-ios": "ios-arm64_x86_64-simulator", // Universal | ||
|
|
||
| // "aarch64-apple-ios-macabi": "", // Catalyst | ||
| // "x86_64-apple-ios": "ios-x86_64", |
Copilot
AI
Oct 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commented-out line is now obsolete since x86_64-apple-ios has been added to the active configuration on line 27. The outdated comment should be removed to avoid confusion.
| // "x86_64-apple-ios": "ios-x86_64", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree - and expect this will go away in a future PR 👍
| if (libraryPaths.length === 0) { | ||
| return []; |
Copilot
AI
Oct 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty array check is unnecessary because the function is called with pre-filtered groups (darwinLibraries and iosSimulatorLibraries). If either group is empty, it should not be included in the input array to avoid processing empty groups. Consider filtering out empty groups before calling this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we're also exporting this function, I don't want to make too many assumptions on how it's called. Also, it seems the call pattern is much simpler here than if we had to not pass the array argument (darwinLibraries or iosSimulatorLibraries) if it's empty.
7d36b10 to
fad29d9
Compare
This needs to be rebased on main to pick up #288 (once merged).