From 03f120872127c3f93b74704e9981ffb542525e27 Mon Sep 17 00:00:00 2001 From: Victoria Ashworth <15619084+vashworth@users.noreply.github.com> Date: Wed, 12 Mar 2025 15:53:50 -0500 Subject: [PATCH] Fix SwiftPM scheme migration to handle when there are no BuildActionEntries (#164660) An Xcode scheme may or may not have `BuildActionEntries`. When migrating to SwiftPM, if Flutter cannot find `BuildActionEntries` in the xcscheme, append the `PreAction` to the end of the `BuildAction`. Fixes https://github.com/flutter/flutter/issues/163086. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- ...package_manager_integration_migration.dart | 20 +++--- ...ge_manager_integration_migration_test.dart | 67 +++++++++++++++++-- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/packages/flutter_tools/lib/src/migrations/swift_package_manager_integration_migration.dart b/packages/flutter_tools/lib/src/migrations/swift_package_manager_integration_migration.dart index b04eed06d9..d8b250c24d 100644 --- a/packages/flutter_tools/lib/src/migrations/swift_package_manager_integration_migration.dart +++ b/packages/flutter_tools/lib/src/migrations/swift_package_manager_integration_migration.dart @@ -311,18 +311,18 @@ class SwiftPackageManagerIntegrationMigration extends ProjectMigrator { $newContent '''; - final String? buildActionEntries = + String? buildAction = schemeLines.where((String line) => line.contains('')).firstOrNull; - if (buildActionEntries == null) { - throw Exception( - 'Failed to parse ${schemeFile.basename}: Could not find BuildActionEntries.', - ); - } else { - newScheme = schemeContent.replaceFirst( - buildActionEntries, - '$newContent$buildActionEntries', - ); + if (buildAction == null) { + // If there are no BuildActionEntries, append before end of BuildAction. + buildAction = + schemeLines.where((String line) => line.contains('')).firstOrNull; + + if (buildAction == null) { + throw Exception('Failed to parse ${schemeFile.basename}: Could not find BuildAction.'); + } } + newScheme = schemeContent.replaceFirst(buildAction, '$newContent$buildAction'); } schemeFile.writeAsStringSync(newScheme); diff --git a/packages/flutter_tools/test/general.shard/migrations/swift_package_manager_integration_migration_test.dart b/packages/flutter_tools/test/general.shard/migrations/swift_package_manager_integration_migration_test.dart index 98ada0d56c..0fcc50df98 100644 --- a/packages/flutter_tools/test/general.shard/migrations/swift_package_manager_integration_migration_test.dart +++ b/packages/flutter_tools/test/general.shard/migrations/swift_package_manager_integration_migration_test.dart @@ -464,7 +464,7 @@ void main() { }, ); - testWithoutContext('fails if cannot find BuildActionEntries in scheme', () async { + testWithoutContext('fails if cannot find BuildAction in scheme', () async { final MemoryFileSystem memoryFileSystem = MemoryFileSystem(); final BufferLogger testLogger = BufferLogger.test(); final FakeXcodeProject project = FakeXcodeProject( @@ -490,7 +490,7 @@ void main() { await expectLater( () => projectMigration.migrate(), throwsToolExit( - message: 'Failed to parse Runner.xcscheme: Could not find BuildActionEntries', + message: 'Failed to parse Runner.xcscheme: Could not find BuildAction', ), ); }); @@ -525,6 +525,42 @@ void main() { ); }); + testWithoutContext('successfully updates scheme with no BuildActionEntries', () async { + final MemoryFileSystem memoryFileSystem = MemoryFileSystem(); + final BufferLogger testLogger = BufferLogger.test(); + final FakeXcodeProject project = FakeXcodeProject( + platform: platform.name, + fileSystem: memoryFileSystem, + logger: testLogger, + ); + _createProjectFiles(project, platform); + project.xcodeProjectSchemeFile().writeAsStringSync( + _validBuildActions(platform, hasBuildEntries: false), + ); + + final FakePlistParser plistParser = FakePlistParser.multiple([ + _plutilOutput(_allSectionsMigratedAsJson(platform)), + _plutilOutput(_allSectionsMigratedAsJson(platform)), + ]); + final SwiftPackageManagerIntegrationMigration projectMigration = + SwiftPackageManagerIntegrationMigration( + project, + platform, + BuildInfo.debug, + xcodeProjectInterpreter: FakeXcodeProjectInterpreter(), + logger: testLogger, + fileSystem: memoryFileSystem, + plistParser: plistParser, + features: swiftPackageManagerFullyEnabledFlags, + ); + + await projectMigration.migrate(); + expect( + project.xcodeProjectSchemeFile().readAsStringSync(), + _validBuildActions(platform, hasFrameworkScript: true, hasBuildEntries: false), + ); + }); + testWithoutContext('successfully updates scheme with preexisting PreActions', () async { final MemoryFileSystem memoryFileSystem = MemoryFileSystem(); final BufferLogger testLogger = BufferLogger.test(); @@ -2621,6 +2657,7 @@ String _validBuildActions( SupportedPlatform platform, { bool hasPreActions = false, bool hasFrameworkScript = false, + bool hasBuildEntries = true, }) { final String scriptText; if (platform == SupportedPlatform.ios) { @@ -2656,11 +2693,11 @@ String _validBuildActions( \n '''; } - return ''' - $preActions - + + String buildEntries = ''; + if (hasBuildEntries) { + buildEntries = ''' +\n +'''; + } + return ''' + + $preActions$buildEntries + + +${_validBuildableReference(platform)} + + + '''; }