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

This reverts commit 76cbc462.

packages/flutter_tools/lib/src/cache.dart

@@ -700,6 +700,8 @@ 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,6 +117,7 @@ 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,8 +8,11 @@ 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';
@@ -166,6 +169,32 @@ 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,6 +330,7 @@ class UpdatePackagesCommand extends FlutterCommand {
           context: PubContext.updatePackages,
           directory: tempDir.path,
           upgrade: true,
+          checkLastModified: false,
           offline: offline,
           flutterRootOverride: upgrade
             ? temporaryFlutterSdk.path
@@ -412,6 +413,7 @@ 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,6 +296,7 @@ class UpgradeCommandRunner {
         context: PubContext.pubUpgrade,
         directory: projectRoot,
         upgrade: true,
+        checkLastModified: false,
         generateSyntheticPackage: false,
       );
     }

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

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

packages/flutter_tools/lib/src/context_runner.dart

@@ -236,6 +236,8 @@ 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,6 +80,7 @@ abstract class Pub {
     @required Platform platform,
     @required BotDetector botDetector,
     @required Usage usage,
+    File Function() toolStampFile,
   }) = _DefaultPub;
 
   /// Runs `pub get`.
@@ -92,6 +93,8 @@ abstract class Pub {
     bool skipIfAbsent = false,
     bool upgrade = false,
     bool offline = false,
+    bool checkLastModified = true,
+    bool skipPubspecYamlCheck = false,
     bool generateSyntheticPackage = false,
     String flutterRootOverride,
   });
@@ -138,7 +141,9 @@ class _DefaultPub implements Pub {
     @required Platform platform,
     @required BotDetector botDetector,
     @required Usage usage,
-  }) : _fileSystem = fileSystem,
+   File Function() toolStampFile,
+  }) : _toolStampFile = toolStampFile,
+       _fileSystem = fileSystem,
        _logger = logger,
        _platform = platform,
        _botDetector = botDetector,
@@ -154,6 +159,7 @@ class _DefaultPub implements Pub {
   final Platform _platform;
   final BotDetector _botDetector;
   final Usage _usage;
+  final File Function() _toolStampFile;
 
   @override
   Future<void> get({
@@ -162,52 +168,88 @@ 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'));
 
-    final String command = upgrade ? 'upgrade' : 'get';
-    final Status status = _logger.startProgress(
-      'Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...',
-      timeout: const TimeoutConfiguration().slowOperation,
-    );
-    final bool verbose = _logger.isVerbose;
-    final List<String> args = <String>[
-      if (verbose)
-        '--verbose'
-      else
-        '--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,
+    if (!skipPubspecYamlCheck && !pubSpecYaml.existsSync()) {
+      if (!skipIfAbsent) {
+        throwToolExit('$directory: no pubspec.yaml found');
+      }
+      return;
+    }
+
+    final DateTime originalPubspecYamlModificationTime = pubSpecYaml.lastModifiedSync();
+
+    if (!checkLastModified || _shouldRunPubGet(
+      pubSpecYaml: pubSpecYaml,
+      packageConfigFile: packageConfigFile,
+    )) {
+      final String command = upgrade ? 'upgrade' : 'get';
+      final Status status = _logger.startProgress(
+        'Running "flutter pub $command" in ${_fileSystem.path.basename(directory)}...',
+        timeout: const TimeoutConfiguration().slowOperation,
       );
-      status.stop();
-    // The exception is rethrown, so don't catch only Exceptions.
-    } catch (exception) { // ignore: avoid_catches_without_on_clauses
-      status.cancel();
-      rethrow;
+      final bool verbose = _logger.isVerbose;
+      final List<String> args = <String>[
+        if (verbose)
+          '--verbose'
+        else
+          '--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,
@@ -350,6 +392,23 @@ 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,7 +979,11 @@ 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,6 +743,9 @@ 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).then((void value) {
+      pub.get(context: PubContext.flutterTests, checkLastModified: false).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);
+      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
       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).then((void value) {
+      pub.get(context: PubContext.flutterTests, checkLastModified: false).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).then((void value) {
+      pub.get(context: PubContext.flutterTests, checkLastModified: false).then((void value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -217,6 +217,7 @@ void main() {
     await pub.get(
       context: PubContext.flutterTests,
       generateSyntheticPackage: true,
+      checkLastModified: false,
     );
 
     verify(usage.sendEvent('pub-result', 'flutter-tests', label: 'success')).called(1);
@@ -254,6 +255,7 @@ void main() {
     await pub.get(
       context: PubContext.flutterTests,
       generateSyntheticPackage: true,
+      checkLastModified: false,
     );
 
     expect(
@@ -283,7 +285,7 @@ void main() {
       ),
     );
     try {
-      await pub.get(context: PubContext.flutterTests);
+      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
     } on ToolExit {
       // Ignore.
     }
@@ -312,7 +314,7 @@ void main() {
     fileSystem.file('pubspec.yaml').writeAsStringSync('name: foo');
 
     try {
-      await pub.get(context: PubContext.flutterTests);
+      await pub.get(context: PubContext.flutterTests, checkLastModified: false);
     } on ToolExit {
       // Ignore.
     }
@@ -384,7 +386,7 @@ void main() {
     fileSystem.file('pubspec.yaml')
       ..createSync()
       ..setLastModifiedSync(DateTime(2001));
-    await pub.get(context: PubContext.flutterTests); // pub sets date of .packages to 2002
+    await pub.get(context: PubContext.flutterTests, checkLastModified: true); // pub sets date of .packages to 2002
 
     expect(logger.statusText, 'Running "flutter pub get" in /...\n');
     expect(logger.errorText, isEmpty);
@@ -396,12 +398,44 @@ void main() {
       .setLastModifiedSync(DateTime(2000));
     fileSystem.file('pubspec.yaml')
       .setLastModifiedSync(DateTime(2001));
-    await pub.get(context: PubContext.flutterTests); // pub does nothing
+    await pub.get(context: PubContext.flutterTests, checkLastModified: true); // 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,8 +24,7 @@ final ProcessUtils processUtils = ProcessUtils(processManager: processManager, l
   outputPreferences: OutputPreferences.test(wrapText: true),
   timeoutConfiguration: const TimeoutConfiguration(),
 ));
-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');
+final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter');
 
 /// A test for flutter upgrade & downgrade that checks out a parallel flutter repo.
 void main() {