Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .buildkite/commands/build-and-upload-testflight.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/bash -eu

# Builds one app and uploads it to TestFlight for internal testers.
#
# Part of the "Faster Releases for WordPress and Jetpack" RFC. CI runs one
# invocation per app (in parallel) via the matrix step in pipeline.yml.

Comment on lines +3 to +7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really dislike these AI comments that carry context that's only relevant while a certain project is ongoing.

Suggested change
# Builds one app and uploads it to TestFlight for internal testers.
#
# Part of the "Faster Releases for WordPress and Jetpack" RFC. CI runs one
# invocation per app (in parallel) via the matrix step in pipeline.yml.

APP="${1:?Usage: build-and-upload-testflight.sh <wordpress|jetpack|reader>}"

"$(dirname "${BASH_SOURCE[0]}")/shared-set-up.sh"
"$(dirname "${BASH_SOURCE[0]}")/shared-set-up-distribution.sh"

echo "--- :closed_lock_with_key: Installing Secrets"
bundle exec fastlane run configure_apply

echo "--- :hammer_and_wrench: Building and uploading ${APP} to TestFlight"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Emoji game ++

Suggested change
echo "--- :hammer_and_wrench: Building and uploading ${APP} to TestFlight"
echo "--- :testflight: Building and uploading ${APP} to TestFlight"

bundle exec fastlane build_and_upload_app_for_testflight app:"${APP}"
26 changes: 26 additions & 0 deletions .buildkite/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,32 @@ steps:
- github_commit_status:
context: "Jetpack Prototype Build"

#################
# Continuous Delivery: build & upload each app to TestFlight (internal testers)
#
# Part of the "Faster Releases for WordPress and Jetpack" RFC. On every trunk
# commit, each app builds and uploads to TestFlight in parallel via the matrix
# below. Gated to trunk so it doesn't run on PR branches.
#################
Comment on lines +34 to +40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
#################
# Continuous Delivery: build & upload each app to TestFlight (internal testers)
#
# Part of the "Faster Releases for WordPress and Jetpack" RFC. On every trunk
# commit, each app builds and uploads to TestFlight in parallel via the matrix
# below. Gated to trunk so it doesn't run on PR branches.
#################
#################
# Continuous Delivery: build & upload each app to TestFlight (internal testers) from trunk
#################

- group: ":testflight: TestFlight Builds"
key: testflight_builds_group
steps:
- label: ":testflight: Build & Upload {{matrix}}"
command: ".buildkite/commands/build-and-upload-testflight.sh {{matrix}}"
plugins: [$CI_TOOLKIT_PLUGIN]
if: "build.branch == 'trunk'"
matrix:
- "wordpress"
- "jetpack"
# Reader is omitted: its App Store archive has been broken since #25321
# (PostHelper moved to the app target without updating the Reader target),
# and went undetected after #25179 removed Reader from CI. Re-add once
# the Reader target archives again.
# - "reader"
Comment on lines +51 to +55

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair enough. But I guess the technical issues are downstream of Reader not being in the radar for release at the moment. If there was an higher-level issue on this topic, it'd be better to link that, I think. Just an idea, though.

notify:
- github_commit_status:
context: "TestFlight Build ({{matrix}})"

#################
# Create Builds for Testing
#################
Expand Down
137 changes: 137 additions & 0 deletions fastlane/lanes/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,116 @@
end
end

# Builds a single app and uploads it to TestFlight for *internal* testers,
# stamping the build code with the Buildkite build number.
#
# This is the per-commit "continuous delivery" build from the "Faster Releases
# for WordPress and Jetpack" RFC. It is intentionally additive: the existing
# release lanes are untouched and remain the source of truth until this flow
# is proven.
#
Comment on lines +320 to +324

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# This is the per-commit "continuous delivery" build from the "Faster Releases
# for WordPress and Jetpack" RFC. It is intentionally additive: the existing
# release lanes are untouched and remain the source of truth until this flow
# is proven.
#

# The marketing version (`VERSION_SHORT`) is read from `Version.public.xcconfig`
# as-is. The build code is `<marketing version>.0.<buildkite build number>`
# (e.g. `27.0.0.4567`). Buildkite build numbers increase monotonically, so each
# build for a given marketing version gets a unique, higher build code — which
# is all App Store Connect requires.
Comment on lines +325 to +329

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good. Other apps implement similar strategies relying on Buildkite.

#
# "Internal-only" means no external groups and no external-tester notifications
# (matching the existing Reader upload). Distributing to the a8c staff beta
# group, and separately to the public beta group, is wired up in later phases
# of the RFC.
Comment on lines +330 to +334

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
#
# "Internal-only" means no external groups and no external-tester notifications
# (matching the existing Reader upload). Distributing to the a8c staff beta
# group, and separately to the public beta group, is wired up in later phases
# of the RFC.

