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

export RevenueCatUI xcframework #4172

Merged
merged 12 commits into from
Aug 15, 2024
Merged

export RevenueCatUI xcframework #4172

merged 12 commits into from
Aug 15, 2024

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Aug 12, 2024

We don't currently export RevenueCatUI as an xcframework, which can be a source of confusion and a dealbreaker for customers who need a pre-built framework.
This adds RevenueCatUI's xcframework into the export_xcframeworks lane, and includes the output in the github_release lane.

Should help address:

@aboedo aboedo added the ci label Aug 12, 2024
@aboedo aboedo self-assigned this Aug 12, 2024
@aboedo
Copy link
Member Author

aboedo commented Aug 12, 2024

@RCGitBot please test

@aboedo aboedo mentioned this pull request Aug 12, 2024
@aboedo aboedo force-pushed the andy/revenuecatui_xcframework branch from bbd776e to e811671 Compare August 12, 2024 21:21
@aboedo aboedo changed the base branch from main to andy/fix_tests August 12, 2024 21:21
aboedo added a commit that referenced this pull request Aug 13, 2024
Fixes the tests that were failing in `main`, which you can see in
#4172
Base automatically changed from andy/fix_tests to main August 13, 2024 18:59
@hansemannn
Copy link

@aboedo Thank you for this? Is there a chance to grab a binary from this PR already? It's quite a show stopper for us this week. Many thanks!

@aboedo aboedo force-pushed the andy/revenuecatui_xcframework branch 2 times, most recently from dd98303 to b37920e Compare August 14, 2024 19:29
@aboedo
Copy link
Member Author

aboedo commented Aug 14, 2024

@hansemannn apologies, the approach I tested sadly didn't quite work, but I'm actively trying to figure out why the output format isn't quite correct

@aboedo aboedo force-pushed the andy/revenuecatui_xcframework branch from 13c90ab to 6eaeb9e Compare August 14, 2024 22:44
Comment on lines -6460 to +6462
SKIP_INSTALL = YES;
SKIP_INSTALL = NO;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not strictly needed to have these set correctly since the fastlane lane does set them, but it's also a good idea to have them so that if we want to archive manually it works correctly.

SKIP_INSTALL should be NO for frameworks, and BUILD_LIBRARY_FOR_DISTRIBUTION should be YES

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to sanity check how these are set, sadly this is a terrible way to try to view it but you can go into xcode and look for the settings under build settings, and check them in the targets, i.e.:

image image

