flutter_tools: Auto-generate ExportOptions.plist for manual iOS code signing#177888
Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances flutter build ipa to automatically generate a complete ExportOptions.plist for manual code signing configurations. This is a significant improvement for developers using manual signing, especially for apps with entitlements like Push Notifications. The changes include adding logic to detect manual signing, find the correct provisioning profile, and generate an enhanced plist with the necessary signing information. The implementation includes robust fallback mechanisms and extensive logging. New unit tests have been added to cover the new logic, and existing tests are updated. The code is well-documented and follows good practices for testability and maintainability. I have a couple of minor suggestions for improving code style and readability.
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
8d1771c to
959303b
Compare
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
cfd6ca6 to
0ac20f1
Compare
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
f9b8aa3 to
a10a5f0
Compare
028bcb9 to
b3a900b
Compare
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
b3a900b to
7c71a1f
Compare
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
vashworth
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Overall the concept sounds good to me. I left some comments on tweaking the implementation to make it more testable.
packages/flutter_tools/test/commands.shard/hermetic/build_ipa_export_plist_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
Outdated
Show resolved
Hide resolved
a9e070d to
7c71a1f
Compare
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
- Remove design decision comment from BuildIOSArchiveCommand class doc - Remove empty integration test placeholders (move rationale to PR description) - Remove redundant buildInfo declaration (line 575) - Make createExportPlist public with buildSettings parameter - Fetch buildSettings at call site and pass to public method - Remove test-only createExportPlistForTesting method - Remove unnecessary private wrapper methods - Update tests to use public createExportPlist method via command instance - Update all tests to pass buildSettings instead of individual parameters Follows Flutter's style guide: test APIs belong in test frameworks, not in the codebase.
- Remove design decision comment from BuildIOSArchiveCommand class doc - Remove empty integration test placeholders (move rationale to PR description) - Remove redundant buildInfo declaration (line 575) - Make createExportPlist public with buildSettings parameter - Fetch buildSettings at call site and pass to public method - Remove test-only createExportPlistForTesting method - Remove unnecessary private wrapper methods - Update tests to use public createExportPlist method via command instance - Update all tests to pass buildSettings instead of individual parameters Follows Flutter's style guide: test APIs belong in test frameworks, not in the codebase.
4070b45 to
afa4f9a
Compare
- Remove design decision comment from BuildIOSArchiveCommand class doc - Remove empty integration test placeholders (move rationale to PR description) - Remove redundant buildInfo declaration (line 575) - Make createExportPlist public with buildSettings parameter - Fetch buildSettings at call site and pass to public method - Remove test-only createExportPlistForTesting method - Remove unnecessary private wrapper methods - Update tests to use public createExportPlist method via command instance - Update all tests to pass buildSettings instead of individual parameters Follows Flutter's style guide: test APIs belong in test frameworks, not in the codebase.
…iveCommand - Introduced ProfileData class to encapsulate provisioning profile UUID and name. - Updated _findProvisioningProfileUuid to utilize ProfileData for improved clarity and efficiency. - Enhanced _parseProvisioningProfileInfo to extract both UUID and name in a single operation. - Added trace logging for better debugging when provisioning profiles cannot be found. - Removed unused ProcessManager and PlistParser parameters from method signatures in BuildIOSArchiveCommand. This refactor streamlines provisioning profile management and improves logging for manual signing scenarios.
…visioning profile handling - Added detailed documentation for the ProfileData class and its usage in generating ExportOptions.plist. - Implemented logic to auto-generate an enhanced ExportOptions.plist for manual signing scenarios, ensuring compatibility with Xcode's requirements. - Improved logging for provisioning profile lookups, providing clearer trace messages for debugging. - Expanded unit tests to cover various manual signing scenarios, including fallback behaviors and different build modes.
…est practices - Simplify FakeXcodeProjectInterpreter.getBuildSettings by defining defaults first, then spreading overrides (eliminates redundant ?? operators) - Replace listSync() with await for (using list()) in _findProvisioningProfileUuid for better asynchronous code style and non-blocking file system operations Addresses code review feedback from PR flutter#177888
…iene Add comments directly above skip parameters explaining why integration tests are skipped. This satisfies Flutter's tree hygiene requirement that all skipped tests must have justification comments. Fixes CI failure in PR flutter#177888
- Made createExportPlist public with buildSettings parameter - Removed test-only createExportPlistForTesting method - Eliminated design documentation from class docs - Cleaned up empty test placeholders - Fixed logging to use logger instead of globals.logger - Extracted provisioning profile directory lookup into reusable method Tests now call the public method directly with mock dependencies.
Removed static _createSimpleExportPlistStatic and _createManualSigningExportPlistStatic methods and converted them to instance methods _createSimpleExportPlist and _createManualSigningExportPlist. This follows Flutter's testing best practices by avoiding test-only static methods.
Addresses code review feedback on PR flutter#177888 by extracting shared provisioning profile logic into reusable components. Changes: - Made ProvisioningProfile class public and added uuid field - Made parseProvisioningProfile() method public in XcodeCodeSigningSettings - Extracted getProvisioningProfileDirectory() as a public function - Updated build_ios.dart to use shared code from code_signing.dart - Removed ProfileData class and duplicate parsing methods This eliminates approximately 73 lines of duplicate code and establishes a single source of truth for provisioning profile handling across flutter_tools.
…sues - Refactor XcodeCodeSigningSettings instantiation to runCommand and pass it to createExportPlist for dependency injection. - Remove core_devices_test.dart import in build_ipa_export_plist_test.dart to fix analyzer failure. - Implement local FakeXcodeCodeSigningSettings, FakeBuildableIOSApp, and FakeIosProject in build_ipa_export_plist_test.dart using noSuchMethod for robust, self-contained testing. - Remove stale TODO comments in build_ipa_test.dart and build_ipa_export_plist_test.dart.
The test for generating enhanced ExportOptions.plist for manual signing was failing because _findProvisioningProfileUuid used globals.fs and globals.fsUtils instead of the injected FileSystem, making it impossible to test with MemoryFileSystem. Changes: - Add optional fileSystemUtils parameter to createExportPlist - Add required fileSystem and fileSystemUtils parameters to _findProvisioningProfileUuid - Update test to create provisioning profiles directory structure in MemoryFileSystem and pass FakeFileSystemUtils with home directory path - Add FakeFileSystemUtils class to test file The changes are backwards compatible - existing code that doesn't pass fileSystemUtils will continue to use globals.fsUtils as the default.
|
@okorohelijah review |
|
autosubmit label was removed for flutter/flutter/177888, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
flutter_tools: Auto-generate ExportOptions.plist for manual iOS code signing
Summary
Enhance
flutter build ipato automatically generate a complete ExportOptions.plist for manual code signing configurations, eliminating the need for users to create and maintain one manually.Fixes: #177853
Problem
When using manual code signing (
CODE_SIGN_STYLE=Manual) withflutter build ipafor Release/Profile builds, the archive succeeds but the export step fails with:Root cause: Flutter generated an incomplete ExportOptions.plist that only included
methodanduploadBitcode. When using manual signing,xcodebuild -exportArchiverequires:teamID(from project'sDEVELOPMENT_TEAM)signingStyle=manualprovisioningProfiles(mapping bundle ID to provisioning profile UUID)Without these, Xcode cannot resolve the provisioning profile for export, even if the profile is correctly configured and contains the required entitlements.
Solution
Enhanced
_createExportPlist()inBuildIOSArchiveCommandto automatically generate a complete ExportOptions.plist when:CODE_SIGN_STYLE=Manualis detected for the main app targetThe generated plist includes:
method(app-store, ad-hoc, enterprise, etc. - respects user's--export-methodflag)teamID(fromDEVELOPMENT_TEAMbuild setting)signingStyle=manualprovisioningProfilesmapping main app bundle ID to provisioning profile UUIDFallback behavior:
Changes
Core Implementation
ProfileDataclass to encapsulate provisioning profile info (UUID and name)_createExportPlist()with manual signing detection and profile lookup_findProvisioningProfileUuid()to locate provisioning profiles by specifier_parseProvisioningProfileInfo()to decode provisioning profile data onceTesting
build_ipa_export_plist_test.dartwith 7 unit tests covering:Documentation
Added extensive inline comments explaining:
Safety & Scope
What This Fixes
--export-options-plistworkaroundWhat This Does NOT Change
Known Limitations (Future Enhancements)
Pre-merge Checklist
flutter test packages/flutter_tools)dart analyze)Safety Notes
flutter build ipafor Release/Profile buildsVerification
Users can verify the fix by:
CODE_SIGN_STYLE=Manual,DEVELOPMENT_TEAMset, and Push Notifications entitlementsflutter build ipa --release --verbose--export-options-plistRelated Issues
flutter build ipawith manual signing and provisioning profiles #106612 - Supportflutter build ipawith manual signing and provisioning profiles