-
Notifications
You must be signed in to change notification settings - Fork 7
Simplify source code while visiting few cli/ sources
#125
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,73 +64,73 @@ export async function linkXcframework({ | |
| await fs.promises.rm(outputPath, { recursive: true, force: true }); | ||
| await fs.promises.cp(modulePath, tempPath, { recursive: true }); | ||
|
|
||
| const frameworkPaths = await Promise.all( | ||
| fs | ||
| .readdirSync(tempPath, { | ||
| withFileTypes: true, | ||
| }) | ||
| // Following extracted function mimics `glob("*/*.framework/")` | ||
| function globFrameworkDirs<T>( | ||
| startPath: string, | ||
| fn: (parentPath: string, name: string) => Promise<T> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we pick a more semantic name for this argument? It's like an action performed on every triplet + framework. |
||
| ) { | ||
| return fs | ||
| .readdirSync(startPath, { withFileTypes: true }) | ||
| .filter((tripletEntry) => tripletEntry.isDirectory()) | ||
| .flatMap((tripletEntry) => { | ||
| const tripletPath = path.join(tempPath, tripletEntry.name); | ||
| const tripletPath = path.join(startPath, tripletEntry.name); | ||
| return fs | ||
| .readdirSync(tripletPath, { | ||
| withFileTypes: true, | ||
| }) | ||
| .readdirSync(tripletPath, { withFileTypes: true }) | ||
| .filter( | ||
| (frameworkEntry) => | ||
| frameworkEntry.isDirectory() && | ||
| path.extname(frameworkEntry.name) === ".framework" | ||
| ) | ||
| .flatMap(async (frameworkEntry) => { | ||
| const frameworkPath = path.join(tripletPath, frameworkEntry.name); | ||
| const oldLibraryName = path.basename( | ||
| frameworkEntry.name, | ||
| ".framework" | ||
| ); | ||
| const oldLibraryPath = path.join(frameworkPath, oldLibraryName); | ||
| const newFrameworkPath = path.join( | ||
| tripletPath, | ||
| `${newLibraryName}.framework` | ||
| ); | ||
| const newLibraryPath = path.join( | ||
| newFrameworkPath, | ||
| newLibraryName | ||
| ); | ||
| assert( | ||
| fs.existsSync(oldLibraryPath), | ||
| `Expected a library at '${oldLibraryPath}'` | ||
| ); | ||
| // Rename the library | ||
| await fs.promises.rename( | ||
| oldLibraryPath, | ||
| // Cannot use newLibraryPath here, because the framework isn't renamed yet | ||
| path.join(frameworkPath, newLibraryName) | ||
| ); | ||
| // Rename the framework | ||
| await fs.promises.rename(frameworkPath, newFrameworkPath); | ||
| // Expect the library in the new location | ||
| assert(fs.existsSync(newLibraryPath)); | ||
| // Update the binary | ||
| await spawn( | ||
| "install_name_tool", | ||
| [ | ||
| "-id", | ||
| `@rpath/${newLibraryName}.framework/${newLibraryName}`, | ||
| newLibraryPath, | ||
| ], | ||
| { | ||
| outputMode: "buffered", | ||
| } | ||
| ); | ||
| // Update the Info.plist file for the framework | ||
| await updateInfoPlist({ | ||
| filePath: path.join(newFrameworkPath, "Info.plist"), | ||
| oldLibraryName, | ||
| newLibraryName, | ||
| }); | ||
| return newFrameworkPath; | ||
| }); | ||
| }) | ||
| .flatMap(async (frameworkEntry) => | ||
| await fn(tripletPath, frameworkEntry.name) | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| const frameworkPaths = await Promise.all( | ||
| globFrameworkDirs(tempPath, async (tripletPath, frameworkEntryName) => { | ||
| const frameworkPath = path.join(tripletPath, frameworkEntryName); | ||
| const oldLibraryName = path.basename(frameworkEntryName, ".framework"); | ||
| const oldLibraryPath = path.join(frameworkPath, oldLibraryName); | ||
| const newFrameworkPath = path.join( | ||
| tripletPath, | ||
| `${newLibraryName}.framework` | ||
| ); | ||
| const newLibraryPath = path.join(newFrameworkPath, newLibraryName); | ||
| assert( | ||
| fs.existsSync(oldLibraryPath), | ||
| `Expected a library at '${oldLibraryPath}'` | ||
| ); | ||
| // Rename the library | ||
| await fs.promises.rename( | ||
| oldLibraryPath, | ||
| // Cannot use newLibraryPath here, because the framework isn't renamed yet | ||
| path.join(frameworkPath, newLibraryName) | ||
| ); | ||
| // Rename the framework | ||
| await fs.promises.rename(frameworkPath, newFrameworkPath); | ||
| // Expect the library in the new location | ||
| assert(fs.existsSync(newLibraryPath)); | ||
| // Update the binary | ||
| await spawn( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: imho this should be extracted to a separate function |
||
| "install_name_tool", | ||
| [ | ||
| "-id", | ||
| `@rpath/${newLibraryName}.framework/${newLibraryName}`, | ||
| newLibraryPath, | ||
| ], | ||
| { | ||
| outputMode: "buffered", | ||
| } | ||
| ); | ||
| // Update the Info.plist file for the framework | ||
| await updateInfoPlist({ | ||
| filePath: path.join(newFrameworkPath, "Info.plist"), | ||
| oldLibraryName, | ||
| newLibraryName, | ||
| }); | ||
| return newFrameworkPath; | ||
| }) | ||
| ); | ||
|
|
||
| // Create a new xcframework from the renamed frameworks | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,12 +68,10 @@ export async function linkModules({ | |
| }); | ||
|
|
||
| // Find absolute paths to xcframeworks | ||
| const absoluteModulePaths = Object.entries(dependenciesByName).flatMap( | ||
| ([, dependency]) => { | ||
| return dependency.modulePaths.map((modulePath) => | ||
| path.join(dependency.path, modulePath) | ||
| ); | ||
| } | ||
| const absoluteModulePaths = Object.values(dependenciesByName).flatMap( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💙 |
||
| (dependency) => dependency.modulePaths.map( | ||
| (modulePath) => path.join(dependency.path, modulePath) | ||
| ) | ||
| ); | ||
|
|
||
| if (hasDuplicateLibraryNames(absoluteModulePaths, naming)) { | ||
|
|
@@ -82,28 +80,25 @@ export async function linkModules({ | |
| } | ||
|
|
||
| return Promise.all( | ||
| Object.entries(dependenciesByName).flatMap(([, dependency]) => { | ||
| return dependency.modulePaths.map(async (modulePath) => { | ||
| const originalPath = path.join(dependency.path, modulePath); | ||
| try { | ||
| return await linker({ | ||
| modulePath: originalPath, | ||
| incremental, | ||
| naming, | ||
| platform, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof SpawnFailure) { | ||
| return { | ||
| originalPath, | ||
| skipped: false, | ||
| failure: error, | ||
| }; | ||
| } else { | ||
| throw error; | ||
| } | ||
| absoluteModulePaths.map(async (originalPath) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💙 |
||
| try { | ||
| return await linker({ | ||
| modulePath: originalPath, | ||
| incremental, | ||
| naming, | ||
| platform, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof SpawnFailure) { | ||
| return { | ||
| originalPath, | ||
| skipped: false, | ||
| failure: error, | ||
| }; | ||
| } else { | ||
| throw error; | ||
| } | ||
| }); | ||
| } | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
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.
Nit: this can be moved to path-utils
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.
Since this also applies a side-effect, it feels to me that it's doing a bit more than simply globbing?
Perhaps