Skip to content

Conversation

@kraenhansen
Copy link
Collaborator

Merging this PR will:

  • Refactor Ferric's logic for determining the per-target Apple Xcframework slice, to prioritize a universal library if available and fallback to an arch specific slice.

This will allow a simplification on CI where we don't have to build weak-node-api for architectures we're not building Ferric targets for anyway.

@kraenhansen kraenhansen self-assigned this Nov 2, 2025
@kraenhansen kraenhansen added Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Ferric 🦀 labels Nov 2, 2025
@cursor
Copy link

cursor bot commented Nov 2, 2025

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.

@kraenhansen kraenhansen requested a review from Copilot November 2, 2025 15:36
Copy link
Contributor

Copilot AI left a 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 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

Comment on lines +176 to +179
const result = APPLE_XCFRAMEWORK_SLICES_PER_TARGET[target].find((slice) => {
const candidatePath = path.join(xcframeworkPath, slice);
return fs.existsSync(candidatePath);
});
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
});
assert(
result,
`No matching slice found in weak-node-api.xcframework for target ${target}`,
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(', ')}

Suggested change
`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(', ')}`,

Copilot uses AI. Check for mistakes.
@kraenhansen kraenhansen merged commit 1d636d6 into main Nov 2, 2025
7 checks passed
@kraenhansen kraenhansen deleted the kh/ferric-fallback-on-non-universal-libraries branch November 2, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Ferric 🦀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants