search

urbanairship / ios-library

Urban Airship iOS SDK
http://urbanairship.com
Apache License 2.0
478 stars 265 forks source link

To reduce impact on build times, provide an alternate Airship Swift package that excludes Airship source in favor of xcframeworks #347

Closed mpsmithhatch closed 1 year ago

mpsmithhatch commented 1 year ago

Preliminary Info

What are the versions of any relevant development tools you are using?

Swift Package Manager (SPM) Swift 5.7+, Xcode 14.2

Report

What unexpected behavior are you seeing?

When using SPM to bring the Airship Swift Package dependency into a project, the entire source tree is copied in the SPM user cache, and Airship is compiled from source any time one performs a clean build.

I don't actually want to build Airship source at all. I would like to depend on pre-built release artifacts (xcframeworks) ideally.

However, I would still like to use SPM for declaring that dependency.

There are some examples of how others have approached this in the wild: Airbnb recently started providing a lottie-spm git repo which is explicitly for the purpose of having a faster downloading Swift package which brings in released Lottie binary xcframeworks from releases assets in their main GitHub repo's releases. Another example is this repo which mirrors Firebase releases, generating a 'wrapper' Swift package that points at pre-built binary xcframeworks hosted in Github release assets.

What is the expected behavior?

I would like the ability to still use SPM, but point at an Airship maintained Swift package that brings in xcframeworks via binaryTarget's. This way, my local Xcode won't ever be cloning the entire Airship repo (including source) or spending time compiling that source.

What are the steps to reproduce the unexpected behavior?

Set up a sample project with a spm dependency to urbanairship/ios-library. It clones the entire repo into the spm user level cache. Build that project and it compiles Airship.

Sherlouk commented 1 year ago

Was about to open a similar issue, but glad to see other folks wanting the same behaviour.

We've recently introduced XCMetrics into our workflow and Airship is one of the largest dependencies impacting our clean build times - we'd hope to see a great reduction from it adopting xcframeworks.

rlepinski commented 1 year ago

This is great feedback, its something we should be able to provide since we already xcframeworks. Based on our release, a separate repo that wraps the released xcframeworks is probably the approach we will go with. I am just not sure on when we will get to it at the moment.

@Sherlouk Make sure you are on at least 16.10.1 or newer. We had a compiler issue with swiftui that made our build incredibly slow. Build times should be close to around ~30 seconds for Airship. Still slow I know, but it was 130 seconds in a few builds.

Sherlouk commented 1 year ago

@Sherlouk Make sure you are on at least 16.10.1 or newer. We had a compiler issue with swiftui that made our build incredibly slow. Build times should be close to around ~30 seconds for Airship. Still slow I know, but it was 130 seconds in a few builds.

Thanks for the tip, though we are on 16.10.6 so unlikely to be the problem here 😟

rlepinski commented 1 year ago

Sorry for the delay on this, we just started on it and will hopefully have something in the next week or two

rlepinski commented 1 year ago

@Sherlouk @mpsmithhatch https://github.com/urbanairship/ios-library-prebuilt

Right now its only for 17.x

Sherlouk commented 1 year ago

Thanks Ryan, will give it a go.

mpsmithhatch commented 1 year ago

Just saw this. Thanks @rlepinski !

We're still on 16.x as well, so I won't be able to thoroughly test this yet.

I did drop it in to our project Package.swift's real quick to see what popped, and it looks like mostly the things listed here: https://github.com/urbanairship/ios-library/blob/main/Documentation/Migration/migration-guide-16-17.md

If you can provide a 16.x version, we can probably adopt and test this a lot sooner. However, if that's too much trouble to get a 16.x version of this going, I'll bundle attempting to adopt this into when we upgrade to and test 17.x.