Fallback on non-universal libraries when getting path for weak-node-api framework#303
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 2. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Apple XCFramework slice selection logic in Ferric to support fallback behavior when universal libraries are unavailable. Instead of hard-coding a single slice per target, it now prioritizes universal libraries but can fall back to architecture-specific slices.
Key changes:
- Replace single slice mapping with prioritized slice arrays for each Apple target
- Implement runtime slice detection with fallback logic in
getWeakNodeApiFrameworkPath
| const result = APPLE_XCFRAMEWORK_SLICES_PER_TARGET[target].find((slice) => { | ||
| const candidatePath = path.join(xcframeworkPath, slice); | ||
| return fs.existsSync(candidatePath); | ||
| }); |
There was a problem hiding this comment.
The fs.existsSync call inside the find loop could be inefficient if there are many slices to check. Consider using fs.promises.access with proper error handling for better performance, or at minimum add early termination if the first slice is found.
| }); | ||
| assert( | ||
| result, | ||
| `No matching slice found in weak-node-api.xcframework for target ${target}`, |
There was a problem hiding this comment.
The error message could be more helpful by listing the searched slice names. Consider: No matching slice found in weak-node-api.xcframework for target ${target}. Searched slices: ${APPLE_XCFRAMEWORK_SLICES_PER_TARGET[target].join(', ')}
| `No matching slice found in weak-node-api.xcframework for target ${target}`, | |
| `No matching slice found in weak-node-api.xcframework for target ${target}. Searched slices: ${APPLE_XCFRAMEWORK_SLICES_PER_TARGET[target].join(', ')}`, |
Merging this PR will:
This will allow a simplification on CI where we don't have to build
weak-node-apifor architectures we're not building Ferric targets for anyway.