Merge pull request #1483 from collinjackson/engine_safety

In addition to checking for the existence of the engine, ensure that it’s the correct version

bin/flutter

@@ -7,6 +7,7 @@ set -e
 
 export FLUTTER_ROOT=$(dirname $(dirname "${BASH_SOURCE[0]}"))
 FLUTTER_TOOLS_DIR="$FLUTTER_ROOT/packages/flutter_tools"
+FLUTTER_DIR="$FLUTTER_ROOT/packages/flutter"
 SNAPSHOT_PATH="$FLUTTER_ROOT/bin/cache/flutter_tools.snapshot"
 STAMP_PATH="$FLUTTER_ROOT/bin/cache/flutter_tools.stamp"
 SCRIPT_PATH="$FLUTTER_TOOLS_DIR/bin/flutter_tools.dart"
@@ -18,6 +19,7 @@ REVISION=`(cd "$FLUTTER_ROOT"; git rev-parse HEAD)`
 if [ ! -f "$SNAPSHOT_PATH" ] || [ ! -f "$STAMP_PATH" ] || [ `cat "$STAMP_PATH"` != "$REVISION" ] || [ "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]; then
   echo Updating flutter tool...
   (cd "$FLUTTER_TOOLS_DIR"; pub get > /dev/null)
+  (cd "$FLUTTER_DIR"; pub get > /dev/null)  # Allows us to check if sky_engine's REVISION is correct
   $DART --snapshot="$SNAPSHOT_PATH" --package-root="$FLUTTER_TOOLS_DIR/packages" "$SCRIPT_PATH"
   echo -n $REVISION > "$STAMP_PATH"
 fi

bin/flutter.bat

@@ -6,6 +6,7 @@ REM found in the LICENSE file.
 SETLOCAL ENABLEDELAYEDEXPANSION
 FOR %%i IN ("%~dp0..") DO SET "flutter_root=%%~fi" REM Get the parent directory
 SET flutter_tools_dir=%flutter_root%\packages\flutter_tools
+SET flutter_dir=%flutter_root%\packages\flutter
 SET snapshot_path=%flutter_root%\bin\cache\flutter_tools.snapshot
 SET stamp_path=%flutter_root%\bin\cache\flutter_tools.stamp
 SET script_path=%flutter_tools_dir%\bin\flutter_tools.dart
@@ -33,6 +34,9 @@ GOTO :after_snapshot
 CD "%flutter_tools_dir%"
 ECHO Updating flutter tool...
 CALL pub.bat get
+CD "%flutter_dir"
+REM Allows us to check if sky_engine's REVISION is correct
+CALL pub.bat get
 CD "%flutter_root%"
 CALL %dart% --snapshot="%snapshot_path%" --package-root="%flutter_tools_dir%\packages" "%script_path%"
 <nul SET /p=%revision%> "%stamp_path%"

packages/flutter_tools/lib/src/artifacts.dart

@@ -192,12 +192,15 @@ class ArtifactStore {
     }
   }
 
