-
Notifications
You must be signed in to change notification settings - Fork 7
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
chore: distribute CoreCrypto for iOS via Swift Package Manager - WPB-11043 #919
base: main
Are you sure you want to change the base?
Conversation
.binaryTarget( | ||
name: "WireCoreCrypto", | ||
url: "https://github.com/wireapp/core-crypto/releases/download/v3.1.0/WireCoreCrypto.xcframework.zip", | ||
checksum: "3bb569dc7041f5e062abab2fb8a1b175e850d61978deb17150bc52bfe20302d3" |
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.
How should we calculate this checksum? A md5 of the WireCoreCrypto.xcframework.zip
, or something?
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.
swift package compute-checksum <path>
targets: [ | ||
.binaryTarget( | ||
name: "WireCoreCrypto", | ||
url: "https://github.com/wireapp/core-crypto/releases/download/v3.1.0/WireCoreCrypto.xcframework.zip", |
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.
If we're pointing this Package.swift
at a specific release package, then perhaps this file should also be attached to a release rather than the repo in general? (Otherwise, right now, we should be pointing it at v4.1.0 for the current version.)
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.
yes the flow could differ a bit, like:
- draft a release on github
- compute the binary
- calculate checksum
- edit package.swift
- commit
- tag
- release
@@ -0,0 +1,23 @@ | |||
// swift-tools-version: 6.0 |
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.
Showing my own ignorance, but this looks like it may have been generated by some swift tooling. Is that tooling ios/swift-exclusive, or generally available?
I'm trying to weigh the options for autogenerating this file, and it currently feels like the top contenders right now are a (handlebars/jinja/etc) template, or installing proper tooling. But I don't know if installing proper tooling is just "use the proper GHA dependency" or if it's a more complicated thing.
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.
so this header is used by swift binary (SPM) to interpret the file
|
||
let package = Package( | ||
name: "WireCoreCrypto", | ||
platforms: [.iOS(.v16), .macOS(.v12)], |
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.
Can we derive the supported platforms from our source somehow? When would we need to update these values?
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.
usually you would follow whatever wire-ios is supporting, but can support more
What's new in this PR
This PR adds a Package.swift file to allow iOS app to use binary via SPM. Currently it points to 3.1.0.
Note: this probably needs some work to be updated via CC's CI to calculate checksum and update the version.
Note 2: as a first step this file is hosted for now in wire-ios repo. See wireapp/wire-ios#2548
PR Submission Checklist for internal contributors
SQPIT-764
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.