Conversation
|
| try { | ||
| return new Set( | ||
| cp | ||
| .execFileSync("rustup", ["target", "list", "--installed"], { |
There was a problem hiding this comment.
Maybe we can grab all the targets and filter out installed ourselves? This might be handy if you would like to separate supported targets from available/installed. I've managed to do something like this here.
There was a problem hiding this comment.
We could base the list of available targets on a call into "rustup" yeah. I would like that once I figure out how to install more of them (so I can test 😬).
| "arm64-apple-tvos-sim": "arm64", | ||
| "arm64-apple-visionos": "arm64", | ||
| "arm64-apple-visionos-sim": "arm64", | ||
| } satisfies Record<AppleTriplet, AppleArchitecture>; |
There was a problem hiding this comment.
What do you think about detecting installed Xcode targets and then parsing the CPU arch from that? I'm afraid target availability depends on the SDK version. Maybe we can use some code from here?
There was a problem hiding this comment.
That could probably work 👍 And that would make it easier to adopt new SDKs in future versions of XCode, right? I mean - it wouldn't require a change to our tool.
One thing that I'd be mindful of is that we don't want building for all apple triplets (using --apple) becoming host machine dependent, as that could lead to xcframeworks with missing framework+dylib for an SDK they had not installed when building. I'd rather error in that case, so we do need to enumerate a list of Apple triplets for that.
| '<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">', | ||
| '<plist version="1.0">', | ||
| " <dict>", | ||
| ...Object.entries(values).flatMap(([key, value]) => [ |
There was a problem hiding this comment.
Nit: If you wouldn't mind, I'd be happy to contribute my version (from here) as it serializes arrays, objects and strings + includes a tailing new-line. I've also noticed that Apple favors tab instead of spaces, but that's a minor thing.
There was a problem hiding this comment.
This is mostly code I'm moving, I (or you) can follow up with a PR changing that 👍 There is also plist 🙂
| LINES.map((line, lineNumber, lines) => { | ||
| const ratio = lineNumber / lines.length; | ||
| return chalk.rgb(Math.round(250 - 100 * ratio), 0, 0)(line); | ||
| }).join("\n") |
There was a problem hiding this comment.
I probably spent half of my time building this tool, editing this file 😆
packages/ferric/src/build.ts
Outdated
| ); | ||
| } | ||
|
|
||
| // TODO: Handle Apple and constructing an XCFramework |
There was a problem hiding this comment.
Nit: Isn't it done above already? 😅
There was a problem hiding this comment.
Hah yes - that's a leftover 😅
| ): Promise<string[]> { | ||
| const result = []; | ||
| const darwinLibraries = []; | ||
| for (const [target, libraryPath] of libraries) { |
There was a problem hiding this comment.
I don't want to be pushy, maybe we can reuse the groupByPlatformArch<>() function? It was handy to collect libs for Xcframework :) If not, then ignore this comment :)
There was a problem hiding this comment.
I'd happy to do that as a follow-up 👍
There was a problem hiding this comment.
Just looked at the groupByPlatformArch function again. It feels a bit overkill to what this needs (simply extracting the "darwin" libraries from what is known to be a list of target names) - I think it would also take some refactoring to make the types match up.
I'd be happy to merge a PR if you can fit it and even better if we could make use of it somewhere else as well, where the full feature of the function gets used 👍
kraenhansen
force-pushed
the
kh/ferric
branch
3 times, most recently
from
bf48ac8 to
34c26c2
Compare
2 participants
Merging this PR will:
ferric-modulespackage with aferricbin, which calls intocargoto build dynamic libraries and package them into the prebuild formats expected byreact-native-node-api-modules.ferric-building.mov
Marking this as draft to get early feedback and because there's a few outstanding todos that need to be resolved before merging.