Cleanup Build Script, Support XCframeworks

[jurvis]

Reopened of #61. Resolves #59.

Purpose

This PR primarily adds support for running apps that use Libwally-Swift in iOS simulator on M1 Macs. This is achieved by using xcframeworks, which will allow us down the road to support Mac Catalyst targets and publish it on Swift Package Manager.

Implementation

We heavily lean towards more modern approaches to packaging frameworks in this new iteration. Specifically, instead of using lipo which will prevent us from same having the architecture on multiple platforms in a single universal framework, we will turn to xcframeworks. Xcframeworks contain many slices organized by platform, and is platform-aware. This allows us to target more than one platform for the same architecture (e.g. arm64 ios simulator and arm64 ios device)

1. First, we introduce a new External Build System target called LibWallyCore. The role of this target is to expose environment variables that Xcode passes to us via xcodebuild (either via the IDE or CLI) to any arbitrary build process. This allows us to not have to manage architecture-related logic within our shell script, and only build for the architectures relevant for our users.
2. In order to back the External Build System with our own build process of compiling libwally-core, build-libwally.sh needed to be reworked so that it only does one thing: produce a C static library. It is the shell script that the aforementioned target calls when you hit "Build" or "Archive".
3. Framework generation has been moved to build-libwally-swift.sh. This script exists solely for Cocoapods to build xcframeworks for a developer's local dev environment.

You can see the benefit of our new build system in action in build-libwally-swift.sh, where all we need to do is to call Xcode to build the LibWally scheme. And since it lists LibWallyCore as a dependent target, automatically builds the correct framework for the user.

Demo

User Perspective

With SwiftPM:

With Cocoapods:

Developer's Perspective

Testing Notes