Comment on lines +43 to +49
// wrapping with AnyView to type erase is needed because when archiving an xcframework,
// the compiler gets confused between the types returned
// by the different implementations of self.onChange(of:value).
if #available(iOS 17.0, macOS 14.0, tvOS 17.0, watchOS 10.0, *) {
self.onChange(of: value) { _, newValue in action(newValue) }
AnyView(self.onChange(of: value) { _, newValue in action(newValue) })
} else {
self.onChange(of: value) { newValue in action(newValue) }
AnyView(self.onChange(of: value) { newValue in action(newValue) })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one drove me nuts and did not have a nice error message, fun times

case add
case plus
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

talk about driving me NUTS. having it named add creates a collision with a UIKit name, but the collision happens when Swift auto-generates intermediate files, and has a terrible error message.

lane :export_xcframework do |options|
lane :export_xcframeworks do |options|
create_and_sign_xcframework(scheme: 'RevenueCat', product_name: 'RevenueCat')
create_and_sign_xcframework(scheme: 'RevenueCatUIDev', product_name: 'RevenueCatUI')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only note here: I had to use the RevenueCatUIDev scheme, not RevenueCatUI because that one doesn't archive correctly for some reason. That one is tied to SPM. I intend to remove that scheme altogether and rename RevenueCatUIDev -> RevenueCatUI to prevent confusion, but one thing at a time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the product name determines the actual xcframework name that gets sent out

create_and_sign_xcframework(scheme: 'RevenueCatUIDev', product_name: 'RevenueCatUI')
end

private_lane :create_and_sign_xcframework do |options|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this part is just extracting common logic

@aboedo aboedo force-pushed the andy/revenuecatui_xcframework branch from 6eaeb9e to 3f4c171 Compare August 14, 2024 22:50
@aboedo
Copy link
Member Author

aboedo commented Aug 14, 2024

tested using the xcframework that this generates and it works correctly

@aboedo aboedo marked this pull request as ready for review August 14, 2024 22:55
@aboedo
Copy link
Member Author

aboedo commented Aug 14, 2024

@hansemannn I'm re-running manually and will upload the xcframeworks that I get here so you can use them, but it should also be available on all releases once this PR is merged

@aboedo aboedo requested a review from a team August 14, 2024 23:01
@aboedo
Copy link
Member Author

aboedo commented Aug 15, 2024

welp, the whole archive is pretty huge (130mb) so I can't just upload as a comment, but I'll get this merged and add it to the latest release

@hansemannn
Copy link

Thanks a lot! Dropbox / iCloud Drive / Google Drive would also fine, but I am trying to wait 😅

Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved to unblock you, but would be good for someone with more context to have a final look!

fastlane/Fastfile Show resolved Hide resolved
fastlane/README.md Outdated Show resolved Hide resolved
@aboedo
Copy link
Member Author

aboedo commented Aug 15, 2024

@hansemannn we'll get it into the next release which should come out today or tomorrow, but here are the download links in the meantime:

@@ -35,7 +35,7 @@ struct IconView<S: ShapeStyle>: View {
/// An icon to be displayed by `IconView`.
enum PaywallIcon: String, CaseIterable {

case add
case plus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we need to make this have the value of "add" so it codables correctly with the value we use in the JSON response for paywalls 🤔 plus isn't a known icon name

Suggested change
case plus
case plus = "add"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooff right I thought it was just referencing a local file, didn't realize it needed to match a backend name. That's a bit of a problem, given the UIKit naming conflict.
Maybe we can solve it by decoupling the local property name from the key from the backend in the codable side, let me see

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol just saw that you had actually already left a suggestion for the change 🤦 I'll try to repro with a paywall before and after to make sure all things work

Copy link
Contributor

@jamesrb1 jamesrb1 Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful when you make the change that the "+" image still shows up in the live paywall if you select it as the list tick. I ran into this issue during an aborted project reorg attempt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, the image won't show up, gonna need to do something extra here, will post here again when it's solved

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved now. I don't love the solution, but the local assets are named to match the backend exactly, and the backend sends names that might conflict with UIKit, so the solution decouples the two sides.

Simulator Screenshot - iPhone 15 - 2024-08-15 at 13 18 20

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for flagging this!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good solution!

@aboedo aboedo requested a review from joshdholtz August 15, 2024 16:51
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@aboedo aboedo force-pushed the andy/revenuecatui_xcframework branch from 2590cdb to 7a38b68 Compare August 15, 2024 21:07
@aboedo aboedo enabled auto-merge (squash) August 15, 2024 21:07
@aboedo aboedo merged commit 37f3fb1 into main Aug 15, 2024
5 checks passed
@aboedo aboedo deleted the andy/revenuecatui_xcframework branch August 15, 2024 21:18
@aboedo
Copy link
Member Author

aboedo commented Aug 15, 2024

Merged! @hansemannn thanks for bringing this up and for the patience with it! I'll make sure that the next release that goes out includes the xcframework.

In the meantime, please know that there is a bug in the build that I uploaded in this branch earlier, as @joshdholtz pointed out. In that build, all things should work normally, other than if you try to use the + icon in a paywall.

The build included in the release won't have the issue

@hansemannn
Copy link

hansemannn commented Aug 15, 2024

Thanks a lot for the fast solution here - I followed it every day :) We are unblocked already by the build and Paywalls rock! Thanks to the whole team!

aboedo added a commit that referenced this pull request Aug 16, 2024
Follow up to #4172
Adds the xcframework as a circleci artifact

Also fixes a code signing issue when archiving
`RevenueCatUI.xcframework`

linear ticket: SDK-3555
aboedo added a commit that referenced this pull request Aug 16, 2024
The official docs don't say a lot about `SKIP_INSTALL`, but I missed the
mark during the #4172 PR, where the ideal setup is NO for debug, YES for
release. I spent a lot of time figuring this out a couple of years back
but the other day I completely forgot about it 🤦


<img width="1128" alt="image"
src="https://github.com/user-attachments/assets/4256ec42-00c2-4de2-a622-9a151b849781">

<img width="747" alt="image"
src="https://github.com/user-attachments/assets/c8cdf454-0558-489b-bb24-b3006a4ebacb">

Here's ChatGPT's far better explanation than Apple's: 
```
	•	Debug Configuration:
	•	SKIP_INSTALL = YES
	•	During development, you typically don’t need to install (or include) the product (e.g., .framework, .app) in the final build output. The focus is on quick builds, so setting SKIP_INSTALL = YES can help speed up the process by skipping unnecessary steps.
	•	Release Configuration:
	•	SKIP_INSTALL = NO
	•	In the Release configuration, you usually want to include everything in the final build output. This is particularly important when archiving for distribution or creating .xcframeworks, as you need the final product to be available in the archive.

Example Scenario

If you are building a framework that is used by other targets in your project:

	•	Debug Configuration:
	•	Set SKIP_INSTALL = YES so that intermediate builds are faster, and the framework isn’t unnecessarily copied to the install location.
	•	Release Configuration:
	•	Set SKIP_INSTALL = NO so that when you archive the framework, it is included in the final archive, making it available for distribution or further use.

```
joshdholtz added a commit that referenced this pull request Aug 22, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <[email protected]>
nyeu pushed a commit that referenced this pull request Oct 2, 2024
Fixes the tests that were failing in `main`, which you can see in
#4172
nyeu pushed a commit that referenced this pull request Oct 2, 2024
We don't currently export RevenueCatUI as an xcframework, which can be a
source of confusion and a dealbreaker for customers who need a pre-built
framework.
This adds RevenueCatUI's xcframework into the `export_xcframeworks`
lane, and includes the output in the `github_release` lane.


Should help address: 
- #4168
- #4146
- #4056
nyeu pushed a commit that referenced this pull request Oct 2, 2024
Follow up to #4172
Adds the xcframework as a circleci artifact

Also fixes a code signing issue when archiving
`RevenueCatUI.xcframework`

linear ticket: SDK-3555
nyeu pushed a commit that referenced this pull request Oct 2, 2024
The official docs don't say a lot about `SKIP_INSTALL`, but I missed the
mark during the #4172 PR, where the ideal setup is NO for debug, YES for
release. I spent a lot of time figuring this out a couple of years back
but the other day I completely forgot about it 🤦


<img width="1128" alt="image"
src="https://github.com/user-attachments/assets/4256ec42-00c2-4de2-a622-9a151b849781">

<img width="747" alt="image"
src="https://github.com/user-attachments/assets/c8cdf454-0558-489b-bb24-b3006a4ebacb">

Here's ChatGPT's far better explanation than Apple's: 
```
	•	Debug Configuration:
	•	SKIP_INSTALL = YES
	•	During development, you typically don’t need to install (or include) the product (e.g., .framework, .app) in the final build output. The focus is on quick builds, so setting SKIP_INSTALL = YES can help speed up the process by skipping unnecessary steps.
	•	Release Configuration:
	•	SKIP_INSTALL = NO
	•	In the Release configuration, you usually want to include everything in the final build output. This is particularly important when archiving for distribution or creating .xcframeworks, as you need the final product to be available in the archive.

Example Scenario

If you are building a framework that is used by other targets in your project:

	•	Debug Configuration:
	•	Set SKIP_INSTALL = YES so that intermediate builds are faster, and the framework isn’t unnecessarily copied to the install location.
	•	Release Configuration:
	•	Set SKIP_INSTALL = NO so that when you archive the framework, it is included in the final archive, making it available for distribution or further use.

```
nyeu pushed a commit that referenced this pull request Oct 2, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants