[flutter_tools] remove all pub caching logic (#66776)

There have been some more additional reports of a missing 'package:characters' import after upgrading flutter, as well as problems with detecting the correct language version. This has me concerned that our pub caching logic is incorrect. Instead of the tool attempting to guess when pub should be run, always delegate to pub.

packages/flutter_tools/lib/src/cache.dart

@@ -700,8 +700,6 @@ class PubDependencies extends ArtifactSet {
       context: PubContext.pubGet,
       directory: _fileSystem.path.join(_flutterRoot(), 'packages', 'flutter_tools'),
       generateSyntheticPackage: false,
-      skipPubspecYamlCheck: true,
-      checkLastModified: false,
     );
   }
 }

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

@@ -117,7 +117,6 @@ class PackagesGetCommand extends FlutterCommand {
         directory: directory,
         upgrade: upgrade ,
         offline: boolArg('offline'),
-        checkLastModified: false,
         generateSyntheticPackage: flutterProject.manifest.generateSyntheticPackage,
       );
       pubGetTimer.stop();

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

@@ -8,11 +8,8 @@ import '../asset.dart';
 import '../base/common.dart';
 import '../base/file_system.dart';
 import '../build_info.dart';
-import '../build_system/build_system.dart';
 import '../bundle.dart';
 import '../cache.dart';
-import '../dart/generate_synthetic_packages.dart';
-import '../dart/pub.dart';
 import '../devfs.dart';
 import '../globals.dart' as globals;
 import '../project.dart';
@@ -169,32 +166,6 @@ class TestCommand extends FlutterCommand {
         'directory (or one of its subdirectories).');
     }
     final FlutterProject flutterProject = FlutterProject.current();
-    if (shouldRunPub) {
-      if (flutterProject.manifest.generateSyntheticPackage) {
-        final Environment environment = Environment(
-          artifacts: globals.artifacts,
-          logger: globals.logger,
-          cacheDir: globals.cache.getRoot(),
-          engineVersion: globals.flutterVersion.engineRevision,
-          fileSystem: globals.fs,
-          flutterRootDir: globals.fs.directory(Cache.flutterRoot),
-          outputDir: globals.fs.directory(getBuildDirectory()),
-          processManager: globals.processManager,
-          projectDir: flutterProject.directory,
-        );
-
-        await generateLocalizationsSyntheticPackage(
-          environment: environment,
-          buildSystem: globals.buildSystem,
-        );
-      }
-
-      await pub.get(
-        context: PubContext.getVerifyContext(name),
-        skipPubspecYamlCheck: true,
-        generateSyntheticPackage: flutterProject.manifest.generateSyntheticPackage,
-      );
-    }
     final bool buildTestAssets = boolArg('test-assets');
     final List<String> names = stringsArg('name');
     final List<String> plainNames = stringsArg('plain-name');

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

@@ -330,7 +330,6 @@ class UpdatePackagesCommand extends FlutterCommand {
           context: PubContext.updatePackages,
           directory: tempDir.path,
           upgrade: true,
-          checkLastModified: false,
           offline: offline,
           flutterRootOverride: upgrade
             ? temporaryFlutterSdk.path
@@ -413,7 +412,6 @@ class UpdatePackagesCommand extends FlutterCommand {
       await pub.get(
         context: PubContext.updatePackages,
         directory: dir.path,
-        checkLastModified: false,
         offline: offline,
         generateSyntheticPackage: false,
       );

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

@@ -296,7 +296,6 @@ class UpgradeCommandRunner {
         context: PubContext.pubUpgrade,
         directory: projectRoot,
         upgrade: true,
-        checkLastModified: false,
         generateSyntheticPackage: false,
       );
     }

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

@@ -151,7 +151,6 @@ class VersionCommand extends FlutterCommand {
         context: PubContext.pubUpgrade,
         directory: projectRoot,
         upgrade: true,
-        checkLastModified: false,
         generateSyntheticPackage: false,
       );
     }

packages/flutter_tools/lib/src/context_runner.dart

@@ -236,8 +236,6 @@ Future<T> runInContext<T>(
         botDetector: globals.botDetector,
         platform: globals.platform,
         usage: globals.flutterUsage,
-        // Avoid a circular dependency by making this access lazy.
-        toolStampFile: () => globals.cache.getStampFileFor('flutter_tools'),
       ),
       ShutdownHooks: () => ShutdownHooks(logger: globals.logger),
       Stdio: () => Stdio(),

packages/flutter_tools/lib/src/dart/pub.dart