• [x] Swift Package Manager (https://github.com/jurvis/LibWallySwiftPMSample)
• [x] Cocoapods Install (https://github.com/jurvis/LibwallyCocoapodsSample)

Follow-up

~- Add Swift Package Manager support (?)~

• Now that we use xcframeworks, we can probably support Mac Catalyst too (https://github.com/Sjors/libwally-swift/issues/41)

[jurvis]

ack, forgot to mark this as draft. Still need to work on Cocoapods/Carthage support

[jurvis]

@Sjors managed to get Swift Package Manager working by following these steps and uploading a zipped version of the xcframework as a Github release with the following updates to Package.swift.

You can try by installing https://github.com/jurvis/libwally-swift as a dependency on any project. Caveat: SPM seems to not like the v prefix at all. We may have to consider dropping it.

[jurvis]

Still stumped by Cocoapods, opened an issue here https://github.com/CocoaPods/CocoaPods/issues/11309.

We will probably use this build method for Carthage.

[Sjors]

I'll try to get Swift package manager to work for nthKey. I'm tempted to just drop CocoaPods and Carthage if they can't be fixed, but let's try.

SPM seems to not like the v prefix at all. We may have to consider dropping it.

Sad, but possible of course.

Using binary distribution doesn't seem like a good idea, unless they support deterministic builds. Can't Swift packages be distributed as source code? Update I think it does: https://developer.apple.com/documentation/swift_packages/identifying_binary_dependencies

[jurvis]

@Sjors yeah, every time you cut a release using a binary distribution, you will need to run swift package compute-checksum path/to/MyFramework.zip to compute a checksum to be specified in Package.swift.

When installing the package, Xcode checks and throws you a warning/error if they don't match.

[jurvis]

We can look into distributing Swift packages as source, but that will probably involve finding a way to configure the Xcode project to run what we define ./build-libwally.sh to do. Probably a follow-up PR if we want to go down that route.

[Sjors]

When installing the package, Xcode checks and throws you a warning/error if they don't match.

But that only checks the binary of the checksum, and doesn't prove that binary is derived from the source, right?

Probably a follow-up PR if we want to go down that route.

That's fine, but in that case we have to make sure CocoaPods still works after this PR, because I'm not going to consume binary packages in my own project :-)

[jurvis]

But that only checks the binary of the checksum, and doesn't prove that binary is derived from the source, right?

I want to say it does, mostly because the full compute-checksum command requires a package-path to be passed in as a parameter:

swift package --package-path /path/to/libwally-swift compute-checksum LibwallySwift.xcframework.zip

When running within the libwally-swift directory, it isn't necessary:

swift package compute-checksum LibwallySwift.xcframework.zip

I can ask around and see if I can get confirmation on this 😄

[Sjors]

The checksum of a zip file only tells you what is in the zip file. Even if the zip file contains both code and a binary, the checksum doesn't prove that the binary is actually generated from the code.

[jurvis]

@Sjors ah, gotcha. I see your concern now. That makes sense. I'll keep tabs on the Cocoapods issue and update this PR to keep it working 😄

[jurvis]

hey @Sjors I don't know how long we'll have to wait until we get back a response on our GitHub issue, I figured it may just be quicker if I figured out how to use Xcode to run an external build config so we won't need to manually call build-libwally.sh and handle every possible architecture we want to support.

in essence, Github projects will use the external build config target to pass a set of envvars, build the libraries it needs, and produce the framework necessary. I'm still working on it while we wait in this forked repo, but it's already opened up an opportunity for us to get GitHub CI in. (see: https://github.com/jurvis/libwally-swift/actions/runs/2200395095)

[jurvis]

@Sjors fixed Cocoapods. I also updated the PR to include a more detailed implementation description alongside some new demo screenshots. I also attached two sample projects, with Cocoapods and SwiftPM, that you can clone to try.

[jurvis]

@Sjors thanks for the review! I'll take a look at the build errors on your nthKey PR and see if I can reproduce it. fwiw, my CI PR uses a x86_64 box but I can borrow my girlfriend's Intel Mac to test 😄

[Sjors]

@jurvis your Podfile example might be easier to use for reproducing the issue.

I should add that I was able to build for the simulator and run the tests.

[jurvis]

@jurvis your Podfile example might be easier to use for reproducing the issue.

I should add that I was able to build for the simulator and run the tests.

Oh okay. That's actually really helpful to know. I think it explains in part why the GitHub action for building for the device fails. I'll take care of it!

[jurvis]

@Sjors if it's not too much trouble, can you post your config.log in a gist for me to look at?

[Sjors]

@jurvis here you go: https://termbin.com/9m17

[jurvis]

@Sjors I managed to get it fixed! tested on an Intel Mac as well. go ahead and change the branch in your Podfile for nth-key and the build should work now

apologies for the confusing branching -- I keep messing up a rebase and ended up with a spiderweb of branches

[jurvis]

@Sjors lmk when it's good to cleanup and squash some of these commits; I wanted to keep them around so it's easier for you to review.

[Sjors]

lmk when it's good to cleanup and squash some of these commits

Just squash them whenever you like, I can use git (range)-diff to compare.

[jurvis]

@Sjors I think it should work for real now.

turns out the issue was with Cocoapods asking for support for multiple architectures (arm64 and x86_64) on Intel macs, which makes sense, since you'll need the arm64 version to archive for iOS devices and x86_64 for the simulator.

I needed to modify the script to essentially do almost the same thing as the old one by lipo-ing those two potential architectures. In Apple Silicon Macs, this wouldn't matter, because we'll only be building for one architecture -- arm64.

[Sjors]

Thanks, this worked!

Let's clean up the PR just a little bit more, so in the future it's easier to understand what changed.

Can you drop 8884c21f1cbd05e2db8271f201161b250094bbc6? Adding a Swift package can be done in a separate PR, and it hardcodes a binary in your fork.

Can you squash a255ccb72f6f21ed1a2fbb43c2715ab746dc55d7 into the previous two commits?

I also get the impression that 2dba1acb6b0c1485850947ca63406cf9f47920dd is changing some of the things that were introduced in earlier commits. However, I'm not sure if squashing all of it is a good idea, since this PR is a rather big change.

Ideally each commit leaves the project in a working state, though that may be too much to ask with such a big change in the build process.

[jurvis]

Can you drop https://github.com/Sjors/libwally-swift/commit/8884c21f1cbd05e2db8271f201161b250094bbc6? Adding a Swift package can be done in a separate PR, and it hardcodes a binary in your fork.

Sure thing! Reopened at #73

Can you squash https://github.com/Sjors/libwally-swift/commit/a255ccb72f6f21ed1a2fbb43c2715ab746dc55d7 into the previous two commits?

Done

I also get the impression that https://github.com/Sjors/libwally-swift/commit/2dba1acb6b0c1485850947ca63406cf9f47920dd is changing some of the things that were introduced in earlier commits. However, I'm not sure if squashing all of it is a good idea, since this PR is a rather big change.

yeah, I deliberately left this one separate because it changes some of the static lib import behaviours in Xcode.

[Sjors]

Why does the last commit message say "Delete Package.swift from pbxproj"?

[jurvis]

@Sjors oh, it was my Git client that added it there. I was cleaning up the SPM commit and realized Xcode didn't remove the file import. I'll nix the commit message. all the trial-and-error has gotten me confused - my bad 😅

[Sjors]

Thanks, I'll do one more quick test and then merge this thing...

[Sjors]

One more issue... if you build for the simulator, e.g. to run the test suite, and then for the device, it complains:

Cleaning the build folder is not enough to make that go away. I had to do a git clean -dfx in the libwally-core submodule. That's probably too much to ask. Maybe we should put the build artefacts elsewhere?

Also, unrelated, and can wait for a followup, it might be a good idea to switch src/secp256k back to the correct commit after we finish building, so the submodule is not in a changed / dirty state.

[jurvis]

@Sjors yeah, unfortunately, the only way to get it to work is to cd into libwally-core and run make clean. I don't think it's too much to ask at all, let me try and figure out how to do this quickly.

Also, unrelated, and can wait for a followup, it might be a good idea to switch src/secp256k back to the correct commit after we finish building, so the submodule is not in a changed / dirty state.

I'll file a new issue for this, I will prefer to close this earlier because I'll be busy over the next couple of days and don't want to block.

[jurvis]

@Sjors I just thought about this a little more, I don't think we'll need to do anything too complicated.

I just turned off "Build Active Architectures Only" so Xcode knows to build for all supported architectures. In the case of dev, I believe it will be x86_64 and arm64. Sure, it'll take longer since we're building the framework to support both more architectures, but it's only for dev (and setting -destination in build-libwally-swift.sh will set the supported architecture it needs accordingly), so it's probably fine.

fwiw, build still green on CI: https://github.com/jurvis/libwally-swift/actions/runs/2261318103

[Sjors]

@jurvis building both architectures is fine by me. I'll test again.

[Sjors]

Still getting that error. First I run the tests on the simulator, and then I try to build for a device:

I don't think it's actually building for the device:

[jurvis]

@Sjors thanks for that! I was able to reproduce. I just tried adding my build artifact cleanup code snippet to the scheme's pre-build script instead.

can you try again? let me know how you'll like the commits squashed, I don't have an opinion either way.

[Sjors]

It worked!

[jurvis]

@Sjors yay! thank you so much for your patience in review this. I will rebase the changes on #75 so we can get some nice CI running for the project :)