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

Add SPM support with XCF #789

Merged
merged 11 commits into from
Apr 28, 2021
Merged

Conversation

julepka
Copy link
Contributor

@julepka julepka commented Mar 12, 2021

Introducing Swift Package Manager support with xcframework.

Details:

  • Created a script create_xcframewok.sh to build an xcframwork for Swift for iOS, iOSSimulator, MacOS. Basically, took it from AppSpector tutorial. The xcframework is created in the build folder. When releasing a new version, we'll need to add the generated xcframework to the release assets to have a direct link to it. Related documentation will be updated.
  • Added Package.swift file to support SPM. It should be placed in the root of the repo. It contains a link to the xcframework. When releasing a new version, the link should be updated with a proper version (future version you are going to release, pretty similar to carthage). So, when you create a release/tag on a GitHub you can reference a corresponding commit.
  • Example projects will be added in the upcoming version together with updated OpenSSL version.
  • I had to remove OpenSSL from SPM inside Themis.xcodeproj because of the signature issues. It failed our Carthage tests. It seems to be resolved with Xcode 12.5 that is not available for Github actions yet.

Checklist

  • Change is covered by automated tests – will be covered separately, added to our TODO list
  • The coding guidelines are followed
  • Example projects and code samples are up-to-date (in case of API changes) – will be covered separatly
  • Changelog is updated (in case of notable or breaking changes) – TODO when links are updated

@vixentael vixentael added W-SwiftThemis 🔶 Wrapper: SwiftThemis, Swift API M-SPM Package manager: Swift Package Manager, Swift, iOS and macOS labels Mar 12, 2021
repositoryURL = "https://github.com/julepka/openssl-apple";
requirement = {
kind = upToNextMajorVersion;
minimumVersion = 1.1.1080202;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
minimumVersion = 1.1.1080202;
minimumVersion = 1.1.108203;

TODO: re-define version after merging cossacklabs/openssl-apple#22

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, and the link will point to the cosacklabs repo.

@vixentael vixentael mentioned this pull request Mar 12, 2021
1 task
@ilammy
Copy link
Collaborator

ilammy commented Mar 13, 2021

Also, I should probably move the create_xcframeworks.sh script to some folder. I believe the scripts one should be good. Correct me if I'm wrong.

I think that scripts/create_xcframeworks.sh would be the right location, yes.

Copy link
Collaborator

@ilammy ilammy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good! Thank you, @julepka, for your brain tissue donation to the Apple Gods. I'm sorry you had to do this.

My questions from cossacklabs/openssl-apple#22 applies here as well: regarding the static/dynamic divide. Just in case Themis would need to become a static framework, or switch to shared dynamic OpenSSL, it's interesting to know how painful that would be.

Also, there are some new for Themis future specifically:

  • Since there is no precedent of attaching binaries to Themis releases, how that would play out in the general release flow?
  • Given that CLOpenSSL serves as a model example of making binary-only releases, what would become to CocoaPods and Carthage publications of Themis? Will they eventually migrate to using the same binaries as SPM, or continue building Themis from source?

name: "themis",
targets: ["themis"]),
],
dependencies: [],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I wanted to ask, “Where is cl-openssl?“ But then I realized that it's not here because themis.xcframework will be statically linked to openssl.xcframework, hence users of themis.xcframework don't need to install shared OpenSSL.

Package.swift Outdated Show resolved Hide resolved
Themis.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
create_xcframeworks.sh Outdated Show resolved Hide resolved
create_xcframeworks.sh Outdated Show resolved Hide resolved
create_xcframeworks.sh Outdated Show resolved Hide resolved
@ilammy
Copy link
Collaborator

ilammy commented Mar 13, 2021

Oh, and one more thing: it would be cool to exercise the script on CI, just to be sure it will work in the time of need.

@julepka
Copy link
Contributor Author

julepka commented Mar 15, 2021

@ilammy

My questions from cossacklabs/openssl-apple#22 applies here as well: regarding the static/dynamic divide. Just in case Themis would need to become a static framework, or switch to shared dynamic OpenSSL, it's interesting to know how painful that would be.

