Extract fastlane automation to share it between SDKs by tonidero · Pull Request #1719 · RevenueCat/purchases-ios

tonidero · 2022-06-20T12:34:04Z

Description

https://revenuecats.atlassian.net/browse/CSDK-27

In this PR we separate part of the fastlane automation currently in Fastfile to a separate CommonFastfile. This file will be extracted to its own repo in a future PR.

The idea here is to reuse the automation we currently have on the iOS SDK in all our other SDK repos (as much as possible).

Some of the challenges I had to deal with were:

The bump lane used to recreate RevenueCat-Swift.h, which is an iOS only task. In order to extract bump, I had to remove that part from that lane. Instead I added a check to the release-checks lane to make sure it was updated.
The common lanes require some specific configuration per repo. In order to avoid wrapping those lanes in other lanes within the main Fastfile, I currently set this data as environment variables. Not a 100% sure about this solution, but I think it should be mostly ok.

TODO

Test github interactions (create branch, commit, push, create PR,...). The parameters seem to be correct but I didn't want to create release branches and PRs without alerting anyone. I plan to test these with an android release tomorrow.

… creating autogenerated header lane public. Remove updating header from bump task

tonidero · 2022-06-20T12:35:25Z

+
+  create_pull_request(
+    title: "Release/#{new_version_number}",
+    base: "main",


This doesn't really work for hotfixes. But I think this is ok for now, we can add more automation in the future. This is how it worked until now in the iOS repo

tonidero · 2022-06-20T12:36:14Z

+  next_version_snapshot = "#{next_version}-SNAPSHOT"
+
+  org = "RevenueCat"
+  repo = ENV["REPO_NAME"]


Note that now we have to pass the name of the repo. Currently using environment variables for this.

can we add a prompt with a selector (and default timeout for no-entry) in the event that the ENV doesn't contain anything?

You mean using UI.input to have the dev input the repo name? I can do that, but I wonder if it's necessary... In my mind, these ENVs would be set in the Fastfile on each SDK so devs don't ever need to input anything for these. If you think there is a use case where we could get here without setting these environment variables I can do that though, and in that case we might want to do the same for the other environment variables.

I was thinking in the infrequent case that we need to run this on our local machine we don't want to fiddle with ENV stuff. What you describe here, though makes sense, and it seems like we'd never have to manually set these during a manual deploy.

TLDR: my comment can be ignored 😄

tonidero · 2022-06-20T12:38:20Z

+  ../Examples/MagicWeatherSwiftUI/Magic Weather SwiftUI.xcodeproj/project.pbxproj,
+  ../scripts/docs/index.html
+"
+ENV["GITHUB_RELEASE_UPLOAD_ASSETS"] = "RevenueCat.framework.zip,RevenueCat.xcframework.zip"


Not sure if this is the best place to setup all these environment variables. They aren't secret or anything so I think they are ok here, but lmk if you have any other ideas.

Nah, I think that's fine

tonidero · 2022-06-20T12:42:15Z

-
-    files_to_update = [
-      '../RevenueCat.podspec',
-      '../Sources/Purchasing/Purchases.swift',


This location was outdated (File seems to have been moved to ../Sources/Purchasing/Purchases/Purchases.swift). Seems like that change happened a few days ago. However, I couldn't see the current version number on that file (we do get the frameworkVersion programmatically though) so I removed it from the list. Lmk if you think I should readd it.

Great catch! /cc @taquitos

Huh, would ya look at that? Nice find!

tonidero · 2022-06-20T12:47:26Z


  desc "Creates RevenueCat-Swift.h for a new release"
-  private_lane :compile_autogenerated_header do |options|
+  lane :compile_autogenerated_header do |options|


I made this public in case a dev wants to recreate RevenueCat-Swift.h header, which might be useful if we now catch this as an error during the release-checks lane.

tonidero · 2022-06-20T12:48:13Z

 # Available Actions

-### check_pods
+### bump


The order of these actions has changed... Not sure if that's a problem. It does include the actions from CommonFastfile though.

Order shouldn't matter 👍🏻

tonidero · 2022-06-20T12:58:37Z

+/// they should not be used for managing secure or sensitive information such as subscription status, coins, etc.
+/// Key names starting with “$” are reserved names used by RevenueCat. For a full list of key restrictions refer
+/// <a href="https://docs.revenuecat.com/docs/subscriber-attributes">to our guide</a>
+SWIFT_CLASS_NAMED("Attribution")


This had quite a few changes due to some of the recent changes in main. I updated it here so we don't forget later. This is the result of running the compile_autogenerated_header lane without any changes.

This gets generated automatically for every release, so we're not going to forget 😅
I'd recommend reverting it, because it helps to look at this diff when we ship a new release to see every change included.

As part of the changes on this PR, this won't be generated automatically moving forward, but we will have a test on the release-checks lane that will fail if this file is outdated (More conversation on that here: https://revenuecat.slack.com/archives/C03HPN9UE80/p1655465010272579). Devs can also generate it with the lane compile_autogenerated_header, made available on this PR. That said, going to revert these changes here so they can be added once we make a release.

Hmm why can't we call this lane from fastlane bump?
Considering we always need to update it, we're going to forget so seems like a point of friction when making releases.

Right, I wasn't sure about this and asked it over Slack. Basically the idea is to make the bump lane platform independent. Other platforms don't need to update that header, so I removed it from that lane. We have some alternatives if we want to have it, like wrapping the bump lane in another iOS only lane, or add some kind of placeholder lane to be implemented by each platform... We could also not share the bump lane, but seems like a lost opportunity. Honestly I didn't like those options too much but maybe having a bump_ios lane wrapping the bump lane would be ok?

like wrapping the bump lane in another iOS only lane,

Yeah that's exactly what I had in mind. We can share most of the code, but some platform specific things will always be needed.

Sounds good! Will do that tomorrow. Thanks!

tonidero · 2022-06-20T13:00:08Z

I believe test failures are not due to these changes.

tonidero · 2022-06-20T13:02:16Z

+end
+
+def current_version_number
+  File.read("../.version").strip


Note how this checks for the existance of a .version file in the root folder. This is not the case in most of our repos. However, I think it's a good pattern to follow to have a common versioning system, since Android, iOS, Flutter,... each have their own versioning systems that would be tricky to abstract. So I propose following that pattern in all our SDKs.

💯💯💯

tonidero · 2022-06-20T13:05:55Z

I think this can be reviewed now, but I will hold off merging until I can test a proper release with these changes.

tonidero · 2022-06-20T13:11:30Z

-  sh("sed", '-i', backup_extension, sed_regex, path)
-end
-
-def current_version_number


Also note that this is still used on the main Fastfile. Importing CommonFastfile also makes those utility functions available here. I thought about making that a lane... But I wasn't sure. Lmk if you think it should be a lane.

I don't think they need to be a lane.

NachoSoto · 2022-06-21T16:52:51Z

+/// they should not be used for managing secure or sensitive information such as subscription status, coins, etc.
+/// Key names starting with “$” are reserved names used by RevenueCat. For a full list of key restrictions refer
+/// <a href="https://docs.revenuecat.com/docs/subscriber-attributes">to our guide</a>
+SWIFT_CLASS_NAMED("Attribution")


This gets generated automatically for every release, so we're not going to forget 😅
I'd recommend reverting it, because it helps to look at this diff when we ship a new release to see every change included.

NachoSoto · 2022-06-21T16:54:54Z

-
-    files_to_update = [
-      '../RevenueCat.podspec',
-      '../Sources/Purchasing/Purchases.swift',


Great catch! /cc @taquitos

NachoSoto · 2022-06-21T16:55:28Z

 # Available Actions

-### check_pods
+### bump


Order shouldn't matter 👍🏻

This reverts commit 139e033.

…tead

tonidero · 2022-06-22T10:26:29Z

-    previous_version_number = current_version_number
-
-    version_number_without_prerelease_modifiers = new_version_number.split("-")[0]
-    increment_version_number(version_number: version_number_without_prerelease_modifiers)


I hadn't noticed this was an iOS only operation. This increments the version in the xcodeproj in a smarter way, using some specialized tools.

I substituted this with our more "manual" version replacement (changing all occurences of the old version number with the new one). I tested it a few times, and it seems to generate the same changes, but lmk if you don't like that and we can keep this task as sdk-dependant

A more platform-agnostic approach like you describe makes sense to me. In the future it might even make sure to extract out ENV["FILES_TO_UPDATE_VERSION_STRING"] into separate file that gets loaded.

tonidero · 2022-06-22T11:13:51Z

I tested the bump lane on the purchases-android repo pointing to these changes. The changes to support this were: RevenueCat/purchases-android#550 and the created release PR looked like: RevenueCat/purchases-android#549

…dates to shared process

tonidero · 2022-06-23T09:50:24Z

 GITHUB_PULL_REQUEST_TEAM_REVIEWERS=
+
+# Optional for updating changelog. Can be the same as GITHUB_PULL_REQUEST_API_TOKEN and improves issues with github api rate limits.
+GITHUB_TOKEN=


Adding this token avoided the rate limit issues I had when testing this, so I added it here, since it can be useful.

❤️

@joshdholtz also told me you can use GITHUB_RATE_LIMIT_SLEEP=5.

tonidero · 2022-06-23T09:54:57Z

  end

+  desc "Bump version, update swift header, edit changelog, and create pull request"
+  lane :bump_ios do |options|


From the conversation in #1719 (comment). This new lane uses the bump lane which is platform agnostic and adds a new step which will regenerate the RevenueCat-Swift.h header and commit and push if there were changes. Note that this will happen on a separate commit from the version bump changes, I added the version number to the commit name though, so it's easier to track changes on that file.

But it'll be in the same PR right?

That's correct!

tonidero · 2022-06-23T09:55:50Z

  lane :release_checks do |options|
    version_number = current_version_number
    check_no_git_tag_exists(version_number)
+    check_autogenerated_header_up_to_date


I'm keeping this check even after the addition of the bump_ios lane since I think it will be useful to make sure we don't forget to update it on a release

tonidero · 2022-06-27T10:37:42Z

@RevenueCat/sdk bump

NachoSoto

Just a couple small things, but I think this is ready.

NachoSoto · 2022-06-27T20:29:17Z

 #### Releasing:
 1. Create a `fastlane/.env` file with your GitHub API token (see `fastlane/.env.SAMPLE`). This will be used to create the PR, so you should use your own token so the PR gets assigned to you. 
-2. Run `bundle exec fastlane bump`
+2. Run `bundle exec fastlane ios bump_ios`


ios bump_ios seems redundant. Couldn't the "wrapped" lane also be called bump?

I couldn't call the common bump lane directly, since they would share the name and end up in an inifinite loop. I thought about a couple options, like calling the terminal fastlane command from the fastfile with sh or moving the logic to a different method that can be shared by the common bump lane and the ios bump lane, but I thought those weren't clean. In the end, I renamed the common bump lane to bump_version_update_changelog_create_pr. With this, each platform can create their own bump lane that is just a wrapper around the bump_version_update_changelog_create_pr lane, which I think is fine and if we have to add platform specific logic in the future is probably cleaner.

NachoSoto · 2022-06-27T20:30:02Z

 GITHUB_PULL_REQUEST_TEAM_REVIEWERS=
+
+# Optional for updating changelog. Can be the same as GITHUB_PULL_REQUEST_API_TOKEN and improves issues with github api rate limits.
+GITHUB_TOKEN=


❤️

@joshdholtz also told me you can use GITHUB_RATE_LIMIT_SLEEP=5.

NachoSoto · 2022-06-27T20:31:56Z

+  org = "RevenueCat"
+  repo = ENV["REPO_NAME"]
+  UI.user_error!("missing repo name env") unless repo


This is duplicated 3 times.

NachoSoto · 2022-06-27T20:32:23Z

+    UI.user_error!("Please add a CHANGELOG.latest.md file before calling this lane")
+  end
+  commit_hash = last_git_commit[:commit_hash]
+  puts commit_hash


I wonder if we left this out accidentally (I see it's in the original one too).

Probably, I will remove it, since I don't think it's that useful 👍

NachoSoto · 2022-06-27T20:33:27Z

+    bump(options)
+    compile_autogenerated_header_commit_and_push(options)


NachoSoto · 2022-06-27T20:33:36Z

  lane :release_checks do |options|
    version_number = current_version_number
    check_no_git_tag_exists(version_number)
+    check_autogenerated_header_up_to_date


NachoSoto · 2022-06-27T20:33:50Z

+    begin
+      ensure_git_status_clean
+    rescue => e
+      UI.error("RevenueCat-Swift.h autogenerated file changed. Make sure it's up-to-date before releasing.")


…e. This is needed in iOS since it requires extra steps. Other projects can just wrap the new longer name into a bump lane for ease of use

https://revenuecats.atlassian.net/browse/CSDK-27 Added a `Fastfile` containing all the shared automation logic between our SDKs. This PR is just moving the `CommonFastfile` in iOS that was added here: RevenueCat/purchases-ios#1719. Once this is merged, I will remove the one in the iOS repo and import this one there.

Looks like this was changed in #1719 but not updated.

…r_up_to_date` This was added in #1719, but unfortunately [it fails when calling `ensure_git_status_clean`](https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/7379/workflows/f8feaf5d-42f7-4154-8782-9c2f1d36aa6b/jobs/30726). So since the header is already automatically generated, we can just go back to the previous behavior and not call this.

Fixes https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/7382/workflows/91dd2e3a-9244-405e-b04c-44d2a4de047f/jobs/30748 With #1719 this was importing `CommonFastfile`. However, some release scripts have a different working directory, like `Tests/InstallationTests/CarthageInstallation`, that `import` the root `Fastfile`. When that happens, this `import` was being evaluated from that working directory, and failing to find `CommonFastfile`.

…r_up_to_date` (#1747) This was added in #1719, but unfortunately [it fails when calling `ensure_git_status_clean`](https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/7379/workflows/f8feaf5d-42f7-4154-8782-9c2f1d36aa6b/jobs/30726). So since the header is already automatically generated, we can just go back to the previous behavior and not call this.

#1748) Fixes https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/7382/workflows/91dd2e3a-9244-405e-b04c-44d2a4de047f/jobs/30748 With #1719 this was importing `CommonFastfile`. However, some release scripts have a different working directory, like `Tests/InstallationTests/CarthageInstallation`, that `import` the root `Fastfile`. When that happens, this `import` was being evaluated from that working directory, and failing to find `CommonFastfile`.

tonidero added 3 commits June 20, 2022 11:42

Add check for up to date autogenerated header on release checks. Make…

9a940ef

… creating autogenerated header lane public. Remove updating header from bump task

Extract common lanes to CommonFastfile for reusability

9dde54d

Update

139e033

tonidero commented Jun 20, 2022

View reviewed changes

tonidero marked this pull request as ready for review June 20, 2022 13:06

tonidero requested a review from a team June 20, 2022 13:06

tonidero commented Jun 20, 2022

View reviewed changes

NachoSoto reviewed Jun 21, 2022

View reviewed changes

NachoSoto requested a review from a team June 21, 2022 16:56

Revert "Update"

e8968d7

This reverts commit 139e033.

tonidero self-assigned this Jun 22, 2022

tonidero added 2 commits June 22, 2022 11:43

Fix incorrect variable name in check

0041c9d

Remove increment_version_number task and change versions manually ins…

630ff0d

…tead

tonidero force-pushed the csdk-27 branch from d09f4d1 to 630ff0d Compare June 22, 2022 10:21

tonidero commented Jun 22, 2022

View reviewed changes

tonidero requested review from NachoSoto and taquitos June 22, 2022 11:15

Added bump_ios lane that will also update RevenueCat-Swift.h. Some up…

370f978

…dates to shared process

tonidero commented Jun 23, 2022

View reviewed changes

NachoSoto approved these changes Jun 27, 2022

View reviewed changes

tonidero added 2 commits June 29, 2022 15:39

Fixes from review

38fa6ab

Rename common bump lane so it does not collide with projects bump lan…

08e81c3

…e. This is needed in iOS since it requires extra steps. Other projects can just wrap the new longer name into a bump lane for ease of use

tonidero merged commit f752582 into main Jun 30, 2022

tonidero deleted the csdk-27 branch June 30, 2022 08:40

tonidero mentioned this pull request Jun 30, 2022

Create common sdk Fastfile RevenueCat/fastlane-plugin-revenuecat_internal#1

Closed

NachoSoto added a commit that referenced this pull request Jun 30, 2022

RELEASING.md: fixed bump lane name

aef73de

Looks like this was changed in #1719 but not updated.

NachoSoto mentioned this pull request Jun 30, 2022

RELEASING.md: fixed bump lane name #1744

Merged

NachoSoto added a commit that referenced this pull request Jun 30, 2022

RELEASING.md: fixed bump lane name (#1744)

458b876

Looks like this was changed in #1719 but not updated.

NachoSoto mentioned this pull request Jul 1, 2022

fastlane release_checks: removed call to check_autogenerated_header_up_to_date #1747

Merged

NachoSoto mentioned this pull request Jul 1, 2022

Fastfile: changed CommonFastfile import relative to current file #1748

Merged

tonidero mentioned this pull request Jul 4, 2022

Fix replace version without prerelease modifiers #1751

Merged

		bump(options)
		compile_autogenerated_header_commit_and_push(options)

Uh oh!

Conversation

tonidero commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero Jun 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero commented Jun 20, 2022

Uh oh!

tonidero Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero commented Jun 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero commented Jun 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonidero commented Jun 20, 2022 •

edited

Loading

tonidero Jun 22, 2022 •

edited

Loading

tonidero Jun 20, 2022 •

edited

Loading