#
# @param [String] app One of `wordpress`, `jetpack`, or `reader`.
#
# @called_by CI
#
desc 'Builds one app and uploads it to TestFlight for internal testers'
lane :build_and_upload_app_for_testflight do |app:|
app = app.to_s.downcase

case app
when 'wordpress'
scheme = 'WordPress'
output_name = APP_STORE_CONNECT_BUILD_NAME_WORDPRESS
beta_app_description_path = WORDPRESS_BETA_APP_DESCRIPTION_PATH
sentry_project_slug = SENTRY_PROJECT_SLUG_WORDPRESS
app_identifier = WORDPRESS_BUNDLE_IDENTIFIER
update_certs_and_profiles_wordpress_app_store
when 'jetpack'
scheme = 'Jetpack'
output_name = APP_STORE_CONNECT_BUILD_NAME_JETPACK
beta_app_description_path = JETPACK_BETA_APP_DESCRIPTION_PATH
sentry_project_slug = SENTRY_PROJECT_SLUG_JETPACK
app_identifier = JETPACK_BUNDLE_IDENTIFIER
update_certs_and_profiles_jetpack_app_store
when 'reader'
scheme = 'Reader'
output_name = APP_STORE_CONNECT_BUILD_NAME_READER
beta_app_description_path = BETA_APP_DESCRIPTION_PATH_READER
sentry_project_slug = nil # Reader does not have a Sentry project yet
app_identifier = nil
update_certs_and_profiles_app_store_reader
Comment on lines +346 to +365

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

update_certs_and_profiles_[wordpress|jetpack]_app_store vs update_certs_and_profiles_app_store_reader

Will be addressed, eventually, in https://linear.app/a8c/issue/AINFRA-2556

else
UI.user_error!("Unknown app '#{app}'. Expected one of: wordpress, jetpack, reader")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick: this could be reimplemented with something like

options = {
  wordpress: {
    scheme: '...',
    ...
    code_signing_lambda: # I don't remember how to write lambdas...
  },
  jetpack: ...
}

Then the check could be like

app_to_build = app.to_s.downcase.to_sym
app_to_build_options = options[app_to_build]

UI.user_error!("Unknown app '#{app_to_build}'. Expected one of: #{options.keys.map { |k| k.to_s }.join(', ')}") unless app_to_build

Anyway, this is just a syntax nitpick. It jumped to my mind because of the hardcoded list of accepted options. Might not even be worth implementing given how static the list of targets is anyway. Very low chances of having another app any time soon.

end

build_number = ENV.fetch('BUILDKITE_BUILD_NUMBER', nil)
UI.user_error!('BUILDKITE_BUILD_NUMBER is not set — this lane is meant to run on CI') if build_number.nil?

build_code = "#{release_version_current}.0.#{build_number}"
UI.important("Building #{scheme} #{release_version_current} (#{build_code}) for internal TestFlight distribution")

sentry_check_cli_installed

Comment on lines +376 to +377

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we move this to the very top to fail early?

build_app(
scheme: scheme,
workspace: WORKSPACE_PATH,
clean: true,
output_directory: BUILD_PRODUCTS_PATH,
output_name: output_name,
derived_data_path: DERIVED_DATA_PATH,
export_team_id: get_required_env('EXT_EXPORT_TEAM_ID'),
xcargs: { VERSION_LONG: build_code },
export_options: { **COMMON_EXPORT_OPTIONS, method: 'app-store' }
)

upload_app_to_testflight_internal(
ipa_path: lane_context[SharedValues::IPA_OUTPUT_PATH],
beta_app_description_path: beta_app_description_path
)

# Upload symbols so crashes from these builds symbolicate in Sentry.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Upload symbols so crashes from these builds symbolicate in Sentry.

next if sentry_project_slug.nil?

sentry_debug_files_upload(
auth_token: get_required_env('SENTRY_AUTH_TOKEN'),
org_slug: SENTRY_ORG_SLUG,
project_slug: sentry_project_slug,
path: lane_context[SharedValues::DSYM_OUTPUT_PATH]
)

upload_gutenberg_sourcemaps(
sentry_project_slug: sentry_project_slug,
release_version: release_version_current,
build_version: build_code,
app_identifier: app_identifier
)
end

