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

Fix Swift builds #294

Merged
merged 2 commits into from
May 21, 2024
Merged

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented May 20, 2024

Fixes #231
Fixes #279

Unfortunately Apple broke our Swift packages with the recent Xcode 15.3 release. Here, we add the (now) required information to (hopefully) make our builds works again.

Moreover, we add a oneliner, automatically patching LDKNode.swift with the fix for #231 in the build script.

@tnull tnull marked this pull request as draft May 20, 2024 11:56
@reez
Copy link

reez commented May 20, 2024

tl;dr I got this working in Xcode 15.4 w a small additional fix.

But since what we are fixing is a bit of a black box here's my running notes on the process of how I tested and the fix I applied in case anyone else wants to run thru what I did or did something different or has any additional insights

Environment

I have the new Xcode 15.4, as well as still have the old (pre-everything-breaks) Xcode 15.2.

I’m using Xcode 15.2 as sanity tests for where everything should be working regardless so I can compare vs fixes for Xcode 15.4.

Versions


I’m using a version of Monday https://github.com/reez/Monday/pull/88/commits that is keeping up with all of the new changes in ldk-node https://github.com/lightningdevkit/ldk-node/commits/main/

the last change on ldk-node is b7c4862 which matches the local build of the swift bindings I’m using for that branch of Monday reez/Monday@81581bd

Current PR

https://github.com/tnull/ldk-node/tree/2024-05-fix-swift-builds only adds two commits on top of b7c4862

When I pull down #294 to use in Monday as a remote package for reez/Monday#88 I get a bunch of errors (as if all of the new commits in ldk-node from the past month+ or so haven’t been added) ...

Example: NodeStatus “Cannot find type ‘NodeStatus’ in scope”, even though I have the code for it https://github.com/reez/Monday/pull/88/files#diff-1f8112828f9e05100c7ec541d3485329e2f6ce3c20e634e2998cda39c41e6afeR18 , which I guess makes sense if https://github.com/tnull/ldk-node/blob/2024-05-fix-swift-builds/bindings/swift/Sources/LDKNode/LDKNode.swift hasn’t been updated in 4 months (even though code for NodeStatus has been merged into main).

Screenshot 2024-05-20 at 10 46 31 AM

Local Build

So lets locally build https://github.com/tnull/ldk-node/tree/2024-05-fix-swift-builds

I’ll fork tnull repo and build swift bindings locally.

I got these bindings to build and run successfully on Xcode 15.2!

Screenshot 2024-05-20 at 11 06 00 AM

And 

I got these bindings to build and run successfully on Xcode 15.4!


Screenshot 2024-05-20 at 11 08 18 AM

Now trying to archive Monday and upload to App Store Connect (last TestFlight build was 91), I got a failure for Info.plist (I ran into a small extra nuance like this when trying to do the same thing in bdk-ffi/bdk-swift when actually archiving+uploading with BDKSwiftExample Wallet so this might be what I ran into with BDK as well, lets do a BDK+LDKNode PR review/comparison real quick to see if we can catch anything)

Screenshot 2024-05-20 at 11 12 57 AM

This PR vs BDK PR (It looked like part of the Info plists were different than mine)

bindings/swift/LDKNodeFFI.xcframework/Info.plist



https://github.com/lightningdevkit/ldk-node/pull/294/files#diff-b50a77f58d4ab17a37b3151ffd1647c1c542fd693541edc82271d2a684556397R50-R51 100.0

https://github.com/bitcoindevkit/bdk-ffi/pull/466/files#diff-1d35206bd681a5c7562b5984ff5b2ce4cc413701e8c7195d5dcd614e653965b6R50-R51 15.0

bindings/swift/LDKNodeFFI.xcframework/ios-arm64/LDKNodeFFI.framework/Info.plist

https://github.com/lightningdevkit/ldk-node/pull/294/files#diff-62d9da0702a87267b9ac959c6b8e9262a604cbaa01412cc7e5567a0e9f4c007cR15-R16 15.0

https://github.com/bitcoindevkit/bdk-ffi/pull/466/files#diff-0006e59b40dfcfac3a5ebe4d63e74f75154f415f45b7e95f0dc8318aa5a604a0R15-R16 100

bdk-swift/bdkFFI.xcframework/ios-arm64_x86_64-simulator/bdkFFI.framework/Info.plist

Looks good

bindings/swift/LDKNodeFFI.xcframework/macos-arm64_x86_64/LDKNodeFFI.framework/Info.plist

