breez-sdk icon indicating copy to clipboard operation
breez-sdk copied to clipboard

Published 20 hours ago • verified publisher icon breez

Move Swift publishing workflow to main repo

Open cnixbtc opened this issue 1 year ago • 20 comments
trafficstars

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:

  • Streamlining the publishing process by integrating the Swift package into the main repo's CI workflow.
  • Improving build runtime and reliability by reusing the already built binary SDK and language bindings from the main repo's workflows.

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.

  1. 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 the breez-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.

  2. 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 the breez-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.

    • Option 1: Use a GitHub Personal Access Token to replicate the current setup. This would involve creating a release in the 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.
    • Option 2: Use a GitHub app to provide short-lived tokens for the workflow. This option offers better security with non-user-bound tokens but requires more setup.
    • Option 3: Create the release in the main breez-sdk-greenlight repo instead of the breez-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.
    • Option 4: Find an alternative hosting solution outside of GitHub for the artifact, accessible to our users.

✋ Feedback Request

I'd appreciate your thoughts on these options, particularly regarding the second point. Looking forward to your feedback. 🙏

cnixbtc avatar Jul 08 '24 13:07 cnixbtc

2. 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 the breez-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.

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).

JssDWt avatar Aug 16 '24 11:08 JssDWt

Also check out the https://github.com/breez/breez-sdk-liquid repo, it publishes from the main publishing flow

dangeross avatar Aug 16 '24 12:08 dangeross

Thanks for the feedback guys and the pointer to the other repos.

Will have a look!

cnixbtc avatar Aug 20 '24 17:08 cnixbtc

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. 🙏

cnixbtc avatar Aug 31 '24 09:08 cnixbtc

@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.

yaslama avatar Aug 31 '24 17:08 yaslama

Looks good it me, if it works, let's merge it. Did you test this with publish=false with some different configurations?

JssDWt avatar Sep 01 '24 15:09 JssDWt

Looks good it me, if it works, let's merge it. Did you test this with publish=false with some different configurations?

@JssDWt Thanks for your feedback! 🙇 I primarily tested the syncing logic to ensure that the React Native and Flutter publishing workflows behave as follows:

  • Both Swift and Android packages are published: React Native and Flutter are published only if both Swift and Android publishing succeed.
  • Either Swift or Android package is published: React Native and Flutter are published only if the respective (Swift or Android) publishing succeeds.
  • Neither Swift nor Android packages are published: React Native and Flutter are published regardless of Swift or Android status, allowing for RN/Flutter patches without re-publishing Swift or Android.
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.

cnixbtc avatar Sep 03 '24 17:09 cnixbtc

⚠️ Before we merge this, we need to set the SWIFT_RELEASE_TOKEN for the https://github.com/breez/breez-sdk-swift repo as a GH actions secret here.

Who can help me with that?

cnixbtc avatar Sep 03 '24 17:09 cnixbtc

Here's my test notes:

  • Tests using dummy binaries (publish == false)

            * Wokflow config: ["maven", "swift"]

                  * https://github.com/breez/breez-sdk/actions/runs/9928882340
                  * jobs dependent on maven and swift run after maven and swift as expected
        * Wokflow config: ["maven"]

                  * https://github.com/breez/breez-sdk/actions/runs/9928828401
                  * jobs dependent on maven and swift run after maven as expected
        * Wokflow config: ["swift"]

                  * https://github.com/breez/breez-sdk/actions/runs/9928935630
                  * jobs dependent on maven and swift run after swift as expected
        * Wokflow config: ["maven", "swift"]

                  * hardcoded a fail for swift
                  * https://github.com/breez/breez-sdk/actions/runs/9928994716
                  * jobs dependent on maven and swift don't run as expected
        * Wokflow config: ["golang"]

                  * https://github.com/breez/breez-sdk/actions/runs/9929068463
                  * jobs dependent on maven and swift run after as expected

  • Tests using actual binaries (publish == false)

            * Wokflow config: ["flutter", "swift"]

                  * https://github.com/breez/breez-sdk/actions/runs/9929607744
                  * jobs dependent on maven and swift run after maven and swift as expected

