feat: split unsigned IPA into runner and screen streamer#28
Merged
Conversation
Produce two separate IPAs instead of one combined archive: - devicekit-ios-runner.ipa (XCUITest runner for device automation) - devicekit-ios-screenstreamer.ipa (Screen Stream app) iOS can only install one bundle per IPA, so they must be separate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCI workflow artifact handling was changed: artifact name 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 45-53: Before packaging, remove any stale artifacts: delete
$(EXPORT_PATH)/Payload and existing archive files for the scheme (e.g.
$(EXPORT_PATH)/$(SCHEME)-runner.ipa and
$(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa) to ensure deterministic output,
then recreate Payload with mkdir -p; replace the current sequence around the
Payload/zip steps so you run rm -rf "$(EXPORT_PATH)/Payload" && rm -f
"$(EXPORT_PATH)/$(SCHEME)-runner.ipa" && rm -f
"$(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa" before creating Payload and
copying the .app bundles, and ensure you also remove the temporary Payload after
each zip to avoid leaks between the two packaging steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93b178b4-788d-4ca6-bb2e-fd9d8b6e037c
📒 Files selected for processing (2)
.github/workflows/build.ymlMakefile
Comment on lines
+45
to
+53
| @mkdir -p $(EXPORT_PATH)/Payload | ||
| @cp -r $(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME).app $(EXPORT_PATH)/Payload/ | ||
| @cp -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" $(EXPORT_PATH)/Payload/ | ||
| @scripts/patch-runner.sh "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos" "$(EXPORT_PATH)/Payload" | ||
| @cd $(EXPORT_PATH) && zip -r $(SCHEME)-unsigned.ipa Payload | ||
| @cd $(EXPORT_PATH) && zip -r $(SCHEME)-runner.ipa Payload | ||
| @rm -rf $(EXPORT_PATH)/Payload | ||
| @echo "Runner IPA created at: $(EXPORT_PATH)/$(SCHEME)-runner.ipa" | ||
| @echo "Packaging screen streamer IPA..." | ||
| @mkdir -p $(EXPORT_PATH)/Payload | ||
| @cp -r $(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME).app $(EXPORT_PATH)/Payload/ | ||
| @cd $(EXPORT_PATH) && zip -r $(SCHEME)-screenstreamer.ipa Payload |
There was a problem hiding this comment.
Clean Payload and old archives before repackaging.
Line 45 only creates Payload, and Lines 47/53 write into whatever $(SCHEME)-*.ipa already exists. If make ipa-unsigned is rerun after a partial/interrupted build, stale app bundles/files can leak into the new artifacts, so the output is no longer deterministic.
Proposed hardening
`@echo` "Packaging runner IPA..."
- `@mkdir` -p $(EXPORT_PATH)/Payload
- `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" $(EXPORT_PATH)/Payload/
- `@cd` $(EXPORT_PATH) && zip -r $(SCHEME)-runner.ipa Payload
+ `@rm` -rf "$(EXPORT_PATH)/Payload" "$(EXPORT_PATH)/$(SCHEME)-runner.ipa"
+ `@mkdir` -p "$(EXPORT_PATH)/Payload"
+ `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" "$(EXPORT_PATH)/Payload/"
+ `@cd` "$(EXPORT_PATH)" && zip -r "$(SCHEME)-runner.ipa" Payload
`@rm` -rf $(EXPORT_PATH)/Payload
`@echo` "Runner IPA created at: $(EXPORT_PATH)/$(SCHEME)-runner.ipa"
`@echo` "Packaging screen streamer IPA..."
- `@mkdir` -p $(EXPORT_PATH)/Payload
- `@cp` -r $(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME).app $(EXPORT_PATH)/Payload/
- `@cd` $(EXPORT_PATH) && zip -r $(SCHEME)-screenstreamer.ipa Payload
+ `@rm` -rf "$(EXPORT_PATH)/Payload" "$(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa"
+ `@mkdir` -p "$(EXPORT_PATH)/Payload"
+ `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME).app" "$(EXPORT_PATH)/Payload/"
+ `@cd` "$(EXPORT_PATH)" && zip -r "$(SCHEME)-screenstreamer.ipa" Payload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 45 - 53, Before packaging, remove any stale artifacts:
delete $(EXPORT_PATH)/Payload and existing archive files for the scheme (e.g.
$(EXPORT_PATH)/$(SCHEME)-runner.ipa and
$(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa) to ensure deterministic output,
then recreate Payload with mkdir -p; replace the current sequence around the
Payload/zip steps so you run rm -rf "$(EXPORT_PATH)/Payload" && rm -f
"$(EXPORT_PATH)/$(SCHEME)-runner.ipa" && rm -f
"$(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa" before creating Payload and
copying the .app bundles, and ensure you also remove the temporary Payload after
each zip to avoid leaks between the two packaging steps.
Remove screen streamer app from build artifacts. The broadcast extension will be refactored to a separate repo.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Makefile (2)
45-48:⚠️ Potential issue | 🟠 MajorRecreate the IPA instead of updating it in place.
Line 47 writes into any existing
$(SCHEME)-runner.ipa.zip -rupdates matching entries, but it does not remove files that disappeared fromPayload, so rerunningmake ipa-unsignedcan leak stale bundle contents into the new artifact.Proposed change
`@echo` "Packaging runner IPA..." - `@mkdir` -p $(EXPORT_PATH)/Payload - `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" $(EXPORT_PATH)/Payload/ - `@cd` $(EXPORT_PATH) && zip -r $(SCHEME)-runner.ipa Payload - `@rm` -rf $(EXPORT_PATH)/Payload + `@rm` -rf "$(EXPORT_PATH)/Payload" "$(EXPORT_PATH)/$(SCHEME)-runner.ipa" + `@mkdir` -p "$(EXPORT_PATH)/Payload" + `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" "$(EXPORT_PATH)/Payload/" + `@cd` "$(EXPORT_PATH)" && zip -r "$(SCHEME)-runner.ipa" Payload + `@rm` -rf "$(EXPORT_PATH)/Payload"Run this to verify the
zip -rbehavior in isolation:#!/bin/bash set -euo pipefail tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT mkdir -p "$tmp/Payload/App.app" printf 'old\n' > "$tmp/Payload/App.app/old.txt" ( cd "$tmp" zip -qr test.ipa Payload ) rm "$tmp/Payload/App.app/old.txt" printf 'new\n' > "$tmp/Payload/App.app/new.txt" ( cd "$tmp" zip -qr test.ipa Payload ) python - "$tmp/test.ipa" <<'PY' import sys, zipfile zf = zipfile.ZipFile(sys.argv[1]) names = sorted(zf.namelist()) print("\n".join(names)) print("\nstale_old_present =", "Payload/App.app/old.txt" in names) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 45 - 48, The Makefile's zip step currently updates an existing $(SCHEME)-runner.ipa which can leave stale files; before running zip -r, delete any existing archive at $(EXPORT_PATH)/$(SCHEME)-runner.ipa (or use zip --filesync/ -FS) so the new archive is created cleanly from $(EXPORT_PATH)/Payload; update the snippet around the cp/cd/zip steps to remove the old $(SCHEME)-runner.ipa (rm -f $(EXPORT_PATH)/$(SCHEME)-runner.ipa) or replace zip -r with zip -r -FS to ensure removed files do not persist.
44-49:⚠️ Potential issue | 🟠 MajorEmit the screen streamer IPA from
ipa-unsignedtoo.This target now stops after creating
$(SCHEME)-runner.ipa. Nothing inipa-unsignedrepackages$(SCHEME).appinto a second IPA anymore, so the runner/screen-streamer split is only half implemented.Proposed change
`@echo` "Packaging runner IPA..." `@mkdir` -p $(EXPORT_PATH)/Payload `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" $(EXPORT_PATH)/Payload/ `@cd` $(EXPORT_PATH) && zip -r $(SCHEME)-runner.ipa Payload `@rm` -rf $(EXPORT_PATH)/Payload `@echo` "Runner IPA created at: $(EXPORT_PATH)/$(SCHEME)-runner.ipa" + `@echo` "Packaging screen streamer IPA..." + `@mkdir` -p $(EXPORT_PATH)/Payload + `@cp` -r "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME).app" $(EXPORT_PATH)/Payload/ + `@cd` $(EXPORT_PATH) && zip -r $(SCHEME)-screenstreamer.ipa Payload + `@rm` -rf $(EXPORT_PATH)/Payload + `@echo` "Screen streamer IPA created at: $(EXPORT_PATH)/$(SCHEME)-screenstreamer.ipa"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 44 - 49, The current Makefile block that packages the runner IPA (uses EXPORT_PATH, BUILD_DIR, CONFIGURATION, SCHEME) stops after creating $(SCHEME)-runner.ipa and no longer repackages the app in the ipa-unsigned export; update this packaging step to also create the screen-streamer IPA from the ipa-unsigned output. Concretely: after the existing runner packaging (the block that cp's "$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app" into $(EXPORT_PATH)/Payload and zips it), add analogous commands that copy the $(SCHEME).app produced by the ipa-unsigned path into a fresh Payload, zip it as the screen-streamer IPA (use a clear name like $(SCHEME)-screen-streamer.ipa or match existing naming conventions), and clean up the temporary Payload; reference the same variables (EXPORT_PATH, SCHEME, BUILD_DIR/ipa-unsigned output) so both runner and screen-streamer IPAs are produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Around line 45-48: The Makefile's zip step currently updates an existing
$(SCHEME)-runner.ipa which can leave stale files; before running zip -r, delete
any existing archive at $(EXPORT_PATH)/$(SCHEME)-runner.ipa (or use zip
--filesync/ -FS) so the new archive is created cleanly from
$(EXPORT_PATH)/Payload; update the snippet around the cp/cd/zip steps to remove
the old $(SCHEME)-runner.ipa (rm -f $(EXPORT_PATH)/$(SCHEME)-runner.ipa) or
replace zip -r with zip -r -FS to ensure removed files do not persist.
- Around line 44-49: The current Makefile block that packages the runner IPA
(uses EXPORT_PATH, BUILD_DIR, CONFIGURATION, SCHEME) stops after creating
$(SCHEME)-runner.ipa and no longer repackages the app in the ipa-unsigned
export; update this packaging step to also create the screen-streamer IPA from
the ipa-unsigned output. Concretely: after the existing runner packaging (the
block that cp's
"$(BUILD_DIR)/Build/Products/$(CONFIGURATION)-iphoneos/$(SCHEME)UITests-Runner.app"
into $(EXPORT_PATH)/Payload and zips it), add analogous commands that copy the
$(SCHEME).app produced by the ipa-unsigned path into a fresh Payload, zip it as
the screen-streamer IPA (use a clear name like $(SCHEME)-screen-streamer.ipa or
match existing naming conventions), and clean up the temporary Payload;
reference the same variables (EXPORT_PATH, SCHEME, BUILD_DIR/ipa-unsigned
output) so both runner and screen-streamer IPAs are produced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.