# Convenience wrapper that builds and uploads each app to TestFlight, one after
# another. CI builds each app in parallel via a Buildkite matrix instead; this
# lane is handy for running the whole set locally.
#
# Reader is omitted until its App Store archive is fixed (broken since #25321).
# The per-app lane still supports `app: 'reader'`; just re-add it here once it builds.
#
desc 'Builds and uploads WordPress and Jetpack to TestFlight (internal)'
lane :build_all_apps_for_testflight do
%w[wordpress jetpack].each do |app|
build_and_upload_app_for_testflight(app: app)
end
end
Comment on lines +413 to +425

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I debate this given we should never build apps from devs machine. But... never say never, right?

If we keep it, then I'd suggest a visible message regarding Reader not being part of the set, which is against what the name suggests.

Suggested change
# Convenience wrapper that builds and uploads each app to TestFlight, one after
# another. CI builds each app in parallel via a Buildkite matrix instead; this
# lane is handy for running the whole set locally.
#
# Reader is omitted until its App Store archive is fixed (broken since #25321).
# The per-app lane still supports `app: 'reader'`; just re-add it here once it builds.
#
desc 'Builds and uploads WordPress and Jetpack to TestFlight (internal)'
lane :build_all_apps_for_testflight do
%w[wordpress jetpack].each do |app|
build_and_upload_app_for_testflight(app: app)
end
end
# Convenience wrapper that builds and uploads each app to TestFlight, one after
# another. CI builds each app in parallel via a Buildkite matrix instead; this
# lane is handy for running the whole set locally.
#
# Reader is omitted until its App Store archive is fixed (broken since #25321).
# The per-app lane still supports `app: 'reader'`; just re-add it here once it builds.
#
desc 'Builds and uploads WordPress and Jetpack to TestFlight (internal)'
lane :build_all_apps_for_testflight do
apps_to_build = %[wordpress jetpack]
UI.important("Will only build #{apps_to_build}.join('and '). Reader is omitted because currently broken.")
apps_to_build.each do |app|
build_and_upload_app_for_testflight(app: app)
end
end


# Builds the WordPress app for a Prototype Build ("WordPress Alpha" scheme), and uploads it to Firebase App Distribution
#
# @called_by CI
Expand Down Expand Up @@ -426,6 +536,33 @@ def upload_build_to_testflight(ipa_path:, whats_new_path:, distribution_groups:,
)
end

# Uploads an already-built IPA to TestFlight for internal testers only.
#
# @param [String] ipa_path Path to the built IPA.
# @param [String] beta_app_description_path Path to the beta app description file.
#
def upload_app_to_testflight_internal(ipa_path:, beta_app_description_path:)
# TBD (RFC D4): the "what's new" text for per-commit internal builds is not
# finalized yet. For now, generate a minimal placeholder from the commit
# metadata so the upload has something to show. Public-beta builds will get
# richer notes generated from the PRs merged since the previous public build.
Comment on lines +545 to +548

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TBD (RFC D4): the "what's new" text for per-commit internal builds is not
# finalized yet. For now, generate a minimal placeholder from the commit
# metadata so the upload has something to show. Public-beta builds will get
# richer notes generated from the PRs merged since the previous public build.
# TODO: the "what's new" text for per-commit internal builds is not
# finalized yet. For now, generate a minimal placeholder from the commit
# metadata so the upload has something to show. Public-beta builds will get
# richer notes generated from the PRs merged since the previous public build.

As far as I know, TBD is not a common "tag" for looking up leftover work.

changelog = "Automated build from `#{ENV.fetch('BUILDKITE_BRANCH', 'unknown')}` (#{ENV.fetch('BUILDKITE_COMMIT', 'unknown')[0...7]})."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
changelog = "Automated build from `#{ENV.fetch('BUILDKITE_BRANCH', 'unknown')}` (#{ENV.fetch('BUILDKITE_COMMIT', 'unknown')[0...7]})."
changelog = "Automated build from `#{ENV.fetch('BUILDKITE_BRANCH', 'unknown branch')}` (#{ENV.fetch('BUILDKITE_COMMIT', 'unknown commit')[0...7]})."

whats_new_path = File.join(Dir.tmpdir, 'testflight_whats_new.txt')

begin
File.write(whats_new_path, changelog)

upload_build_to_testflight(
ipa_path: ipa_path,
whats_new_path: whats_new_path,
distribution_groups: [], # Internal-only for now (RFC D2): no external groups.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
distribution_groups: [], # Internal-only for now (RFC D2): no external groups.
distribution_groups: [],

beta_app_description_path: beta_app_description_path
)
ensure
FileUtils.rm_rf(whats_new_path)
end
Comment on lines +561 to +563

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you considered using the Dir.mktmpdir with a block, so that we don't have to manually manage the rm?

end

# Send a Slack message to the specified channel
#
# @param [String] message The message to send to the channel
Expand Down