-  static void ensureHasSkyEnginePackage() {
-    Directory skyEnginePackage = new Directory(path.join(packageRoot, 'sky_engine'));
-    if (!skyEnginePackage.existsSync()) {
+  static void validateSkyEnginePackage() {
+    if (engineRevision == null) {
       printError("Cannot locate the sky_engine package; did you include 'flutter' in your pubspec.yaml file?");
       throw new ProcessExit(2);
     }
+    if (engineRevision != expectedEngineRevision) {
+      printError("Error: incompatible sky_engine package; please run 'pub get' to get the correct one.\n");
+      throw new ProcessExit(2);
+    }
   }
 
   static String _engineRevision;
@@ -211,6 +214,18 @@ class ArtifactStore {
     return _engineRevision;
   }
 
+  static String _expectedEngineRevision;
+
+  static String get expectedEngineRevision {
+    if (_expectedEngineRevision == null) {
+      // TODO(jackson): Parse the .packages file and use the path from there instead
+      File revisionFile = new File(path.join(flutterRoot, 'packages', 'flutter', 'packages', 'sky_engine', 'REVISION'));
+      if (revisionFile.existsSync())
+        _expectedEngineRevision = revisionFile.readAsStringSync();
+    }
+    return _expectedEngineRevision;
+  }
+
   static String getCloudStorageBaseUrl(String platform) {
     return _getCloudStorageBaseUrl(
       platform: platform,
@@ -295,7 +310,7 @@ class ArtifactStore {
   }
 
   static Directory _getCacheDirForPlatform(String platform) {
-    ensureHasSkyEnginePackage();
+    validateSkyEnginePackage();
     Directory baseDir = _getBaseCacheDir();
     // TODO(jamesr): Add support for more configurations.
     String config = 'Release';

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

@@ -20,13 +20,18 @@ class TestCommand extends FlutterCommand {
 
   bool get requiresProjectRoot => false;
 
-  String get projectRootValidationErrorMessage {
-    return 'Error: No pubspec.yaml file found.\n'
-      'If you wish to run the tests in the Flutter repository\'s \'flutter\' package,\n'
-      'pass --flutter-repo before any test paths. Otherwise, run this command from the\n'
-      'root of your project. Test files must be called *_test.dart and must reside in\n'
-      'the package\'s \'test\' directory (or one of its subdirectories).';
-  }
+  @override
+  Validator projectRootValidator = () {
+    if (!FileSystemEntity.isFileSync('pubspec.yaml')) {
+      printError('Error: No pubspec.yaml file found.\n'
+        'If you wish to run the tests in the Flutter repository\'s \'flutter\' package,\n'
+        'pass --flutter-repo before any test paths. Otherwise, run this command from the\n'
+        'root of your project. Test files must be called *_test.dart and must reside in\n'
+        'the package\'s \'test\' directory (or one of its subdirectories).');
+      return false;
+    }
+    return true;
+  };
 
   Future<String> _getShellPath(BuildConfiguration config) async {
     if (config.type == BuildType.prebuilt) {
@@ -80,7 +85,7 @@ class TestCommand extends FlutterCommand {
     List<String> testArgs = argResults.rest.map((String testPath) => path.absolute(testPath)).toList();
 
     final bool runFlutterTests = argResults['flutter-repo'];
-    if (!runFlutterTests && !validateProjectRoot())
+    if (!runFlutterTests && !projectRootValidator())
       return 1;
 
     // If we're running the flutter tests, we want to use the packages directory

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

@@ -10,22 +10,19 @@ import 'package:args/command_runner.dart';
 import '../application_package.dart';
 import '../base/context.dart';
 import '../build_configuration.dart';
+import '../artifacts.dart';
 import '../device.dart';
 import '../toolchain.dart';
 import 'flutter_command_runner.dart';
 
+typedef bool Validator();
+
 abstract class FlutterCommand extends Command {
   FlutterCommandRunner get runner => super.runner;
 
   /// Whether this command needs to be run from the root of a project.
   bool get requiresProjectRoot => true;
 
-  String get projectRootValidationErrorMessage {
-    return 'Error: No pubspec.yaml file found.\n'
-      'This command should be run from the root of your Flutter project. Do not run\n'
-      'this command from the root of your git clone of Flutter.';
-  }
-
   List<BuildConfiguration> get buildConfigurations => runner.buildConfigurations;
 
   Future downloadApplicationPackages() async {
@@ -49,18 +46,22 @@ abstract class FlutterCommand extends Command {
   }
 
   Future<int> run() async {
-    if (requiresProjectRoot && !validateProjectRoot())
+    if (requiresProjectRoot && !projectRootValidator())
       return 1;
     return await runInProject();
   }
 
-  bool validateProjectRoot() {
+  // This is a field so that you can modify the value for testing.
+  Validator projectRootValidator = () {
     if (!FileSystemEntity.isFileSync('pubspec.yaml')) {
-      printError(projectRootValidationErrorMessage);
+      printError('Error: No pubspec.yaml file found.\n'
+        'This command should be run from the root of your Flutter project.\n'
+        'Do not run this command from the root of your git clone of Flutter.');
       return false;
     }
+    ArtifactStore.validateSkyEnginePackage();
     return true;
-  }
+  };
 
   Future<int> runInProject();
 

packages/flutter_tools/test/src/mocks.dart

@@ -48,5 +48,6 @@ void applyMocksToCommand(FlutterCommand command) {
   command
     ..applicationPackages = new MockApplicationPackageStore()
     ..toolchain = new MockToolchain()
-    ..devices = new MockDeviceStore();
+    ..devices = new MockDeviceStore()
+    ..projectRootValidator = () => true;
 }