Looks good

scripts/uniffi_bindgen_generate_swift.sh



We dont have this change in bdk-ffi, I’m just going to assume these are strictly for #231

Other

Also just make sure versions match Package.swift of specific repo (that's what I'm trying to match in the Info.plist files)

Yes both ldk-node https://github.com/tnull/ldk-node/blob/2024-05-fix-swift-builds/bindings/swift/Package.swift and bdk-ffi https://github.com/bitcoindevkit/bdk-ffi/blob/release/0.31/bdk-swift/Package.swift seem to support the same:


  • .macOS(.v12)
  • .iOS(.v15)

My additional fix

So lets update tnull fork branch with my additional fixes applied on top

Screenshot 2024-05-20 at 11 27 17 AM Screenshot 2024-05-20 at 11 27 20 AM

Build succeeds still!

Screenshot 2024-05-20 at 11 29 03 AM

I was able to archive and upload to App Store Connect with Xcode 15.4!

Screenshot 2024-05-20 at 11 29 38 AM

I also tested using that new TestFlight build 92 on a physical device myself as well, and that worked!

Everything works similar to bdk now where at least from my testing I'm able to do all of these things in Xcode 15.4:

  • Build
  • Run
  • Archive
  • Upload to App Store Connect

Follow up:

All of the changes that make this work locally just need to propagate to LDKNode.swift on the remote repo so that this will work for remotely pulling this into a project. Happy to help look into that, and also test that that works properly for Xcode 15.4 too, but I think that all should be pretty simple and quick to do (famous last words).

@tnull tnull force-pushed the 2024-05-fix-swift-builds branch from 974267d to 6dec002 Compare May 20, 2024 17:47
@tnull
Copy link
Collaborator Author

tnull commented May 20, 2024

Example: NodeStatus “Cannot find type ‘NodeStatus’ in scope”, even though I have the code for it https://github.com/reez/Monday/pull/88/files#diff-1f8112828f9e05100c7ec541d3485329e2f6ce3c20e634e2998cda39c41e6afeR18 , which I guess makes sense if https://github.com/tnull/ldk-node/blob/2024-05-fix-swift-builds/bindings/swift/Sources/LDKNode/LDKNode.swift hasn’t been updated in 4 months (even though code for NodeStatus has been merged into main).

Right, this is expected. We only update the checked-in files and the xcframework.zip upon the new release. IIUC, you could successfully use a local build to build and publish. I just amended the minor oversight (regarding 100.0) above.

So, if there's nothing else from your side, I'll merge this PR tomorrow. Then I'll backport it on top of 0.2.1 will release a fixed v0.2.2.

@tnull tnull marked this pull request as ready for review May 20, 2024 17:53
@tnull tnull changed the title WIP Fix Swift builds Fix Swift builds May 20, 2024
@reez
Copy link

reez commented May 20, 2024

Example: NodeStatus “Cannot find type ‘NodeStatus’ in scope”, even though I have the code for it https://github.com/reez/Monday/pull/88/files#diff-1f8112828f9e05100c7ec541d3485329e2f6ce3c20e634e2998cda39c41e6afeR18 , which I guess makes sense if https://github.com/tnull/ldk-node/blob/2024-05-fix-swift-builds/bindings/swift/Sources/LDKNode/LDKNode.swift hasn’t been updated in 4 months (even though code for NodeStatus has been merged into main).

Right, this is expected. We only update the checked-in files and the xcframework.zip upon the new release. IIUC, you could successfully use a local build to build and publish. I just amended the minor oversight (regarding 100.0) above.

So, if there's nothing else from your side, I'll merge this PR tomorrow. Then I'll backport it on top of 0.2.1 will release a fixed v0.2.2.

Ok sounds good, then yeah all my testing is done now until it gets released, and then I'll just make sure to test again once the new release is cut just for good measure. 🚀

@tnull tnull merged commit daf20e3 into lightningdevkit:main May 21, 2024
10 of 13 checks passed
@tnull
Copy link
Collaborator Author

tnull commented May 21, 2024

@reez Just backported these fixes and released v0.2.2 via #296. Let me know if it works for you as expected!

@reez
Copy link

reez commented May 21, 2024

@reez Just backported these fixes and released v0.2.2 via #296. Let me know if it works for you as expected!

Everything works as expected from me pulling in v0.2.2 remotely, build+run+archive+upload 🚀 reez/Monday#89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if we also need to apply Info.plist fixes Swift builds are broken on macOS
2 participants