Move prefetchSwiftPackages to be per platform#186468
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Swift package prefetching logic by moving the implementation from XcodeProjectInterpreter to the XcodeBasedProject class and renaming the original method to prefetchSwiftPackagesForProject. Corresponding tests were also relocated. Critical feedback identifies a missing convert.dart import in xcode_project.dart which will lead to compilation errors, and an invalid constructor implementation in the TestIosProject test class.
| // All `xcodebuild` project commands will download and resolve Swift packages. | ||
| // We should always prefetch Swift packages before running any `xcodebuild` project command | ||
| // to control the output. | ||
| await prefetchSwiftPackages(xcodeProject, buildDirectory: buildDirectory, quiet: false); |
There was a problem hiding this comment.
just to clarify - the original code prefetches for both macOS & iOS?
There was a problem hiding this comment.
The original code prefetched for whichever was first (iOS) and then wouldn't prefetch for the next (macOS) because the process would have already been set.
| xcodebuildProjectCommandArguments: _xcodebuildProjectCommandArguments( | ||
| buildDirectory, | ||
| // skipPackageUpdatesAndValidation should be false so that when subsequent xcodebuild | ||
| // commands run, packages should already be resolved, downloaded, updated, and validated. |
There was a problem hiding this comment.
is this comment accurate? what if subsequent xcodebuild command runs before packages are done fetching?
There was a problem hiding this comment.
This is accurate. Subsequent xcodebuild commands should not be run before packages are done fetching. The only case where that could happen is if waitForCompletion is false, which is not set when using fetchDependenciesAndGenerateXcodebuildArgs.
This PR moves the logic for
prefetchSwiftPackagesfrom theXcodeProjectInterpreterclass to theXcodeBasedProjectclass so that it can be run per platformIncremental step toward #185218.
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.