Clean Xcode workspace during flutter clean #38992

jmagman · 2019-08-21T18:08:24Z

Description

Xcode workspace clean during `flutter clean.
Remove macOS ephemeral directories
Add progresses to give users more insight into what is being cleaned:

$ flutter clean 
Cleaning Xcode workspace for scheme Runner...                       1.1s
Deleting build...                                                   22ms
Deleting .android...                                                 3ms
Deleting .ios...                                                    12ms

Related Issues

Fixes #34462

Tests

I updated clean_test, swapped it to a MemoryFileSystem since the macOS directory structure was getting to be a lot with mocks.

Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

jmagman · 2019-08-21T18:10:39Z

If you have time to give this a quick test to see if it correctly cleans the macOS workspace that would be greatly appreciated!

Works as expected. Do you want to add the Pods directory under macos to the list though? (ios too, I would think, but I don't know if there are special considerations.) It's gitignored, and sometimes needs to be nuked to resolve issues, so seems like it fits the bill for something that clean should eliminate.

codecov · 2019-08-21T18:59:30Z

Codecov Report

Merging #38992 into master will increase coverage by 0.56%.
The diff coverage is 84.84%.

@@            Coverage Diff             @@
##           master   #38992      +/-   ##
==========================================
+ Coverage   55.75%   56.31%   +0.56%     
==========================================
  Files         195      195              
  Lines       18303    18321      +18     
==========================================
+ Hits        10204    10318     +114     
+ Misses       8099     8003      -96

Flag	Coverage Δ
#flutter_tool	`56.31% <84.84%> (+0.56%)`	⬆️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/commands/run.dart	`62.81% <ø> (+2.71%)`	⬆️
.../flutter_tools/lib/src/runner/flutter_command.dart	`79.82% <ø> (+0.69%)`	⬆️
packages/flutter_tools/lib/src/ios/xcodeproj.dart	`85.16% <0%> (-1.69%)`	⬇️
packages/flutter_tools/lib/src/macos/xcode.dart	`60.71% <100%> (+7.14%)`	⬆️
packages/flutter_tools/lib/src/commands/clean.dart	`91.66% <93.1%> (-3.58%)`	⬇️
packages/flutter_tools/lib/src/base/flags.dart	`5.55% <0%> (-44.45%)`	⬇️
...ackages/flutter_tools/lib/src/commands/config.dart	`52% <0%> (-34.67%)`	⬇️
...es/flutter_tools/lib/src/macos/macos_workflow.dart	`55.55% <0%> (-33.34%)`	⬇️
packages/flutter_tools/lib/src/base/io.dart	`45.94% <0%> (-5.41%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`90.77% <0%> (-1.95%)`	⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e34e80...98bb395. Read the comment docs.

stuartmorgan · 2019-08-21T19:02:54Z

@stuartmorgan How do you feel about the macOS ephemeral directories being deleted during flutter clean?

Sounds great, I think I assumed that was already happening actually.

Worth noting is that when I named that, I didn't know that iOS/Android add-to-app had used the same term for something different, which may be confusing in contexts like this one. I should maybe rename it.

If you have time to give this a quick test to see if it correctly cleans the macOS workspace that would be greatly appreciated!

Will do a bit later today.

jonahwilliams · 2019-08-21T19:42:32Z

packages/flutter_tools/lib/src/commands/clean.dart

-    final String path = file.path;
-    printStatus("Deleting '$path${fs.path.separator}'.");
+  Future<void> _cleanXcode(XcodeBasedProject xcodeProject) async {
+    if (xcodeProject.existsSync()) {


Could reduce the nesting here with an early return.

Suggested change

if (xcodeProject.existsSync()) {

if (!xcodeProject.existsSync()) {

return;

}

jonahwilliams · 2019-08-21T19:43:41Z

packages/flutter_tools/lib/src/commands/clean.dart

+  }
+
+  @visibleForTesting
+  void deleteFile(FileSystemEntity file) {
    if (file.existsSync()) {


jonahwilliams · 2019-08-21T20:01:07Z

packages/flutter_tools/lib/src/commands/clean.dart

+    // Clean Xcode to remove intermediate DerivedData artifacts.
+    // Do this before removing ephemeral directory, which would delete the xcworkspace.
+    final FlutterProject flutterProject = FlutterProject.current();
+    if (xcode.isInstalledAndMeetsVersionCheck) {


I don't think this check can handle being run on windows host machines, you might need a platform guard as well.

Maybe getCurrentHostPlatform() == HostPlatform.darwin_x64 needs to be added to isInstalledAndMeetsVersionCheck instead?

Sounds reasonable. You can use platform.isMacOS for this check. It is an injected value so that is still testable

stuartmorgan · 2019-08-21T20:17:09Z

packages/flutter_tools/lib/src/commands/clean.dart

+    if (!file.existsSync()) {
+      return;
+    }
+    final Status deletionStatus = logger.startProgress('Deleting ${file.basename}...', timeout: timeoutConfiguration.fastOperation);


It seemed surprising in use that this showed the progress spinner; it was very fast.

stuartmorgan · 2019-08-21T20:18:00Z

packages/flutter_tools/lib/src/commands/clean.dart

+      final Directory xcodeWorkspace = xcodeProject.xcodeWorkspace;
+      final XcodeProjectInfo projectInfo = await xcodeProjectInterpreter.getInfo(xcodeWorkspace.parent.path);
+      for (String scheme in projectInfo.schemes) {
+        xcodeStatus = logger.startProgress('Cleaning Xcode workspace for scheme $scheme...', timeout: timeoutConfiguration.slowOperation);


I'm seeing no newlines after the spinners, which made the output really weird:

$ flutter clean Cleaning Xcode workspace for scheme Build Flutter Bundle... ⣽Deleting build... 105ms Deleting ephemeral... 4ms ⣻$

I just pushed a fix for the progress part, but I suspect the workspace clean is failing. Can you try again with flutter clean -v?

This is the workspace section of the -v output (using the FDE example app; full paths elided):

[ +169 ms] Cleaning Xcode workspace... [ +5 ms] executing: [.../example/macos/] /usr/bin/xcodebuild -list [+1031 ms] Information about project "Runner": Targets: Runner Build Flutter Bundle Build Configurations: Debug Release If no build configuration is specified and -scheme is not passed then "Release" is used. Schemes: Build Flutter Bundle Runner [ +2 ms] executing: [.../example/] /usr/bin/xcodebuild -workspace .../example/macos/Runner.xcworkspace -scheme Build Flutter Bundle -quiet clean [+1082 ms] Exit code 0 from: /usr/bin/xcodebuild -workspace .../example/macos/Runner.xcworkspace -scheme Build Flutter Bundle -quiet clean [ +1 ms] executing: [.../example/] /usr/bin/xcodebuild -workspace .../example/macos/Runner.xcworkspace -scheme Runner -quiet clean [+1088 ms] Exit code 0 from: /usr/bin/xcodebuild -workspace .../example/macos/Runner.xcworkspace -scheme Runner -quiet clean

That looks correct. Do you still see the lack of newlines without -v?

No, formatting without -v looked good in that test.

…ove dead method instead of migrating

jmagman · 2019-08-21T20:45:19Z

packages/flutter_tools/lib/src/commands/run.dart

-  @override
-  void printNoConnectedDevices() {
-    super.printNoConnectedDevices();
-    if (getCurrentHostPlatform() == HostPlatform.darwin_x64 &&


I was about to migrate this if statement but I realized it was dead code as of #35084.

jmagman · 2019-08-21T20:52:47Z

If you have time to give this a quick test to see if it correctly cleans the macOS workspace that would be greatly appreciated!

Works as expected. Do you want to add the Pods directory under macos to the list though? (ios too, I would think, but I don't know if there are special considerations.) It's gitignored, and sometimes needs to be nuked to resolve issues, so seems like it fits the bill for something that clean should eliminate.

(That was actually a comment from @stuartmorgan not me!)

I think people wouldn't expect to have to run pod install again after a flutter clean, so that would be a behavior change.
We need a flutter scrub 😁

jonahwilliams

LGTM once @stuartmorgan's concerns are addressed.

stuartmorgan · 2019-08-21T21:10:09Z

(That was actually a comment from @stuartmorgan not me!)

Sorry, somehow I frequently hit "Edit" instead of "Quote Reply". Usually I notice before submitting, but apparently not always.

I think people wouldn't expect to have to run pod install again after a flutter clean, so that would be a behavior change.

But flutter build/flutter run runs pod install automatically, just as we manage running xcodebuild

jmagman · 2019-08-21T21:15:39Z

(That was actually a comment from @stuartmorgan not me!)

Sorry, somehow I frequently hit "Edit" instead of "Quote Reply". Usually I notice before submitting, but apparently not always.

I think people wouldn't expect to have to run pod install again after a flutter clean, so that would be a behavior change.

But flutter build/flutter run runs pod install automatically, just as we manage running xcodebuild

I was thinking of the case where they flutter clean and go back to the workspace in Xcode and can't just hit build to have all the assets recreated because they are missing their Pods project.

I could add it and we can see if people complain. @xster What do you think?

jmagman · 2019-08-21T21:18:57Z

(That was actually a comment from @stuartmorgan not me!)

Sorry, somehow I frequently hit "Edit" instead of "Quote Reply". Usually I notice before submitting, but apparently not always.

I think people wouldn't expect to have to run pod install again after a flutter clean, so that would be a behavior change.

But flutter build/flutter run runs pod install automatically, just as we manage running xcodebuild

I was thinking of the case where they flutter clean and go back to the workspace in Xcode and can't just hit build to have all the assets recreated because they are missing their Pods project.

I could add it and we can see if people complain. @xster What do you think?

Actually I filed #39003 instead for discussion, it can be a separate PR.

stuartmorgan · 2019-08-21T21:21:53Z

I was thinking of the case where they flutter clean and go back the workspace in Xcode and can't just hit build

If you nuke macos/Flutter/ephemeral, that already doesn't work.

jmagman · 2019-08-21T21:23:24Z

I was thinking of the case where they flutter clean and go back the workspace in Xcode and can't just hit build

If you nuke macos/Flutter/ephemeral, that already doesn't work.

Oh hm that's how the iOS and android ephemeral directories work. I guess the naming did confuse me.

stuartmorgan

LGTM, but I'd prefer we remove Pods for macOS.

jmagman · 2019-08-21T21:25:54Z

LGTM, but I'd prefer we remove Pods for macOS.

I filed #39003, a separate PR wouldn't conflict with this change.

stuartmorgan · 2019-08-21T21:28:05Z

On macOS, the goal is for Flutter/ephemeral to contain everything that's not checked in (Pods is currently a major outlier) generated by Flutter tooling. That includes the xcconfig, which will break building from within Xcode.

xster

LGTM

xster · 2019-08-21T23:12:32Z

packages/flutter_tools/lib/src/commands/clean.dart

@@ -28,38 +32,70 @@ class CleanCommand extends FlutterCommand {

  @override
  Future<FlutterCommandResult> runCommand() async {
+    // Clean Xcode to remove intermediate DerivedData artifacts.
+    // Do this before removing ephemeral directory, which would delete the xcworkspace.
+    final FlutterProject flutterProject = FlutterProject.current();


noob question, do we already know they're in a valid flutter project directory?

flutter clean requires a pubspec.yaml

$ flutter clean Error: No pubspec.yaml file found. This command should be run from the root of your Flutter project. Do not run this command from the root of your git clone of Flutter.

xster · 2019-08-21T23:14:04Z

packages/flutter_tools/lib/src/commands/clean.dart

+    if (!xcodeProject.existsSync()) {
+      return;
+    }
+    final Status xcodeStatus = logger.startProgress('Cleaning Xcode workspace...', timeout: timeoutConfiguration.slowOperation);


I think it'll be possible to have both an iOS and a macOS project right? Should this message be more specific?

xster · 2019-08-21T23:19:26Z

@stuartmorgan, Jenn's been moving the env var setting in the xcconfigs into an env setting .sh. We should be consistent on the 2 platforms.

We should also be consistent about where the outputs of flutter build aar/ios-framework go. If they go to build/, then dependent platform projects will also break if that's where they pointed their dependencies to. So there, it would be consistent with dependent macOS apps not building as well. But I'm not sure if we thought these effects and explicitly decided on them.

PiotrWpl · 2020-02-10T18:05:49Z

Hi @jmagman
maybe You will be able to help us. After upgrading Flutter in our project to 1.12.13+hotfix.7, flutter clean is sometimes unacceptably long :/
For example:

[10:13:15]: $ flutter clean
[10:21:28]: ▸ Cleaning Xcode workspace... 491.4s (!)
[10:21:29]: ▸ Deleting build... 521ms
[10:21:29]: ▸ Deleting .dart_tool... 10ms

I have no idea what's going on :/ it's not like this with every build (usually it's about 40-60s), but it's very annoying and significantly increasing our CI builds and tests.

To give You a better background of why it's important for us:

we have a very custom implementation of build flavours in our Flutter project
because of above, we have multiple pubspec.yaml files where we have different dependencies defined depends on the app variant we are building
that means, we need to do flutter clean and flutter packages get every time when we are changing build variant
but at the same time, most of the code is shared/common so we need to be sure that change for specific app variant didn't break another app variant

To prevent possible build issues we created a couple of: UI tests using Flutter Driver, static code analysis, and unit tests - and use them as a part of our continues integration process.
But now because we have multiple flutter clean calls in the verification procedure, our process is a lot longer than before.

Do You have any idea why cleaning xcode workspace is so long sometimes? Is there any way to make this faster? Or at least debug to better understand the reasons and improve?

BTW: We are using Azure DevOps for our CI/CD. But we have similar issues with flutter clean on our local Macbooks.

Thanks in advance!
Piotr

jmagman · 2020-02-10T20:29:39Z

@PiotrWpl I'm not sure why cleaning the workspace would take that long. Unfortunately running:

$ xcodebuild -workspace /Users/magder/Projects/test_create/ios/Runner.xcworkspace -scheme Runner clean -showBuildTimingSummary

in ios/ didn't give me a real timing summary.

Can you file an issue for why you need to run flutter clean between building different build configs/flavors? You shouldn't need to, and I'm not sure what cache issue you're hitting there.

Can you make your own CI script instead of relying on flutter clean? It's made to be thorough, not performant. It's supposed to be run after upgrades, etc.

flutter/flutter#38992 以下のエラーに対処するため。 ``` error: Could not delete `/Users/runner/runners/2.262.1/work/whimsicalendar/whimsicalendar/build/ios` because it was not created by the build system. ```

Clean Xcode workspace during flutter clean

19ff285

jmagman added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 21, 2019

jmagman requested a review from jonahwilliams August 21, 2019 18:08

jmagman self-assigned this Aug 21, 2019

googlebot added the cla: yes label Aug 21, 2019

jonahwilliams reviewed Aug 21, 2019

View reviewed changes

Reduce nesting

4f4c844

jonahwilliams reviewed Aug 21, 2019

View reviewed changes

stuartmorgan reviewed Aug 21, 2019

View reviewed changes

Add platform check to isInstalledAndMeetsVersionCheck, add tests, rem…

6bb90fa

…ove dead method instead of migrating

jmagman commented Aug 21, 2019

View reviewed changes

Stop all started progresses

9572881

Unused import

93438ed

jonahwilliams approved these changes Aug 21, 2019

View reviewed changes

jmagman mentioned this pull request Aug 21, 2019

Remove generated Pod directory on flutter clean #39003

Open

stuartmorgan approved these changes Aug 21, 2019

View reviewed changes

jmagman added 2 commits August 21, 2019 14:39

M

5051b22

Handle xcode-select on non macOS platforms in tests

98bb395

xster reviewed Aug 21, 2019

View reviewed changes

jmagman merged commit 892d62f into flutter:master Aug 22, 2019

jmagman deleted the clean branch August 22, 2019 01:43

This was referenced Nov 18, 2019

flutter clean doesn't clean everything #12631

Closed

Xcode 11 Beta 6 keeps stale code between clean builds and archive commands #39401

Closed

This was referenced Jan 23, 2020

iPad running iOS 12 -- Stuck on Splash Screen #22765

Closed

iOS: Blank screen in running my flutter app in release mode #36247

Open

agreaves mentioned this pull request May 1, 2020

Xcode clean step hanging when running Flutter clean #56156

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean Xcode workspace during flutter clean #38992

Clean Xcode workspace during flutter clean #38992

jmagman commented Aug 21, 2019

jmagman commented Aug 21, 2019 •

edited by stuartmorgan

codecov bot commented Aug 21, 2019 •

edited

stuartmorgan commented Aug 21, 2019

jonahwilliams Aug 21, 2019

jonahwilliams Aug 21, 2019

jonahwilliams Aug 21, 2019

jmagman Aug 21, 2019

jonahwilliams Aug 21, 2019

stuartmorgan Aug 21, 2019

stuartmorgan Aug 21, 2019

jmagman Aug 21, 2019

stuartmorgan Aug 21, 2019

jmagman Aug 21, 2019

stuartmorgan Aug 21, 2019

jmagman Aug 21, 2019

jmagman commented Aug 21, 2019 •

edited

jonahwilliams left a comment

stuartmorgan commented Aug 21, 2019

jmagman commented Aug 21, 2019 •

edited

jmagman commented Aug 21, 2019

stuartmorgan commented Aug 21, 2019

jmagman commented Aug 21, 2019

stuartmorgan left a comment

jmagman commented Aug 21, 2019

stuartmorgan commented Aug 21, 2019

xster left a comment

xster Aug 21, 2019

jmagman Aug 21, 2019

xster Aug 21, 2019

xster commented Aug 21, 2019

PiotrWpl commented Feb 10, 2020

jmagman commented Feb 10, 2020 •

edited

Clean Xcode workspace during flutter clean #38992

Clean Xcode workspace during flutter clean #38992

Conversation

jmagman commented Aug 21, 2019

Description

Related Issues

Tests

Checklist

Breaking Change

jmagman commented Aug 21, 2019 • edited by stuartmorgan

codecov bot commented Aug 21, 2019 • edited

Codecov Report

stuartmorgan commented Aug 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman commented Aug 21, 2019 • edited

jonahwilliams left a comment

Choose a reason for hiding this comment

stuartmorgan commented Aug 21, 2019

jmagman commented Aug 21, 2019 • edited

jmagman commented Aug 21, 2019

stuartmorgan commented Aug 21, 2019

jmagman commented Aug 21, 2019

stuartmorgan left a comment

Choose a reason for hiding this comment

jmagman commented Aug 21, 2019

stuartmorgan commented Aug 21, 2019

xster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xster commented Aug 21, 2019

PiotrWpl commented Feb 10, 2020

jmagman commented Feb 10, 2020 • edited

jmagman commented Aug 21, 2019 •

edited by stuartmorgan

codecov bot commented Aug 21, 2019 •

edited

jmagman commented Aug 21, 2019 •

edited

jmagman commented Aug 21, 2019 •

edited

jmagman commented Feb 10, 2020 •

edited