@@ -80,7 +80,6 @@ abstract class Pub {
     @required Platform platform,
     @required BotDetector botDetector,
     @required Usage usage,
-    File Function() toolStampFile,
   }) = _DefaultPub;
 
   /// Runs `pub get`.
@@ -93,8 +92,6 @@ abstract class Pub {
     bool skipIfAbsent = false,
     bool upgrade = false,
     bool offline = false,
-    bool checkLastModified = true,
-    bool skipPubspecYamlCheck = false,
     bool generateSyntheticPackage = false,
     String flutterRootOverride,
   });
@@ -141,9 +138,7 @@ class _DefaultPub implements Pub {
     @required Platform platform,
     @required BotDetector botDetector,
     @required Usage usage,
-   File Function() toolStampFile,
+  }) : _fileSystem = fileSystem,
-  }) : _toolStampFile = toolStampFile,
-       _fileSystem = fileSystem,
        _logger = logger,
        _platform = platform,
        _botDetector = botDetector,
@@ -159,7 +154,6 @@ class _DefaultPub implements Pub {
   final Platform _platform;
   final BotDetector _botDetector;
   final Usage _usage;
-  final File Function() _toolStampFile;
 
   @override
   Future<void> get({
@@ -168,88 +162,52 @@ class _DefaultPub implements Pub {
     bool skipIfAbsent = false,
     bool upgrade = false,
     bool offline = false,
-    bool checkLastModified = true,
-    bool skipPubspecYamlCheck = false,
     bool generateSyntheticPackage = false,
     String flutterRootOverride,
   }) async {
     directory ??= _fileSystem.currentDirectory.path;
-
-    final File pubSpecYaml = _fileSystem.file(
-      _fileSystem.path.join(directory, 'pubspec.yaml'));
     final File packageConfigFile = _fileSystem.file(
       _fileSystem.path.join(directory, '.dart_tool', 'package_config.json'));
     final Directory generatedDirectory = _fileSystem.directory(
       _fileSystem.path.join(directory, '.dart_tool', 'flutter_gen'));
 
-    if (!skipPubspecYamlCheck && !pubSpecYaml.existsSync()) {
+    final String command = upgrade ? 'upgrade' : 'get';
-      if (!skipIfAbsent) {
+    final Status status = _logger.startProgress(
-        throwToolExit('$directory: no pubspec.yaml found');
+      'Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...',
-      }
+      timeout: const TimeoutConfiguration().slowOperation,
-      return;
+    );
-    }
+    final bool verbose = _logger.isVerbose;
-
+    final List<String> args = <String>[
-    final DateTime originalPubspecYamlModificationTime = pubSpecYaml.lastModifiedSync();
+      if (verbose)
-
+        '--verbose'
-    if (!checkLastModified || _shouldRunPubGet(
+      else
-      pubSpecYaml: pubSpecYaml,
+        '--verbosity=warning',
-      packageConfigFile: packageConfigFile,
+      ...<String>[
-    )) {
+        command,
-      final String command = upgrade ? 'upgrade' : 'get';
+        '--no-precompile',
-      final Status status = _logger.startProgress(
+      ],
-        'Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...',
+      if (offline)
-        timeout: const TimeoutConfiguration().slowOperation,
+        '--offline',
+    ];
+    try {
+      await batch(
+        args,
+        context: context,
+        directory: directory,
+        failureMessage: 'pub $command failed',
+        retry: true,
+        flutterRootOverride: flutterRootOverride,
       );
-      final bool verbose = _logger.isVerbose;
+      status.stop();
-      final List<String> args = <String>[
+    // The exception is rethrown, so don't catch only Exceptions.
-        if (verbose)
+    } catch (exception) { // ignore: avoid_catches_without_on_clauses
-          '--verbose'
+      status.cancel();
-        else
+      rethrow;
-          '--verbosity=warning',
-        ...<String>[
-          command,
-          '--no-precompile',
-        ],
-        if (offline)
-          '--offline',
-      ];
-      try {
-        await batch(
-          args,
-          context: context,
-          directory: directory,
-          failureMessage: 'pub $command failed',
-          retry: true,
-          flutterRootOverride: flutterRootOverride,
-        );
-        status.stop();
-      // The exception is rethrown, so don't catch only Exceptions.
-      } catch (exception) { // ignore: avoid_catches_without_on_clauses
-        status.cancel();
-        rethrow;
-      }
     }
 
     if (!packageConfigFile.existsSync()) {
       throwToolExit('$directory: pub did not create .dart_tools/package_config.json file.');
     }
-    if (pubSpecYaml.lastModifiedSync() != originalPubspecYamlModificationTime) {
-      throwToolExit(
-        '$directory: unexpected concurrent modification of '
-        'pubspec.yaml while running pub.');
-    }
-    // We don't check if dotPackages was actually modified, because as far as we can tell sometimes
-    // pub will decide it does not need to actually modify it.
-    final DateTime now = DateTime.now();
-    if (now.isBefore(originalPubspecYamlModificationTime)) {
-      _logger.printError(
-        'Warning: File "${_fileSystem.path.absolute(pubSpecYaml.path)}" was created in the future. '
-        'Optimizations that rely on comparing time stamps will be unreliable. Check your '
-        'system clock for accuracy.\n'
-        'The timestamp was: $originalPubspecYamlModificationTime\n'
-        'The time now is: $now'
-      );
-    }
     await _updatePackageConfig(
       packageConfigFile,
       generatedDirectory,
@@ -392,23 +350,6 @@ class _DefaultPub implements Pub {
     return <String>[sdkPath, ...arguments];
   }
 
-  bool _shouldRunPubGet({ @required File pubSpecYaml, @required File packageConfigFile }) {
-    if (!packageConfigFile.existsSync()) {
-      return true;
-    }
-    final DateTime dotPackagesLastModified = packageConfigFile.lastModifiedSync();
-    if (pubSpecYaml.lastModifiedSync().isAfter(dotPackagesLastModified)) {
-      return true;
-    }
-    final File toolStampFile = _toolStampFile != null ? _toolStampFile() : null;
-    if (toolStampFile != null &&
-        toolStampFile.existsSync() &&
-        toolStampFile.lastModifiedSync().isAfter(dotPackagesLastModified)) {
-      return true;
-    }
-    return false;
-  }
-
   // Returns the environment value that should be used when running pub.
   //
   // Includes any existing environment variable, if one exists.

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

@@ -950,9 +950,9 @@ abstract class FlutterCommand extends Command<void> {
       // First always update universal artifacts, as some of these (e.g.
       // ios-deploy on macOS) are required to determine `requiredArtifacts`.
       await globals.cache.updateAll(<DevelopmentArtifact>{DevelopmentArtifact.universal});
-
       await globals.cache.updateAll(await requiredArtifacts);
     }
+    Cache.releaseLock();
 
     await validateCommand();
 
@@ -979,11 +979,7 @@ abstract class FlutterCommand extends Command<void> {
         context: PubContext.getVerifyContext(name),
         generateSyntheticPackage: project.manifest.generateSyntheticPackage,
       );
-      // All done updating dependencies. Release the cache lock.
-      Cache.releaseLock();
       await project.ensureReadyForPlatformSpecificTooling(checkProjects: true);
-    } else {
-      Cache.releaseLock();
     }
 
     setupApplicationPackages();

packages/flutter_tools/test/general.shard/cache_test.dart

@@ -743,9 +743,6 @@ void main() {
     verify(pub.get(
       context: PubContext.pubGet,
       directory: 'packages/flutter_tools',
-      generateSyntheticPackage: false,
-      skipPubspecYamlCheck: true,
-      checkLastModified: false,
     )).called(1);
   });
 }

packages/flutter_tools/test/general.shard/dart/pub_get_test.dart

@@ -49,7 +49,7 @@ void main() {
     FakeAsync().run((FakeAsync time) {
       expect(processMock.lastPubEnvironment, isNull);
       expect(logger.statusText, '');
-      pub.get(context: PubContext.flutterTests, checkLastModified: false).then((void value) {
+      pub.get(context: PubContext.flutterTests).then((void value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -114,7 +114,7 @@ void main() {
       processManager: MockProcessManager(66, stderr: 'err1\nerr2\nerr3\n', stdout: 'out1\nout2\nout3\n'),
     );
     try {
-      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
+      await pub.get(context: PubContext.flutterTests);
       throw AssertionError('pubGet did not fail');
     } on ToolExit catch (error) {
       expect(error.message, 'pub get failed (66; err3)');
@@ -149,7 +149,7 @@ void main() {
       MockDirectory.findCache = true;
       expect(processMock.lastPubEnvironment, isNull);
       expect(processMock.lastPubCache, isNull);
-      pub.get(context: PubContext.flutterTests, checkLastModified: false).then((void value) {
+      pub.get(context: PubContext.flutterTests).then((void value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -182,7 +182,7 @@ void main() {
       expect(processMock.lastPubCache, isNull);
 
       String error;
-      pub.get(context: PubContext.flutterTests, checkLastModified: false).then((void value) {
+      pub.get(context: PubContext.flutterTests).then((void value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -217,7 +217,6 @@ void main() {
     await pub.get(
       context: PubContext.flutterTests,
       generateSyntheticPackage: true,
-      checkLastModified: false,
     );
 
     verify(usage.sendEvent('pub-result', 'flutter-tests', label: 'success')).called(1);
@@ -255,7 +254,6 @@ void main() {
     await pub.get(
       context: PubContext.flutterTests,
       generateSyntheticPackage: true,
-      checkLastModified: false,
     );
 
     expect(
@@ -285,7 +283,7 @@ void main() {
       ),
     );
     try {
-      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
+      await pub.get(context: PubContext.flutterTests);
     } on ToolExit {
       // Ignore.
     }
@@ -314,7 +312,7 @@ void main() {
     fileSystem.file('pubspec.yaml').writeAsStringSync('name: foo');
 
     try {
-      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
+      await pub.get(context: PubContext.flutterTests);
     } on ToolExit {
       // Ignore.
     }
@@ -386,7 +384,7 @@ void main() {
     fileSystem.file('pubspec.yaml')
       ..createSync()
       ..setLastModifiedSync(DateTime(2001));
-    await pub.get(context: PubContext.flutterTests, checkLastModified: true); // pub sets date of .packages to 2002
+    await pub.get(context: PubContext.flutterTests); // pub sets date of .packages to 2002
 
     expect(logger.statusText, 'Running "flutter pub get" in /...\n');
     expect(logger.errorText, isEmpty);
@@ -398,44 +396,12 @@ void main() {
       .setLastModifiedSync(DateTime(2000));
     fileSystem.file('pubspec.yaml')
       .setLastModifiedSync(DateTime(2001));
-    await pub.get(context: PubContext.flutterTests, checkLastModified: true); // pub does nothing
+    await pub.get(context: PubContext.flutterTests); // pub does nothing
 
     expect(logger.statusText, 'Running "flutter pub get" in /...\n');
     expect(logger.errorText, isEmpty);
     expect(fileSystem.file('pubspec.yaml').lastModifiedSync(), DateTime(2001)); // because nothing should touch it
     logger.clear();
-
-    // bad scenario 2: pub changes pubspec.yaml instead
-    fileSystem.file('.dart_tool/package_config.json')
-      .setLastModifiedSync(DateTime(2000));
-    fileSystem.file('pubspec.yaml')
-      .setLastModifiedSync(DateTime(2001));
-    try {
-      await pub.get(context: PubContext.flutterTests, checkLastModified: true);
-      expect(true, isFalse, reason: 'pub.get did not throw');
-    } on ToolExit catch (error) {
-      expect(error.message, '/: unexpected concurrent modification of pubspec.yaml while running pub.');
-    }
-    expect(logger.statusText, 'Running "flutter pub get" in /...\n');
-    expect(logger.errorText, isEmpty);
-    expect(fileSystem.file('pubspec.yaml').lastModifiedSync(), DateTime(2002)); // because fake pub above touched it
-
-    // bad scenario 3: pubspec.yaml was created in the future
-    fileSystem.file('.dart_tool/package_config.json')
-      .setLastModifiedSync(DateTime(2000));
-    fileSystem.file('pubspec.yaml')
-      .setLastModifiedSync(DateTime(9999));
-    assert(DateTime(9999).isAfter(DateTime.now()));
-
-    await pub.get(context: PubContext.flutterTests, checkLastModified: true); // pub does nothing
-
-    expect(logger.statusText, contains('Running "flutter pub get" in /...\n'));
-    expect(logger.errorText, startsWith(
-      'Warning: File "/pubspec.yaml" was created in the future. Optimizations that rely on '
-      'comparing time stamps will be unreliable. Check your system clock for accuracy.\n'
-      'The timestamp was:'
-    ));
-    logger.clear();
   });
 }
 

packages/flutter_tools/test/integration.shard/downgrade_upgrade_integration_test.dart

@@ -24,7 +24,8 @@ final ProcessUtils processUtils = ProcessUtils(processManager: processManager, l
   outputPreferences: OutputPreferences.test(wrapText: true),
   timeoutConfiguration: const TimeoutConfiguration(),
 ));
-final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter');
+final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', platform.isWindows ? 'flutter.bat' : 'flutter');
+final String dartBin = fileSystem.path.join(getFlutterRoot(), 'bin', platform.isWindows ? 'dart.bat' : 'dart');
 
 /// A test for flutter upgrade & downgrade that checks out a parallel flutter repo.
 void main() {