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
diff --git a/bin/flutter b/bin/flutter
index 3803576..1ec69b6 100755
--- a/bin/flutter
+++ b/bin/flutter
@@ -7,6 +7,7 @@
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 @@
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
diff --git a/bin/flutter.bat b/bin/flutter.bat
index 4d8580e..d047aa6 100644
--- a/bin/flutter.bat
+++ b/bin/flutter.bat
@@ -6,6 +6,7 @@
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 @@
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%"
diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart
index 5af0f3c..123b752 100644
--- a/packages/flutter_tools/lib/src/artifacts.dart
+++ b/packages/flutter_tools/lib/src/artifacts.dart
@@ -192,12 +192,15 @@
}
}
- 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 @@
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 @@
}
static Directory _getCacheDirForPlatform(String platform) {
- ensureHasSkyEnginePackage();
+ validateSkyEnginePackage();
Directory baseDir = _getBaseCacheDir();
// TODO(jamesr): Add support for more configurations.
String config = 'Release';
diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart
index 4ca8119..127391b 100644
--- a/packages/flutter_tools/lib/src/commands/test.dart
+++ b/packages/flutter_tools/lib/src/commands/test.dart
@@ -20,13 +20,18 @@
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 @@
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
diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart
index 5cab91d..4df0131 100644
--- a/packages/flutter_tools/lib/src/runner/flutter_command.dart
+++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart
@@ -10,22 +10,19 @@
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 @@
}
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();
diff --git a/packages/flutter_tools/test/src/mocks.dart b/packages/flutter_tools/test/src/mocks.dart
index 9af0463..99161d6 100644
--- a/packages/flutter_tools/test/src/mocks.dart
+++ b/packages/flutter_tools/test/src/mocks.dart
@@ -48,5 +48,6 @@
command
..applicationPackages = new MockApplicationPackageStore()
..toolchain = new MockToolchain()
- ..devices = new MockDeviceStore();
+ ..devices = new MockDeviceStore()
+ ..projectRootValidator = () => true;
}