Great notes!

JssDWt avatar Sep 03 '24 18:09 JssDWt

⚠️ Before we merge this, we need to set the SWIFT_RELEASE_TOKEN for the https://github.com/breez/breez-sdk-swift repo as a GH actions secret here.

Who can help me with that?

@roeierez

JssDWt avatar Sep 03 '24 18:09 JssDWt

⚠️ Before we merge this, we need to set the SWIFT_RELEASE_TOKEN for the https://github.com/breez/breez-sdk-swift repo as a GH actions secret here. Who can help me with that?

@roeierez

@JssDWt @cnixbtc Done.

roeierez avatar Sep 04 '24 08:09 roeierez

Thanks for the corrections @dangeross. Note to myself: No more last minute changes without proofreading. Will be more careful with these things next time. I'll give it one more trial run here: https://github.com/breez/breez-sdk-greenlight/actions/runs/10706106599

Just to double check: Is the COCOAPODS_TRUNK_TOKEN already set as a secret in this repo? If not I think @roeierez will need to add that one too.

cnixbtc avatar Sep 04 '24 16:09 cnixbtc

Thanks for the corrections @dangeross. Note to myself: No more last minute changes without proofreading. Will be more careful with these things next time. I'll give it one more trial run here: https://github.com/breez/breez-sdk-greenlight/actions/runs/10706106599

Just to double check: Is the COCOAPODS_TRUNK_TOKEN already set as a secret in this repo? If not I think @roeierez will need to add that one too.

Test run failed due to some (I think) unrelated issues with building the Kotlin lang bindings. But I saw that @JssDWt did some test runs earlier that were successful. I will be on holiday for the next 2 weeks (starting on the 7th). Let me know guys if you'd prefer to merge this before or after. :)

cnixbtc avatar Sep 05 '24 18:09 cnixbtc

Test run failed due to some (I think) unrelated issues with building the Kotlin lang bindings. But I saw that @JssDWt did some test runs earlier that were successful. I will be on holiday for the next 2 weeks (starting on the 7th). Let me know guys if you'd prefer to merge this before or after. :)

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.

JssDWt avatar Sep 05 '24 19:09 JssDWt

@cnixbtc can you try a rebase?

JssDWt avatar Sep 05 '24 19:09 JssDWt

@cnixbtc can you try a rebase?

Rebased onto main. ✔️ Though I thin there's still issues with the bindings.

cnixbtc avatar Sep 07 '24 08:09 cnixbtc

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.

cnixbtc avatar Sep 07 '24 08:09 cnixbtc

I've removed the rust caches, and now the build works. It was picking time 0.3.30, where it should have picked 0.3.36`. Or maybe the build didn't use your rebased version yet.

JssDWt avatar Sep 08 '24 11:09 JssDWt

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.

  1. ["flutter", "react-native", "swift"]: swift runs first, then flutter and react native in parallel
  2. ["react-native", "flutter"]: both run in parallel without waiting for the (non existing) swift build
  3. ["swift"]: runs as expected

Good to go from my side.

cnixbtc avatar Sep 30 '24 17:09 cnixbtc

Just to double check: Is the COCOAPODS_TRUNK_TOKEN already set as a secret in this repo? If not I think @roeierez will need to add that one too.

@roeierez

dangeross avatar Sep 30 '24 19:09 dangeross

@dangeross I checked with Roei, COCOAPODS_TRUNK_TOKEN is set.

SWIFT_RELEASE_TOKEN is set too and REPO_SSH_KEY is used by other jobs too so must be set already. 🙃

cnixbtc avatar Oct 16 '24 19:10 cnixbtc

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

cnixbtc avatar Oct 23 '24 17:10 cnixbtc