-
Notifications
You must be signed in to change notification settings - Fork 45
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
Move Swift publishing workflow to main repo #1033
Conversation
a1e7a41
to
add1399
Compare
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.
Looking good to me (always hard to say that with CI, you never know if it works)! I'd say if this is tested it's good to go.
We have a similar setup for go. There we pass a repo ssh key to provide commit access. https://github.com/breez/breez-sdk-greenlight/blob/main/.github/workflows/publish-golang.yml#L31. I think we can use the same construct here (so option 1). |
Also check out the https://github.com/breez/breez-sdk-liquid repo, it publishes from the main publishing flow |
Thanks for the feedback guys and the pointer to the other repos. Will have a look! |
Alright, I've added an action-archival step for the XCFramework (the language bindings are already archived here). That means we're now just missing the publishing token analogous to the publishing for the Swift publishing for breez-sdk-liquid. @yaslama Is that ok with you? I know we've previously talked about trying not to use a token. Let me know what you think. 🙏 |
Let's use tokens for now and try to find an alternative. |
Looks good it me, if it works, let's merge it. Did you test this with |
.github/workflows/publish-swift.yml
Outdated
- name: Archive xcframework | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: breez_sdkFFI-${{ inputs.package-version || intouts.ref }}.xcframework |
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.
This is causing the publish CI to fail when testing
name: breez_sdkFFI-${{ inputs.package-version || intouts.ref }}.xcframework | |
name: breez_sdkFFI-${{ inputs.package-version || inputs.ref }}.xcframework |
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.
Damn, typo! Thanks for catching!
.github/workflows/publish-swift.yml
Outdated
|
||
- name: Release and attach XCFramework binary artifact | ||
if: ${{ inputs.publish }} | ||
uses: ncipollo/release-action@v1 |
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.
I seem to remember this action had issues in the breez-sdk-liquid repo when using a token. There we use softprops/action-gh-release@v2
as the release action instead.
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.
Good insight thanks! Will use softprops/action-gh-release@v2
here too. Good for consistency too! 👏
@JssDWt Thanks for your feedback! 🙇 I primarily tested the syncing logic to ensure that the React Native and Flutter publishing workflows behave as follows:
Here's my test notes:
I did some minor updates and addressing feedback after testing these scenarios but I think those should be irrelevant and the tests still valid. Of course, CI is hard to fully test so when we merge this and see that publishing fails feel free to revert and let me know and I will fix whatever problems there might be. |
Who can help me with that? |
This is a problem that comes up after rust 1.80. I ran into this issue with cln as well, I downgraded to rust 1.79 there. Let me see how to fix this here. |
@cnixbtc can you try a rebase? |
Co-authored-by: Jesse de Wit <[email protected]>
Co-authored-by: Ross Savage <[email protected]>
Co-authored-by: Ross Savage <[email protected]>
fde8e27
to
0715a42
Compare
Rebased onto main. ✔️ Though I thin there's still issues with the bindings. |
Guys, as mentioned above, I will be off for 2 weeks now. I can take this over the finish line and when I'm back but of course feel free to merge it beforehand when you feel like it. Whatever it is thanks for the help so far and I'll report back in two weeks. :) If there is anything I can be reached on Slack but won't have a laptop with me to jump into code. |
I've removed the rust caches, and now the build works. It was picking |
Alright, so I did three more test runs. For all three I used the workflow from this branch and the repo code from the 0.6.1 tag.
Good to go from my side. |
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.
LGTM
@dangeross I checked with Roei,
|
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.
LGTM
Merging this now. Please ping me if you feel like something is not working when pushing out the next Swift release. We still have the workflow in the Swift repo as backup in case anything breaks with the new workflow in the main repo here. cc @roeierez |
Swift Publishing Workflow Consolidation
This PR relocates the Swift publishing workflow from the
breez/breez-sdk-swift
repository to the main repository. This change simplifies the publishing process by integrating it with the main repo's CI workflow.📖 Overview
Previously, the Swift publishing workflow required separate triggers, unlike other platforms managed centrally in the main repo via the
publish-all-platforms
workflow. This PR enhances the Swift package publishing by:One interesting point: Because our Flutter and React Native packages rely on the Swift package at runtime, the PR includes logic to ensure that the Flutter and React Native packages are only published if the Swift package is successfully published too, preventing issues with runtime dependencies. If the Swift package is not set to be published, then the Flutter and React Native packages are published nonetheless allowing us to release updates to those packages that depend on an already published Swift package. Special thanks to @JssDWt for the solution adapted from this PR.
❓ Outstanding Questions
Before merging this PR, we need to address two key points. The first is straightforward, while the second requires some feedback.
Setting CocoaPods Token
We need to set
secrets.COCOAPODS_TRUNK_TOKEN
in the main repo to publish to the CocoaPods trunk. This token is currently only set in thebreez-sdk-swift
repo (I believe we're using @roeierez's token there). The simplest solution is to just move the token from the Swift repo to the main repo.Publishing
breez_sdkFFI.xcframework
The Swift package and CocoaPods require a download link to the
breez_sdkFFI.xcframework
binary artifact. Currently, we create a GitHub release in thebreez-sdk-swift
repo and attach the XCFramework as a binary artifact. Moving the workflow to the main repo presents a few challenges for maintaining this setup. I believe options 1 and 3 are the most straightforward, with option 2 being a viable alternative that may need some experimentation.breez-sdk-swift
repo from the CI workflow in the main repo. While simple, this requires using user-bound tokens with broad permissions, which isn't ideal.breez-sdk-greenlight
repo instead of thebreez-sdk-swift
repo and attach the artifact there. This option is easy to set up and doesn't require special tokens, but it requires releases to be created on the main repo whenever we want to update or publish the Swift package. Not ideal, imo.✋ Feedback Request
I'd appreciate your thoughts on these options, particularly regarding the second point. Looking forward to your feedback. 🙏