Parse common SwiftPM errors and only fetch Swift packages when project is using SwiftPM#185218
Parse common SwiftPM errors and only fetch Swift packages when project is using SwiftPM#185218vashworth wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to parse and report specific Swift Package Manager errors related to Flutter plugins during build and migration processes. It adds a parseError utility in SwiftPackageManager and integrates it into the iOS build and migration logic. Review feedback identifies a logic error in xcodeproj.dart where the incorrect directory is used to find the Flutter project, and suggests using allMatches instead of firstMatch in the error parser to ensure all potential plugin errors are evaluated.
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
Code Review
This pull request enhances Swift Package Manager integration by introducing guided error messages for common plugin configuration issues. It implements a parsing mechanism for SwiftPM error output and updates the prefetchSwiftPackages method with a force flag. The migration process is also updated to include a prefetch step with specific error handling. Feedback suggests refining the error-matching regular expressions to support common subdirectories and versioned plugin directories in the pub cache.
| buildDirectory: _fileSystem.directory( | ||
| _platform.buildDirectory(config: _config, fileSystem: _fileSystem), | ||
| ), | ||
| quiet: false, |
There was a problem hiding this comment.
Note I was able to remove the force here because now prefetchSwiftPackagesForProject only runs if the project has migrated to SwiftPM already so it doesn't get run before here.
| // If the project has not migrated to SwiftPM, we don't need to prefetch Swift packages. | ||
| if (!usesSwiftPackageManager || !flutterPluginSwiftPackageInProjectSettings) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is the only change to this functionality. The rest is a copy/paste
| ); | ||
| }); | ||
|
|
||
| testWithoutContext( |
There was a problem hiding this comment.
Moved these tests to xcode_project_test.dart
| }); | ||
|
|
||
| group('projectInfo', () { | ||
| testUsingContext( |
There was a problem hiding this comment.
All of this is just formatting. See my next comment to where the tests really start
| }); | ||
| }); | ||
|
|
||
| testWithoutContext( |
There was a problem hiding this comment.
These tests are copy/pasted with some slight tweaks since it's a new class.
| ); | ||
| }); | ||
|
|
||
| testWithoutContext('prefetchSwiftPackages can run for both platforms', () async { |
There was a problem hiding this comment.
This is a new test
|
Saw you are still updating this PR. Just double check if this is ready for review? |
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
Let me make sure the tests pass first |
|
Is PR description up to date? If not, could you update for the ease of review and for future reference? |
Done. This PR has kind of evolved. Let me know if you want me to break it up. |
Yes, breaking it up would make the review easier, and also helps future readers. Thank you! |
This PR does the following:
prefetchSwiftPackageslogic from theXcodeProjectInterpreterclass to theXcodeBasedProjectclass so that it can be run per platform (ios, macos)projectPathparameter from a String toXcodeBasedProjectprefetchSwiftPackagesis project has not been migrated to SwiftPMSwiftPackageManagerIntegrationMigrationthat prefetches the dependenciesAddresses #184021.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.