diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b77eedd..45f885e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,6 +13,9 @@ on: description: build and archive only, do not upload type: boolean default: false + screenshots_from_run: + description: "id of an earlier run: write the listing again from the pictures it took, building and photographing nothing" + type: string concurrency: # workflow wide, not per ref: the build number is a live query of what @@ -29,6 +32,7 @@ env: jobs: build: + if: ${{ inputs.screenshots_from_run == '' }} runs-on: macos-26 steps: # a dry run signs too, so it needs the same secrets @@ -189,6 +193,7 @@ jobs: # times as long as they do alone; a device to a machine is that, structurally. # It halves the wall clock as well, since the two halves photograph at once. screenshots: + if: ${{ inputs.screenshots_from_run == '' }} runs-on: macos-26 # long by nature, but the default is six hours for a wedged simulator to sit in timeout-minutes: 180 @@ -315,7 +320,7 @@ jobs: # a job per app, fail-fast off, so "Re-run failed jobs" can retry one half upload: needs: build - if: ${{ !inputs.dry_run }} + if: ${{ !inputs.dry_run && inputs.screenshots_from_run == '' }} runs-on: macos-26 strategy: fail-fast: false @@ -351,8 +356,17 @@ jobs: # listing, not the app listing: needs: [upload, screenshot-set] - if: ${{ !inputs.dry_run }} + # !cancelled() rather than success(), or naming a run to repair - which + # skips both of those jobs - would skip this one behind them + if: >- + ${{ !cancelled() && !inputs.dry_run + && (inputs.screenshots_from_run != '' + || (needs.upload.result == 'success' && needs['screenshot-set'].result == 'success')) }} runs-on: ubuntu-24.04 + # to read the artifact off the run being repaired, which is not this one + permissions: + contents: read + actions: read strategy: fail-fast: false matrix: @@ -375,12 +389,16 @@ jobs: given: ${{ inputs.version }} run: scripts/resolve-version.py --input "$given" --dry-run "$dry_run" - # where the lane looks for them, and the same set both apps are given + # where the lane looks for them, and the same set both apps are given. + # From the run being repaired when one was named, so the pictures are the + # ones already taken rather than a quarter hour of simulators again. - name: fetch the screenshots uses: actions/download-artifact@v8 with: name: framed path: fastlane/framed + run-id: ${{ inputs.screenshots_from_run || github.run_id }} + github-token: ${{ secrets.GITHUB_TOKEN }} # Kept, because the check below reads it. `tee` rather than a redirect so # the step still shows what it is doing while it runs. @@ -394,22 +412,26 @@ jobs: set -o pipefail bundle exec fastlane ios ${{ matrix.lane }} 2>&1 | tee listing.log - # A picture that did not land is uploaded again, and if the delete of the - # failed one does not take, the store keeps both. That leaves a locale - # showing the same screenshot twice, which nothing else here would notice - # - deliver reports the run as a success. So the retry itself is the - # failure: the pictures are up either way, and this says to go and look. + # The Fastfile stops deliver from sending a picture twice, so neither of + # these should ever print. They stay because both are silent in the store + # and in this log: deliver ends "Successfully uploaded all screenshots" + # and the job goes green either way. If a fastlane upgrade moves what the + # Fastfile patches, this is what still notices. - name: the screenshots went up once each run: | set -euo pipefail if grep -q "Failed to upload all screenshots" listing.log; then - echo "::error::deliver retried the screenshot upload, which can leave duplicates in App Store Connect. Check ${{ matrix.app }}'s screenshots for every locale before submitting, and re-run this job to upload the set again." + echo "::error::deliver uploaded ${{ matrix.app }}'s screenshots a second time, which leaves locales holding the same picture twice. Look at the set for every locale before submitting." + exit 1 + fi + if grep -q "Too many screenshots found" listing.log; then + echo "::error::a locale of ${{ matrix.app }} was full at ten screenshots and the rest were dropped. The locales named above are short. Look at the set for every locale before submitting." exit 1 fi record: needs: upload - if: ${{ !inputs.dry_run }} + if: ${{ !inputs.dry_run && inputs.screenshots_from_run == '' }} runs-on: ubuntu-24.04 permissions: contents: write diff --git a/README.md b/README.md index feeba2f..ed6a1af 100644 --- a/README.md +++ b/README.md @@ -133,12 +133,14 @@ It runs as five jobs: Both apps always go out together, and nothing chooses one: Pro and Lite are the same sources built as two targets, one of which links no ad sdk. -**If a `listing` job fails saying the screenshots did not go up once each**, the -set is up but a locale may hold a picture twice: deliver uploads what it thinks -did not land, and a delete that does not take leaves both. Look at the app's -screenshots in App Store Connect, and fix the set before submitting. Re-running -the job is worth a try - it clears the set before it uploads - but it has come -back doubled a second time, so check rather than assume. +**If a `listing` job fails saying screenshots are not listed**, they are up: +App Store Connect was slow to list them and nothing was sent twice. Look before +submitting, and re-run the job if a locale really is short. + +**To write a listing again from pictures already taken**, start a release with +`screenshots_from_run` set to that run's id - the `listing` jobs alone, against +its `framed` artifact, five minutes and no build number spent. Each locale is +cleared first, so it repairs a set in any state. **If one app's upload fails, press "Re-run failed jobs".** Only that upload runs again, against the `.ipa` already built and signed - build number included, since diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 6b2d763..00cbe61 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -3,6 +3,53 @@ require "fileutils" require "shellwords" require "tmpdir" +# deliver, whose upload_to_app_store this ends in +require "deliver" + +# deliver sends every picture, then reads the set back from App Store Connect +# and sends again whatever it cannot find. But the store lists a picture some +# seconds after it takes it, so what deliver cannot find is only the locales it +# sent last - sv and tr, at the end of the alphabet. The second copy joins the +# first, fills the set to the ten the store allows, and the pictures that no +# longer fit are then dropped with a line nobody reads. The lane still ends +# "Successfully uploaded all screenshots". +# +# Not one upload has failed here; only the reading back has. So wait and read +# again, and if a picture really is missing, stop and say so - never send a +# second copy. +unless Deliver::UploadScreenshots.instance_method(:retry_upload_screenshots_if_needed).arity == 7 + UI.important("deliver has moved the screenshot retry this patches - screenshots may go up twice again") +end + +module Deliver + class UploadScreenshots + # thirty seconds apart, so the last look is three minutes after the first: + # the store has always listed them within about a minute, and the rest is + # cheap next to a set that has to be repaired by hand + LOOKS = 7 + LOOK_AGAIN_AFTER = 30 + WAITS_AT_MOST = (LOOKS - 1) * LOOK_AGAIN_AFTER + + def retry_upload_screenshots_if_needed(_iterator, states, _count, _tries, _timeout, localizations, screenshots_per_language) + failed = states.fetch("FAILED", 0) + UI.user_error!("App Store Connect could not process #{failed} screenshot(s)") if failed > 0 + return if screenshots_per_language.empty? + + LOOKS.times do |look| + # a new iterator every time: the one deliver counted with asked the + # store before the last pictures were listed, and holds that answer + return if verify_local_screenshots_are_uploaded(AppScreenshotIterator.new(localizations), screenshots_per_language) + break if look == LOOKS - 1 + + UI.message("the store has not listed every screenshot yet - looking again in #{LOOK_AGAIN_AFTER}s") + sleep(LOOK_AGAIN_AFTER) + end + + UI.user_error!("App Store Connect is still not listing every screenshot, #{WAITS_AT_MOST}s after all of them were sent without error. Nothing was sent twice, so the set is at worst short - look at it before submitting.") + end + end +end + default_platform(:ios) # name also picks the app's half of the listing: fastlane/metadata is what both