Fix flutter run cache (#45267)

packages/flutter_tools/lib/src/cache.dart

@@ -63,6 +63,9 @@ class DevelopmentArtifact {
   static const DevelopmentArtifact flutterRunner = DevelopmentArtifact._('flutter_runner', unstable: true);
 
   /// Artifacts required for any development platform.
+  ///
+  /// This does not need to be explicitly returned from requiredArtifacts as
+  /// it will always be downloaded.
   static const DevelopmentArtifact universal = DevelopmentArtifact._('universal');
 
   /// The values of DevelopmentArtifacts.

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

@@ -64,11 +64,6 @@ class AnalyzeCommand extends FlutterCommand {
   @override
   String get description => "Analyze the project's Dart code.";
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   @override
   bool get shouldRunPub {
     // If they're not analyzing the current project.

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

@@ -55,7 +55,6 @@ class BuildAarCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.androidGenSnapshot,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -45,7 +45,6 @@ class BuildApkCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.androidGenSnapshot,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -38,7 +38,6 @@ class BuildAppBundleCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.androidGenSnapshot,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -47,7 +47,6 @@ class BuildFuchsiaCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.fuchsia,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -43,7 +43,6 @@ class BuildIOSCommand extends BuildSubCommand {
 
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
     DevelopmentArtifact.iOS,
   };
 

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

@@ -85,7 +85,6 @@ class BuildIOSFrameworkCommand extends BuildSubCommand {
 
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
     DevelopmentArtifact.iOS,
   };
 

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

@@ -30,7 +30,6 @@ class BuildLinuxCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.linux,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -30,7 +30,6 @@ class BuildMacosCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.macOS,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -30,7 +30,6 @@ class BuildWebCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async =>
       const <DevelopmentArtifact>{
-        DevelopmentArtifact.universal,
         DevelopmentArtifact.web,
       };
 

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

@@ -30,7 +30,6 @@ class BuildWindowsCommand extends BuildSubCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
     DevelopmentArtifact.windows,
-    DevelopmentArtifact.universal,
   };
 
   @override

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

@@ -337,8 +337,6 @@ class CreateCommand extends FlutterCommand {
         'variable was specified. Unable to find package:flutter.', exitCode: 2);
     }
 
-    await Cache.instance.updateAll(<DevelopmentArtifact>{ DevelopmentArtifact.universal });
-
     final String flutterRoot = fs.path.absolute(Cache.flutterRoot);
 
     final String flutterPackagesDirectory = fs.path.join(flutterRoot, 'packages');

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

@@ -33,7 +33,6 @@ class DoctorCommand extends FlutterCommand {
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async {
     return <DevelopmentArtifact>{
-      DevelopmentArtifact.universal,
       // This is required because we use gen_snapshot to check if the host
       // machine can execute the provided artifacts. See `_genSnapshotRuns`
       // in `doctor.dart`.

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

@@ -44,11 +44,6 @@ class FormatCommand extends FlutterCommand {
   @override
   final String description = 'Format one or more dart files.';
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   @override
   String get invocation => '${runner.executableName} $name <one or more paths>';
 

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

@@ -20,11 +20,6 @@ class GenerateCommand extends FlutterCommand {
   @override
   String get name => 'generate';
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   @override
   Future<FlutterCommandResult> runCommand() async {
     Cache.releaseLockEarly();

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

@@ -217,8 +217,6 @@ class IdeConfigCommand extends FlutterCommand {
       throwToolExit('Currently, the only supported IDE is IntelliJ\n$usage', exitCode: 2);
     }
 
-    await Cache.instance.updateAll(<DevelopmentArtifact>{ DevelopmentArtifact.universal });
-
     if (boolArg('update-templates')) {
       _handleTemplateUpdate();
       return null;

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

@@ -37,11 +37,6 @@ class PackagesCommand extends FlutterCommand {
   @override
   final String description = 'Commands for managing Flutter packages.';
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   @override
   Future<FlutterCommandResult> runCommand() async => null;
 }

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

@@ -284,10 +284,6 @@ class RunCommand extends RunCommandBase {
     if (!runningWithPrebuiltApplication) {
       await super.validateCommand();
     }
-    devices = await findAllTargetDevices();
-    if (devices == null) {
-      throwToolExit(null);
-    }
     if (deviceManager.hasSpecifiedAllDevices && runningWithPrebuiltApplication) {
       throwToolExit('Using -d all with --use-application-binary is not supported');
     }
@@ -336,6 +332,11 @@ class RunCommand extends RunCommandBase {
 
     writePidFile(stringArg('pid-file'));
 
+    devices = await findAllTargetDevices();
+    if (devices == null) {
+      throwToolExit(null);
+    }
+
     if (boolArg('machine')) {
       if (devices.length > 1) {
         throwToolExit('--machine does not support -d all.');

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

@@ -106,9 +106,7 @@ class TestCommand extends FastFlutterCommand {
 
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async {
-    final Set<DevelopmentArtifact> results = <DevelopmentArtifact>{
-      DevelopmentArtifact.universal,
-    };
+    final Set<DevelopmentArtifact> results = <DevelopmentArtifact>{};
     if (stringArg('platform') == 'chrome') {
       results.add(DevelopmentArtifact.web);
     }

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

@@ -55,9 +55,7 @@ class UnpackCommand extends FlutterCommand {
 
   @override
   Future<Set<DevelopmentArtifact>> get requiredArtifacts async {
-    final Set<DevelopmentArtifact> result = <DevelopmentArtifact>{
-      DevelopmentArtifact.universal,
-    };
+    final Set<DevelopmentArtifact> result = <DevelopmentArtifact>{};
     final TargetPlatform targetPlatform = getTargetPlatformForName(stringArg('target-platform'));
     switch (targetPlatform) {
       case TargetPlatform.windows_x64:

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

@@ -88,11 +88,6 @@ class UpdatePackagesCommand extends FlutterCommand {
   @override
   final bool hidden;
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   Future<void> _downloadCoverageData() async {
     final Status status = logger.startProgress(
       'Downloading lcov data for package:flutter...',

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

@@ -51,11 +51,6 @@ class UpgradeCommand extends FlutterCommand {
   @override
   bool get shouldUpdateCache => false;
 
-  @override
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
-
   @override
   Future<FlutterCommandResult> runCommand() async {
     await _commandRunner.runCommand(

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -589,9 +589,13 @@ abstract class FlutterCommand extends Command<void> {
   Future<FlutterCommandResult> verifyThenRunCommand(String commandPath) async {
     await validateCommand();
 
-    // Populate the cache. We call this before pub get below so that the sky_engine
-    // package is available in the flutter cache for pub to find.
+    // Populate the cache. We call this before pub get below so that the
+    // sky_engine package is available in the flutter cache for pub to find.
     if (shouldUpdateCache) {
+      // First always update universal artifacts, as some of these (e.g.
+      // idevice_id on macOS) are required to determine `requiredArtifacts`.
+      await cache.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal});
+
       await cache.updateAll(await requiredArtifacts);
     }
 
@@ -617,10 +621,9 @@ abstract class FlutterCommand extends Command<void> {
 
   /// The set of development artifacts required for this command.
   ///
-  /// Defaults to [DevelopmentArtifact.universal].
-  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{
-    DevelopmentArtifact.universal,
-  };
+  /// Defaults to an empty set. Including [DevelopmentArtifact.universal] is
+  /// not required as it is always updated.
+  Future<Set<DevelopmentArtifact>> get requiredArtifacts async => const <DevelopmentArtifact>{};
 
   /// Subclasses must implement this to execute the command.
   /// Optionally provide a [FlutterCommandResult] to send more details about the
@@ -758,9 +761,7 @@ mixin TargetPlatformBasedDevelopmentArtifacts on FlutterCommand {
       return super.requiredArtifacts;
     }
 
-    final Set<DevelopmentArtifact> artifacts = <DevelopmentArtifact>{
-      DevelopmentArtifact.universal,
-    };
+    final Set<DevelopmentArtifact> artifacts = <DevelopmentArtifact>{};
     final DevelopmentArtifact developmentArtifact = _artifactFromTargetPlatform(targetPlatform);
     if (developmentArtifact != null) {
       artifacts.add(developmentArtifact);

packages/flutter_tools/test/commands.shard/hermetic/run_test.dart

@@ -53,13 +53,83 @@ void main() {
       }
     });
 
+    group('cache', () {
+      MemoryFileSystem fs;
+      MockCache mockCache;
+      MockProcessManager mockProcessManager;
+      Directory tempDir;
+
+      setUpAll(() {
+        mockCache = MockCache();
+        fs = MemoryFileSystem();
+        mockProcessManager = MockProcessManager();
+
+        tempDir = fs.systemTempDirectory.createTempSync('flutter_run_test.');
+        fs.currentDirectory = tempDir;
+
+        tempDir.childFile('pubspec.yaml')
+          ..writeAsStringSync('name: flutter_app');
+        tempDir.childFile('.packages')
+          ..writeAsStringSync('# Generated by pub on 2019-11-25 12:38:01.801784.');
+        final Directory libDir = tempDir.childDirectory('lib');
+        libDir.createSync();
+        final File mainFile = libDir.childFile('main.dart');
+        mainFile.writeAsStringSync('void main() {}');
+
+        when(mockDeviceManager.hasSpecifiedDeviceId).thenReturn(false);
+        when(mockDeviceManager.hasSpecifiedAllDevices).thenReturn(false);
+      });
+
+      testUsingContext('updates before checking for devices', () async {
+        final RunCommand command = RunCommand();
+        applyMocksToCommand(command);
+
+        // No devices are attached, we just want to verify update the cache
+        // BEFORE checking for devices
+        when(mockDeviceManager.getDevices()).thenAnswer(
+          (Invocation invocation) => Stream<Device>.fromIterable(<Device>[])
+        );
+        when(mockDeviceManager.findTargetDevices(any)).thenAnswer(
+          (Invocation invocation) => Future<List<Device>>.value(<Device>[])
+        );
+
+        try {
+          await createTestCommandRunner(command).run(<String>[
+            'run',
+            '--no-pub',
+          ]);
+          fail('Exception expected');
+        } on ToolExit catch (e) {
+          // We expect a ToolExit because no devices are attached
+          expect(e.message, null);
+        } catch (e) {
+          fail('ToolExit expected');
+        }
+
+        verifyInOrder(<void>[
+          mockCache.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal}),
+          mockDeviceManager.findTargetDevices(any),
+        ]);
+      }, overrides: <Type, Generator>{
+        ApplicationPackageFactory: () => mockApplicationPackageFactory,
+        Cache: () => mockCache,
+        DeviceManager: () => mockDeviceManager,
+        FileSystem: () => fs,
+        ProcessManager: () => mockProcessManager,
+      });
+    });
+
     group('dart-flags option', () {
       setUpAll(() {
+        final FakeDevice fakeDevice = FakeDevice();
         when(mockDeviceManager.getDevices()).thenAnswer((Invocation invocation) {
           return Stream<Device>.fromIterable(<Device>[
-            FakeDevice(),
+            fakeDevice,
           ]);
         });
+        when(mockDeviceManager.findTargetDevices(any)).thenAnswer(
+          (Invocation invocation) => Future<List<Device>>.value(<Device>[fakeDevice])
+        );
       });
 
       RunCommand command;
@@ -195,11 +265,13 @@ void main() {
       MockWebRunnerFactory mockWebRunnerFactory;
 
       setUpAll(() {
-        when(mockDeviceManager.getDevices()).thenAnswer((Invocation invocation) {
-          return Stream<Device>.fromIterable(<Device>[
-            FakeDevice().._targetPlatform = TargetPlatform.web_javascript,
-          ]);
-        });
+        final FakeDevice fakeDevice = FakeDevice().._targetPlatform = TargetPlatform.web_javascript;
+        when(mockDeviceManager.getDevices()).thenAnswer(
+          (Invocation invocation) => Stream<Device>.fromIterable(<Device>[fakeDevice])
+        );
+        when(mockDeviceManager.findTargetDevices(any)).thenAnswer(
+          (Invocation invocation) => Future<List<Device>>.value(<Device>[fakeDevice])
+        );
       });
 
       RunCommand command;
@@ -279,6 +351,8 @@ void main() {
   });
 }
 
+class MockCache extends Mock implements Cache {}
+
 class MockDeviceManager extends Mock implements DeviceManager {}
 class MockDevice extends Mock implements Device {
   MockDevice(this._targetPlatform);

packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart

@@ -52,7 +52,14 @@ void main() {
     testUsingContext('honors shouldUpdateCache true', () async {
       final DummyFlutterCommand flutterCommand = DummyFlutterCommand(shouldUpdateCache: true);
       await flutterCommand.run();
-      verify(cache.updateAll(any)).called(1);
+      // First call for universal, second for the rest
+      expect(
+        verify(cache.updateAll(captureAny)).captured,
+        <Set<DevelopmentArtifact>>[
+          <DevelopmentArtifact>{DevelopmentArtifact.universal},
+          <DevelopmentArtifact>{},
+        ],
+      );
     },
     overrides: <Type, Generator>{
       Cache: () => cache,