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

Explicitly prevent xcodebuild from changing the binary's build number #19421

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Oct 10, 2022

Without any apparent change on our side, we saw a couple of Pocket Casts builds with unexpected, and unexpectedly incrementing, build numbers on TestFlight. From 7.24.0.3, to 8, to 9.

This behavior could be replicated locally in that project by running the Fastlane automation, but not when archiving via Xcode.

I pin pointed it to the manageAppVersionAndBuildNumber option being true by default in the ExportOptions.plist that Fastlane / xcodebuild generates for gym to convert an archive into an IPA.

Explicitly setting the option to false solves the issue locally.

See Automattic/pocket-casts-ios#373 .

Something noteworthy that I didn't realize when working with Pocket Casts is that the behavior occurs only in method: 'app-store' exports.

To verify the issue on this project, checkout the current tip of the release branch, 838058f, and apply the following patch:

index 1c05a77b05..e3cb20e4ae 100644
--- a/fastlane/lanes/build.rb
+++ b/fastlane/lanes/build.rb
@@ -123,6 +123,8 @@ platform :ios do
       export_options: { method: 'app-store' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     testflight(
       skip_waiting_for_build_processing: true,
       team_id: get_required_env('FASTLANE_ITC_TEAM_ID'),
@@ -173,6 +175,8 @@ platform :ios do
       export_options: { method: 'app-store' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     testflight(
       skip_waiting_for_build_processing: true,
       team_id: '299112',
@@ -215,6 +219,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,
@@ -264,6 +270,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,
@@ -317,6 +325,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,

Run a lane to build a binary for App Store Connect (method: 'app-store') and notice how the build number is 21 instead of the 20.9.0.0 coming from config/Version.public.xcconfig.

bundle exec fastlane build_and_upload_app_store_connect skip_confirm:true skip_prechecks:true

image

Conversely, a lane with method: 'enterprise' won't show that behavior:

bundle exec fastlane build_and_upload_app_center skip_confirm:true skip_prechecks:true

image


Note that this PR targets release/20.9 because that's where the next builds will be deployed.

@mokagio mokagio self-assigned this Oct 10, 2022
@peril-wordpress-mobile
Copy link

Messages
📖 This PR has the 'Releases' label: some checks will be skipped.

Generated by 🚫 dangerJS

@@ -212,7 +219,7 @@
derived_data_path: DERIVED_DATA_PATH,
export_team_id: get_required_env('INT_EXPORT_TEAM_ID'),
export_method: 'enterprise',
export_options: { method: 'enterprise' }
export_options: { **COMMON_EXPORT_OPTIONS, method: 'enterprise' }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mentioned the issue is only in method: 'app-store' builds. However, it still feels appropriate to apply the fix to all builds.

@mokagio mokagio requested a review from a team October 10, 2022 03:44
@mokagio mokagio added this to the 20.9 ❄️ milestone Oct 10, 2022
@mokagio mokagio marked this pull request as ready for review October 10, 2022 03:45
@mokagio mokagio enabled auto-merge October 10, 2022 03:45
# - `manageAppVersionAndBuildNumber: false` prevents `xcodebuild` from bumping
# the build number when extracting an archive into an IPA file. We want to
# use the build number we set!
COMMON_EXPORT_OPTIONS = { manageAppVersionAndBuildNumber: false }
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 Freeze mutable objects assigned to constants.

Without any apparent change on our side, we saw a couple of Pocket Casts
builds with unexpected, and unexpectedly incrementing, build numbers on
TestFlight. From 7.24.0.3, to 8, to 9.

This behavior could be replicated locally in that project by running the
Fastlane automation, but not when archiving via Xcode.

I pin pointed it to the `manageAppVersionAndBuildNumber` option being
true by default in the `ExportOptions.plist` that Fastlane /
`xcodebuild` generates for `gym` to convert an archive into an IPA.

Explicitly setting the option to `false` solves the issue locally.

See Automattic/pocket-casts-ios#373.

To verify the issue on this project, checkout the current tip of the
release branch, 838058f, and apply the
following patch:

```
index 1c05a77b05..e3cb20e4ae 100644
--- a/fastlane/lanes/build.rb
+++ b/fastlane/lanes/build.rb
@@ -123,6 +123,8 @@ platform :ios do
       export_options: { method: 'app-store' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     testflight(
       skip_waiting_for_build_processing: true,
       team_id: get_required_env('FASTLANE_ITC_TEAM_ID'),
@@ -173,6 +175,8 @@ platform :ios do
       export_options: { method: 'app-store' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     testflight(
       skip_waiting_for_build_processing: true,
       team_id: '299112',
@@ -215,6 +219,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,
@@ -264,6 +270,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,
@@ -317,6 +325,8 @@ platform :ios do
       export_options: { method: 'enterprise' }
     )

+    UI.user_error! 'Aborting after building. This was just a test.'
+
     appcenter_upload(
       api_token: get_required_env('APPCENTER_API_TOKEN'),
       owner_name: APPCENTER_OWNER_NAME,
```

Run a lane to build a binary for App Store Connect
(`method: 'app-store'`) and notice how the build number is `21` instead
of the `20.9.0.0` coming from `config/Version.public.xcconfig`.

```
bundle exec fastlane build_and_upload_app_store_connect skip_confirm:true skip_prechecks:true
```

Conversely, a lane with `method: 'enterprise'` won't show that behavior:

```
bundle exec fastlane build_and_upload_app_center skip_confirm:true skip_prechecks:true
```
@mokagio mokagio force-pushed the mokagio/prevent-xcode-build-bump branch from 40f7a84 to 42bc003 Compare October 10, 2022 04:00
@wpmobilebot
Copy link
Contributor

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19421-42bc003 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19421-42bc003 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@AliSoftware
Copy link
Contributor

@mokagio is this PR still relevant even if we finally found out that the culprit might not have been that in the end?

Copy link
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Approving because I think it doesn't hurt to set this explicitly to false anyway, regardless of if this was the real root of the culprit of some builds version 8 and 9 being uploaded to ASC accidentally recently.

@mokagio mokagio merged commit ac01941 into release/20.9 Oct 10, 2022
@mokagio mokagio deleted the mokagio/prevent-xcode-build-bump branch October 10, 2022 10:36
@mokagio
Copy link
Contributor Author

mokagio commented Oct 10, 2022

@AliSoftware thank you for the review and approval. Your rationale is the same I would have brought forward for merging this.

Not sure why CI hasn't shown this behavior yet* but since it can happen locally and we do want to be able to deploy locally in case of emergencies (and avoid accidental local deploys from messing up our build numbers) it still handy to have the parameter set.

* 🤔 actually, maybe it's because it doesn't have an Xcode instance with an account authenticated to our App Store Connect where it can check the build number?

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.

3 participants