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

CI: Run tests on all platforms and verify build #10

Merged
merged 8 commits into from
Mar 18, 2024
Merged

Conversation

bisgardo
Copy link
Contributor

@bisgardo bisgardo commented Mar 13, 2024

This library is not Mac/iThing specific. UniFFI supports other languages that work on all platforms. Furthermore, Swift actually supports Linux and we might want the SDK to support that platform one day.

It therefore seems reasonable to avoid unknowingly adding anything that breaks multiplatform support in this library. Hence we should also test the Rust code on Ubuntu and Windows.

We also run on all available macOS runners, including building the actual Swift package, to gain assurance that older versions work as well. With this, we found that a few changes were required for supporting older versions of Swift (back to the version running on the oldest Mac-based GitHub Actions runner macos-11, which is an Intel Mac):

  • Explicitly import Foundation in Package.swift for using ProcessInfo (for reading env vars).
  • Downgrade swift-tools-version to v5.5 in Package.swift.
  • Renamed to ConcordiumWalletCryptoUniffi.xcframework to match the (similarly renamed) binary target in Package.swift. We cannot just rename the target to match the existing generated file because that name (ConcordiumWalletCrypto) is already used by the library product target.

To avoid wasting resources and also speed up verification of the Swift package itself, we only build the framework for the runner platform, not the full multi-platform bundle produced by make framework.

The library is not Mac/iThing specific. UniFFI supports other languages that work on all platforms. Furthermore, Swift actually supports Linux and we might want the SDK to support that platform one day.

It therefore seems reasonable to avoid unknowingly adding anything that breaks Linux support in this library. Hence we should also test the Rust code on Windows and Ubuntu.
@bisgardo bisgardo marked this pull request as draft March 17, 2024 11:28
We use `ProcessInfo`, which is part of `Foundation`, to read env vars in the package spec.

The package looks to be automatically imported in newer versions of Swift/macOS, but to support older ones we need to do it explicitly.
Rename generated framework to `ConcordiumWalletCryptoUniffi.xcframework` to match the (similarly renamed) binary target in `Package.swift`. We cannot just rename the target to match the existing generated file because that name (`ConcordiumWalletCrypto`) is already used by the library product target.

This is a requirement that apparently has been lifted in newer versions of Swift/macOS, but to support older versions, we follow it.
For the purpose of verifying that the library builds on CI, there's no point in building the binary framework for all platforms, just for the build platform itself.

We take into account that some ("older") runners are Intel macs while others ("newer") run on Apple Silicon.
@bisgardo bisgardo changed the title CI: Run tests on all platforms CI: Run tests on all platforms and verify build Mar 17, 2024
@bisgardo bisgardo marked this pull request as ready for review March 17, 2024 21:14
@bisgardo bisgardo merged commit 955b2fe into main Mar 18, 2024
6 checks passed
@bisgardo bisgardo deleted the ci/multiplatform branch March 18, 2024 12:26
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.

2 participants