The only thing I can say, it is very unpredictable. I've tried the most straightforward and recommended solution. I hope to make it work as an MVP for out XCF and SPM support. Then. we'll see what we can do with the linking type.

Since there is no precedent of attaching binaries to Themis releases, how that would play out in the general release flow?

Yeah, looks like we'll need to describe XCF flow in our releasing checklist. In a perfect world, CI will do everything for us, but for now, we will build and attach XCF manually. We will make XCF work and then improve whats missing.

Given that CLOpenSSL serves as a model example of making binary-only releases, what would become to CocoaPods and Carthage publications of Themis? Will they eventually migrate to using the same binaries as SPM, or continue building Themis from source?

It may be possible, but I can't say for sure right now.

Package.swift Outdated Show resolved Hide resolved
scripts/create_xcframeworks.sh Outdated Show resolved Hide resolved
scripts/create_xcframeworks.sh Outdated Show resolved Hide resolved
scripts/create_xcframeworks.sh Outdated Show resolved Hide resolved
scripts/create_xcframeworks.sh Outdated Show resolved Hide resolved
@julepka
Copy link
Contributor Author

julepka commented Apr 26, 2021

I had to remove cl-openssl SPM packege from the Themis.xcodeproj because Xcode 12 has signing issues if you have a complex structure if binary frameworks dependencies. https://developer.apple.com/forums/thread/656367

It looks like the issue is fixed in Xcode 12.5 but github actions do not support it yet.

@ilammy
Copy link
Collaborator

ilammy commented Apr 27, 2021

I had to remove cl-openssl SPM packege from the Themis.xcodeproj

Hm... So it still depends on that package, but hides it so that Xcode stops eating glue? 🤔

It looks like the issue is fixed in Xcode 12.5 but github actions do not support it yet.

Well, they seem to support it in their macOS 11 image, but that one has been in “private preview mode” for quite a while.

Now that you've mentioned it, I'm kinda sad because Xcode 12.5 requires macOS 11. I'm in for a painful upgrade.

@vixentael
Copy link
Contributor

Now that you've mentioned it, I'm kinda sad because Xcode 12.5 requires macOS 11. I'm in for a painful upgrade.

all of us... all of us 💀

@julepka julepka changed the base branch from master to release/0.13 April 27, 2021 14:50
@julepka julepka changed the base branch from release/0.13 to master April 27, 2021 14:52
@julepka julepka marked this pull request as ready for review April 28, 2021 08:50
@julepka julepka requested a review from shadinua as a code owner April 28, 2021 08:50
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: vixentael <[email protected]>
@julepka julepka merged commit dbe70f2 into cossacklabs:master Apr 28, 2021
julepka added a commit to julepka/themis that referenced this pull request Apr 28, 2021
* add SPM support with XCF

* add swift SPM test project

* add macOS swift SPM example, updated iOS one

* updates per pr comments

* removed cl-openssl from Themis.xcodeproj + edits per pr comments + extra comments

* removed example projects

* updated xcodeproj version

* updated Package.swift

* updated changelog

* Update CHANGELOG.md

Co-authored-by: vixentael <[email protected]>

Co-authored-by: vixentael <[email protected]>
julepka added a commit that referenced this pull request Apr 28, 2021
* Add SPM support with XCF (#789)
* add SPM support with XCF
* add swift SPM test project
* add macOS swift SPM example, updated iOS one
* updates per pr comments
* removed cl-openssl from Themis.xcodeproj + edits per pr comments + extra comments
* removed example projects
* updated xcodeproj version
* updated Package.swift
* updated changelog
* Update CHANGELOG.md
Co-authored-by: vixentael <[email protected]>
Co-authored-by: vixentael <[email protected]>
* fixed automerge issue
* fixed typo
Co-authored-by: vixentael <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-SPM Package manager: Swift Package Manager, Swift, iOS and macOS W-SwiftThemis 🔶 Wrapper: SwiftThemis, Swift API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants