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

Swift package compatibility #1778

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Topheee
Copy link

@Topheee Topheee commented May 7, 2023

Pull Request Checklist

TL;DR This PR makes distributing the SDK as a binary Swift package possible by not exporting the interfaces of depending Swift libraries (Realm, MatrixSDKCrypto, and SwiftyBeaver). This is necessary to solve #1497.

However, it comes with drawbacks. And to understand them, I wrote what follows.

Background

I will stick to Realm as the primary example in the following, but it applies of course to any of the dependencies.

When you try building this repository and ship it as a Swift binary package, it first seems to work.

However, once you try to compile your dependent project, you get linking errors: built with a different Swift version.
This happens if you build on a different machine with a different compiler version. So I added BUILD_LIBRARY_FOR_DISTRIBUTION=YES (explained in Swift Library Evolution), even for the depending Pods (by adding a new row config.build_settings['BUILD_LIBRARY_FOR_DISTRIBUTION'] = 'YES' in the post_install phase of the Podfile). I also added the Realm, MatrixSDKCrypto, and SwiftyBeaver dependencies to the Swift package, believing this would fix the issue.

Turns out the error persists. As explained briefly at 38:10 in the Binary Frameworks in Swift video, Binary Frameworks Cannot Depend on Packages.

So what we need to achieve is linking everything statically into the SDK and thus hiding it from the client. But aren't we already doing this with use_frameworks! :linkage => :static in the Podfile? Turns out, not quite.

Because as explained in at the top of this thread:

Today, if your Swift library "Foo" uses a library "Bar", the headers (or swiftmodule files) of "Bar" must be available to all clients of "Foo".

And thus the emitted .swiftinterface still contains import Realm (and the others)!

Now here is where you might think 'then just add Realm as a dependency to your client'. But that does not work: if you add it as a source package dependency, it would get compiled on your local machine and would not link. You may could add it as a binary package dependency (although I did not test that for the reason following), but that would mean that we need to re-distribute Realm and all the other dependencies as binary packages. And I also believe that it still won't compile.

So what we need is a way to tell Swift that we want those dependencies not exposed. But here is where the drawbacks come in.

There is a way to do such things, but the name already reveals the first drawback: @_implementationOnly. More precisely, the underscore at the beginning of the name, which indicates that it is not a standard yet.

The general limitations of this are discussed on this Swift Forum's Thread, but two of them impact this project the most:

  1. @testable import does not work in conjunction with @_implementationOnly.
  2. In general, if a public type in MatrixSDK overrides a method defined in Realm, the override has to be marked @_implementationOnly as well.

The first one can be resolved with a little compiler control statement trickery (providing -D IS_TEST_RUN using the Other Swift Flags setting in Xcode):

#if IS_TEST_RUN
import SwiftyBeaver
#else
@_implementationOnly import SwiftyBeaver
#endif

This solution again comes with drawbacks: it breaks with the idea that tested code should be compiled the same as code running in production (although @testable already breaks this in a sense), and Xcode complains with 'SwiftyBeaver' inconsistently imported as implementation-only warnings.

Thus, I added it only for the SwiftyBeaver import in MXAnalyticsDestination.swift, because its test is the only one that broke after adding @_implementationOnly.

For the second one I did not find a solution, at least not for the special case where an extension adds protocol conformance to a type, as is done in RLMSupport.swift:

extension RLMArray: Sequence, _RLMCollectionIterator { }
extension RLMDictionary: Sequence, _RLMDictionaryIterator {}
extension RLMSet: Sequence, _RLMCollectionIterator {}
extension RLMResults: Sequence, _RLMCollectionIterator {}

There might also be a way to solve this, but I did not find it.

However, there is only one occurrence in the SDK where it makes use of these extensions; which I quickly refactored. But this of course drops some comfort.

Sorry for long post, but I hope it helps understanding the issue a bit better!

Additions

I made a binary package using these changes and made it available here: https://github.com/Topheee/MatrixSDK.
It builds from the branch this PR requests to merge.

It may make sense to add the Swift package to this repository as well. In contrast to the original, my GitHub action does not build for Catalyst (because I was too stupid to get it to work). But it builds the different platforms in parallel and adds debug symbols on iOS and iOS Simulator.

We might be able to merge Nio's and my action into a sensible one for this project, possibly in a follow-up PR.

Signed-off-by: Christopher Kobusch [email protected]

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.

2 participants