search

breez / breez-sdk-greenlight

MIT License
244 stars 43 forks source link

Move Swift publishing workflow to main repo #1033

Closed cnixbtc closed 1 month ago

cnixbtc commented 4 months ago

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.

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

JssDWt commented 3 months ago

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

dangeross commented 3 months ago

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

cnixbtc commented 3 months ago

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

Will have a look!

cnixbtc commented 2 months ago

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

yaslama commented 2 months ago

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

JssDWt commented 2 months ago

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

cnixbtc commented 2 months ago

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:

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 commented 2 months ago

⚠️ 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?

JssDWt commented 2 months ago

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 commented 2 months ago

⚠️ 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

roeierez commented 2 months ago

⚠️ 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.

cnixbtc commented 2 months ago

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 commented 2 months ago

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

JssDWt commented 2 months ago

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 commented 2 months ago

@cnixbtc can you try a rebase?

cnixbtc commented 2 months ago

@cnixbtc can you try a rebase?

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

cnixbtc commented 2 months ago

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.

JssDWt commented 2 months ago

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

cnixbtc commented 1 month ago

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.

dangeross commented 1 month ago

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

cnixbtc commented 1 month ago

@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 commented 1 month ago

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