Skip to content
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

[Broken] feat(spm): Support plugins as Swift packages #1515

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Dec 17, 2024

🚨 This DOES NOT work currently 🚨

See below for details.

Platforms affected

iOS

Motivation and Context

Ref GH-1089.

Swift package manager is the built-in way to handle 3rd party dependencies for Xcode and Swift projects, and Cocoapods is no longer in active development. We want to provide a way for plugins to express and manage their dependencies as Swift packages.

The best way to do that seems to be for the plugins themselves to (optionally) become Swift packages.

Description

When a plugin declares in its plugin.xml file that it supports <platform name="ios" package="swift"> and contains a Package.swift file in the plugin root, we will treat it as a Swift package and add it as a dependency of the app project (technically a dependency of a stub dependency, because that's easier than monkeying further with Xcodeproj files).

Why This Currently Fails

Each Swift package is treated as its own self-contained library, with access only to the dependencies that it declares. This means that plugins cannot access CordovaLib headers unless the declare a dependency on CordovaLib in their own Package.swift file. That would end up tying them to a very specific version of Cordova and potentially resulting in conflicts due to multiple plugins requesting different versions that don't match the app's version.

It feels like the only workaround is for the plugin install process to edit the Package.swift of each plugin and adjust the CordovaLib dependency to point to the local app one, but then we need to be able to parse and modify arbitrary Package.swift files.

Even with the plan to rewrite, it's not clear (for our own Apache plugins) if we can point them at the master branch of cordova-ios and still pass a release vote, because they would be pointing to a dependency that has not had a release vote. This could result in needing to churn the version dependency in every plugin every time we release a new cordova-ios version (even if we don't need to publish those changes).

Testing

Tested locally against this branch of cordova-plugin-device: https://github.com/dpogue/cordova-plugin-device/tree/spm

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@dpogue dpogue added this to the 8.0.0 milestone Dec 17, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.97%. Comparing base (5cf4d94) to head (b92ceaa).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1515      +/-   ##
==========================================
+ Coverage   81.41%   81.97%   +0.56%     
==========================================
  Files          16       17       +1     
  Lines        1862     1920      +58     
==========================================
+ Hits         1516     1574      +58     
  Misses        346      346              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@breautek
Copy link
Contributor

Even with the plan to rewrite, it's not clear (for our own Apache plugins) if we can point them at the master branch of cordova-ios and still pass a release vote, because they would be pointing to a dependency that has not had a release vote. This could result in needing to churn the version dependency in every plugin every time we release a new cordova-ios version (even if we don't need to publish those changes).

I would think it's not acceptable to point it to master branch as that would effectively be using the dev build. At minimum it would need to be using the rel/<version> tag. But we could potentially have a "Local" SPM package that is just simply shipped with the cordova-ios tarball. The app template could reference it locally via file path. Not 100% sure how that will work with plugins however.

Each Swift package is treated as its own self-contained library, with access only to the dependencies that it declares. This means that plugins cannot access CordovaLib headers unless the declare a dependency on CordovaLib in their own Package.swift file.

I think this is more of a side issue because our plugins are not proper modules. They are just loose source code that eventually makes their way part of the App project. And I assume SPM packages are importing these packages as their own build targets, which means they have their own build configurations (and thus needs a reference to the CordovaLib framework).

This in itself shouldn't be a blocker though, as long as CordovaLib framework is consumed as a dynamic library, instead of a static library. Plugins should be able to declare the dependency on CordovaLib and compile against it without providing the CordovaLib framework itself. The application project will be the one responsible of importing the CordovaLib framework, and making sure that the dynamic library is available during runtime (by packaging it with it's product). Because most plugins are not pre-compiled, if they are any incompatibilities, it should appear during build time. Plugin authors pre-building their plugins and distributing a prebuilt binary however would need to take extra care to not call on an API that might not exist without properly guarding so, as it would cause runtime crashes.

I think this is more of a side issue because our plugins are not proper modules.

Extending on this, a year or two ago I was experimenting with an alternate approach in authoring plugins, so that plugins can have their own native projects and can compile (and potentially run) independently of a Cordova project. Would be very useful to have for using native unit test features. This involved having plugins be in their own projects and having their own build targets, so each plugin would produce their own .framework which gets imported by the Cordova application. This mostly worked but I did run into runtime errors where the cordova framework failed to look up any symbol that came from a dynamic library, wihch the cordova framework does when processing native bridge calls here if my memory serves me right.

Just some thoughts. I've been experimenting with SPM with my own framework, and will be sharing what worked for me once I have something but it is a very different architecture than Cordova.

@dpogue
Copy link
Member Author

dpogue commented Dec 18, 2024

This in itself shouldn't be a blocker though, as long as CordovaLib framework is consumed as a dynamic library, instead of a static library. Plugins should be able to declare the dependency on CordovaLib and compile against it without providing the CordovaLib framework itself.

This actually works if CordovaLib is a static library for the app target, but not when it's consumed as a Swift package itself (which is how we're using it in 8.0.0)

I assume SPM packages are importing these packages as their own build targets, which means they have their own build configurations (and thus needs a reference to the CordovaLib framework)

This is correct, when a plugin is a Swift package, it becomes its own framework target that is linked to the app project, rather than copying plugin source files into the project directly.

@breautek
Copy link
Contributor

This actually works if CordovaLib is a static library for the app target, but not when it's consumed as a Swift package itself (which is how we're using it in 8.0.0)

I would have thought that this would create duplicate symbols in the app binary.

If we have 3 targets:

  1. App (Produces Executable)
  2. CordovaLib (Produces Static Library)
  3. Plugin (Produces Dynamic Library)

CordovaLib itself is standalone.
Plugin would link against CordovaLib and produces an executable object (the shared library). If CordovaLib is static, it's symbols should be embedded into this shared library.
App would link against CordovaLib & Plugin and here during linking I would expect duplicate symbol issues, because CordovaLib symbols will be embedded into both Plugin binary, and the App executable binary.

This configuration would work, but only if you have no plugins, as a plugin that is linking with a static CordovaLib would cause duplicate symbols assuming that the app is also linking against the same static library. Doesn't this happen?

@dpogue
Copy link
Member Author

dpogue commented Dec 18, 2024

In the case where it worked with the static CordovaLib, the plugin did not specify a dependency on CordovaLib as part of its Package.swift file

When using Package.swift, even if the app asks to consume CordovaLib at the top level as a static library, plugins cannot find Cordova headers unless they also include a dependency on CordovaLib

@dpogue dpogue force-pushed the spm-plugins branch 2 times, most recently from 535f00d to 328dd50 Compare January 18, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants