Repository navigation
Make Xcode workspace cleaning optional during flutter clean - #190091
Conversation
Fixes flutter#183946 This commit introduces a new `--clean-xcode-workspace` flag (defaulting to false) to `flutter clean` which allows skipping the expensive `xcodebuild -list` execution that inherently triggers Swift Package resolution over the internet. By default, Flutter will now instantly clean local build directories without polling Xcode. Additionally, this introduces an O(1) whitelist optimization to skip virtual SwiftPM schemes, significantly speeding up Xcode workspace processing when it is explicitly requested.
There was a problem hiding this comment.
Code Review
This pull request introduces a --clean-xcode-workspace flag to the clean command to make Xcode workspace cleaning optional, preventing unnecessary Swift package downloads. It also adds checks to ensure Xcode scheme files exist before cleaning or checking watch companion settings, and updates tests accordingly. Feedback on the changes highlights a critical runtime crash risk when checking the unregistered scheme option, and suggests configuring the new flag with defaultsTo: false and negatable: false for clarity and to avoid redundant options.
|
@okorohelijah Can you update this PR's title and description? It seems to be inaccurate You should also update it so it closes #173940 and #127708 too |
|
Can you also update this error message to include the new flag: flutter/packages/flutter_tools/lib/src/ios/mac.dart Lines 1265 to 1270 in 5a2a94a |
|
LGTM but looks like test is failing: You also have a merge conflict |
| negatable: false, | ||
| help: | ||
| 'Whether to run "xcodebuild clean" on the Xcode workspace for iOS and macOS projects. ' | ||
| "This removes build products and intermediate files from Xcode's build cache and can be slow to complete.", |
There was a problem hiding this comment.
is this accurate? from the PR description the slowness seems to be caused by the fact that xcode clean triggers SwiftPM fetch?
There was a problem hiding this comment.
Yup, that is correct! What the help message implies is that using this flag forces xcodebuild clean, which inherently triggers SwiftPM package resolution before cleaning, causing the slowness.
There was a problem hiding this comment.
I can update the help message so it explicitly mentions the SwiftPM resolution to make it clearer. wdyt
There was a problem hiding this comment.
It's not directly tied to SwiftPM. SwiftPM does impact it, but people have complained about it being slow long before SwiftPM: #127708
|
Reason for revert: breaks Mac_arm64 plugin_lint_mac |
|
Successfully created revert PR: #190890 |
|
flutter clean used to invoke xcodebuild clean on the Xcode workspace, which cleared Xcode's DerivedData and Clang module caches. This turns that off. In plugin_lint_mac.dart, the test first builds the iOS app with In later steps, When we get to the plugin_lint_mac.dart section, we do: await inDirectory(appPath, () async {
await flutter('clean');
await flutter('build', options: <String>['ios', '--no-codesign']);
});
During the next build, You'll need to revert to old behaviour: await inDirectory(appPath, () async {
- await flutter('clean');
+ await flutter('clean', options: <String>['--include-xcode-workspace']);
await flutter('build', options: <String>['ios', '--no-codesign']);
}); |
…lutter#190890) Reverts: [Make Xcode workspace cleaning optional during flutter clean](flutter#190091) Initiated by: @cbracken Reason for reverting: breaks Mac_arm64 plugin_lint_mac Original PR Author: @okorohelijah Reviewed By: @vashworth The original PR description is provided below: This PR introduces a new `--include-xcode-workspace` flag to flutter clean which makes Xcode workspace cleaning optional, bypassing the expensive xcodebuild execution that inherently triggers Swift Package resolution over the internet. By default, it will now instantly clean local build directories without polling or cleaning Xcode. Additionally, this updates several Xcode-specific error messages across the codebase to explicitly instruct users to run `flutter clean --include-xcode-workspace` when clearing Xcode's derived data is required *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes flutter#183946, flutter#173940 and flutter#127708 too *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] 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]. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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
…lutter#191023) The original [PR](flutter#190091) skipped cleaning Xcode's `DerivedData` Clang module cache by default. This caused the `plugin_lint_mac` test to fail because the test dynamically toggles `use_frameworks!` off and expects a clean slate. Since `DerivedData` wasn't wiped, Xcode's cached index expected the previously generated `.framework/Modules/module.modulemap` to exist in `build/` (which `flutter clean` had just deleted), resulting in a `module map file not found` build failure. To fix this, the `--include-xcode-workspace` flag is now added to the test. Since toggling `use_frameworks!` isn't a general use case , there is now an actionable error to gracefully guide developers to run `flutter clean --include-xcode-workspace` if they hit this cache corruption. *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes flutter#183946, flutter#173940 and flutter#127708 too *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [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. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] 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]. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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
This PR introduces a new
--include-xcode-workspaceflag to flutter clean which makes Xcode workspace cleaning optional, bypassing the expensive xcodebuild execution that inherently triggers Swift Package resolution over the internet. By default, it will now instantly clean local build directories without polling or cleaning Xcode. Additionally, this updates several Xcode-specific error messages across the codebase to explicitly instruct users to runflutter clean --include-xcode-workspacewhen clearing Xcode's derived data is requiredList which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
Fixes #183946, #173940 and #127708 too
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
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.