fix(cli): generate binaryTarget entries for custom xcframeworks in Package.swift#8445
fix(cli): generate binaryTarget entries for custom xcframeworks in Package.swift#8445OS-ruimoreiramendes wants to merge 9 commits intomainfrom
Conversation
jcesarmobile
left a comment
There was a problem hiding this comment.
RMET-5155 is about both .xcframework and .framework, this seems to only handle .xcframework.
Not sure if .framework aren't compatible with binaryTarget or if you missed that.
If they are not compatible we should warn some message if .framework files are present as the plugin won't work.
Regular |
|
I prefer to keep them separate |
OS-pedrogustavobilro
left a comment
There was a problem hiding this comment.
I've been testing different apps and everything seems to be working, leaving a couple of comments though, but nothing major I think.
| ); | ||
| await writeFile(packageSwiftPath, content); | ||
| } else { | ||
| const frameworks = getPlatformElement(p, platform, 'framework'); |
There was a problem hiding this comment.
I reckon probably as this method may grow, we may want to split it in a few ones, e.g. one to get the xcframework(s) and binary target, another to write Package.swift, and stuff like that.
But doesn't necessarily need to happen in this PR.
There was a problem hiding this comment.
makes sense, I can do it in this PR.
There was a problem hiding this comment.
done, take another look when you can
There was a problem hiding this comment.
Since we will need to add more entries to the Package.swift(csettings, resource files, etc.), I would rather not have the writeGeneratedPackageSwift method as we will have to keep adding more and more params.
Or if we have it, it should have the methods for getting the texts inside instead of as params, i.e. for this PR, buildBinaryTargetEntries should be inside writeGeneratedPackageSwift to get the texts, rather than being called before to pass the texts as params.
There was a problem hiding this comment.
Makes sense, moved buildBinaryTargetEntries inside writeGeneratedPackageSwift so that as more entries are added in future PRs, they can be handled there directly without growing the parameter list.
I'll apply the same for #8448 as well. Depending on which PR gets merged first there may be a conflict, but it should be straightforward to resolve.
|
Minor: I think the PR title should be |
e236d77 to
899ac65
Compare
When Capacitor CLI generates a
Package.swiftfor a Cordova plugin in SPM mode, custom.xcframeworkfiles declared via<framework custom="true" src="..." />inplugin.xmlwere not being translated intobinaryTargetentries. The xcframework was already being copied to the correct location bycopyPluginsNativeFiles, but the generatedPackage.swifthad no reference to it, causing build failures.This PR adds
binaryTargetentries and the corresponding target dependencies for each custom.xcframeworkfound in the plugin'splugin.xml.Reference https://outsystemsrd.atlassian.net/browse/RMET-5155