From 2641ca6815bf4555ff8c69ddd4b0a7c9baf5694c Mon Sep 17 00:00:00 2001
From: "nweiz@google.com" <nweiz@google.com>
Date: Fri, 5 Sep 2014 23:25:11 +0000
Subject: [PATCH] Use the VM to detect when a precompiled executable is
 out-of-date.

R=rnystrom@google.com

Review URL: https://codereview.chromium.org//548603002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@39938 260f80e4-7a28-3924-810f-c04153c831b5
---
 lib/src/entrypoint.dart                       |   1 +
 lib/src/executable.dart                       |  77 +++++++++++-------
 lib/src/global_packages.dart                  |  18 ++--
 lib/src/utils.dart                            |   5 +-
 test/asset/out-of-date.snapshot               | Bin 0 -> 610 bytes
 test/descriptor.dart                          |   8 ++
 .../snapshots_git_executables_test.dart       |   1 -
 ...=> snapshots_hosted_executables_test.dart} |   1 -
 ...recompiles_if_sdk_is_out_of_date_test.dart |   6 +-
 ...mpiles_if_the_sdk_is_out_of_date_test.dart |   3 +-
 test/test_pub.dart                            |   4 +
 11 files changed, 70 insertions(+), 54 deletions(-)
 create mode 100644 test/asset/out-of-date.snapshot
 rename test/global/activate/{snaphots_hosted_executables_test.dart => snapshots_hosted_executables_test.dart} (96%)

diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart
index c1f0e7d5..fddd024b 100644
--- a/lib/src/entrypoint.dart
+++ b/lib/src/entrypoint.dart
@@ -164,6 +164,7 @@ class Entrypoint {
 
     // If the existing executable was compiled with a different SDK, we need to
     // recompile regardless of what changed.
+    // TODO(nweiz): Use the VM to check this when issue 20802 is fixed.
     var sdkMatches = fileExists(sdkVersionPath) &&
         readTextFile(sdkVersionPath) == "${sdk.version}\n";
     if (!sdkMatches) changed = null;
diff --git a/lib/src/executable.dart b/lib/src/executable.dart
index 3976d0d5..b4d6cf50 100644
--- a/lib/src/executable.dart
+++ b/lib/src/executable.dart
@@ -16,7 +16,6 @@ import 'entrypoint.dart';
 import 'exit_codes.dart' as exit_codes;
 import 'io.dart';
 import 'log.dart' as log;
-import 'sdk.dart' as sdk;
 import 'utils.dart';
 
 /// Runs [executable] from [package] reachable from [entrypoint].
@@ -131,45 +130,61 @@ Future<int> runExecutable(Entrypoint entrypoint, String package,
 /// Runs the snapshot at [path] with [args] and hooks its stdout, stderr, and
 /// sdtin to this process's.
 ///
+/// If [recompile] is passed, it's called if the snapshot is out-of-date. It's
+/// expected to regenerate a snapshot at [path], after which the snapshot will
+/// be re-run. It may return a Future.
+///
+/// If [checked] is set, runs the snapshot in checked mode.
+///
 /// Returns the snapshot's exit code.
 ///
 /// This doesn't do any validation of the snapshot's SDK version.
-Future<int> runSnapshot(String path, Iterable<String> args) async {
+Future<int> runSnapshot(String path, Iterable<String> args, {recompile(),
+    bool checked: false}) async {
   var vmArgs = [path]..addAll(args);
 
-  var process = await Process.start(Platform.executable, vmArgs);
-  // Note: we're not using process.std___.pipe(std___) here because
-  // that prevents pub from also writing to the output streams.
-  process.stderr.listen(stderr.add);
-  process.stdout.listen(stdout.add);
-  stdin.listen(process.stdin.add);
+  // TODO(nweiz): pass a flag to silence the "Wrong full snapshot version"
+  // message when issue 20784 is fixed.
+  if (checked) vmArgs.insert(0, "--checked");
 
-  return process.exitCode;
-}
+  // We need to split stdin so that we can send the same input both to the
+  // first and second process, if we start more than one.
+  var stdin1;
+  var stdin2;
+  if (recompile == null) {
+    stdin1 = stdin;
+  } else {
+    var pair = tee(stdin);
+    stdin1 = pair.first;
+    stdin2 = pair.last;
+  }
 
-/// Runs the executable snapshot at [snapshotPath].
-Future<int> _runCachedExecutable(Entrypoint entrypoint, String snapshotPath,
-    List<String> args) async {
-  // If the snapshot was compiled with a different SDK version, we need to
-  // recompile it.
-  var sdkVersionPath = p.join(".pub", "bin", "sdk-version");
-  if (!fileExists(sdkVersionPath) ||
-      readTextFile(sdkVersionPath) != "${sdk.version}\n") {
-    log.fine("Precompiled executables are out of date.");
-    await entrypoint.precompileExecutables();
+  runProcess(input) async {
+    var process = await Process.start(Platform.executable, vmArgs);
+
+    // Note: we're not using process.std___.pipe(std___) here because
+    // that prevents pub from also writing to the output streams.
+    process.stderr.listen(stderr.add);
+    process.stdout.listen(stdout.add);
+    input.listen(process.stdin.add);
+
+    return process.exitCode;
   }
 
-  // TODO(rnystrom): Use cascade here when async_await compiler supports it.
-  // See: https://github.com/dart-lang/async_await/issues/26.
-  var vmArgs = ["--checked", snapshotPath];
-  vmArgs.addAll(args);
+  var exitCode = await runProcess(stdin1);
+  if (recompile == null || exitCode != 255) return exitCode;
 
-  var process = await Process.start(Platform.executable, vmArgs);
-  // Note: we're not using process.std___.pipe(std___) here because
-  // that prevents pub from also writing to the output streams.
-  process.stderr.listen(stderr.add);
-  process.stdout.listen(stdout.add);
-  stdin.listen(process.stdin.add);
+  // Exit code 255 indicates that the snapshot version was out-of-date. If we
+  // can recompile, do so.
+  await recompile();
+  return runProcess(stdin2);
+}
 
-  return process.exitCode;
+/// Runs the executable snapshot at [snapshotPath].
+Future<int> _runCachedExecutable(Entrypoint entrypoint, String snapshotPath,
+    List<String> args) {
+  return runSnapshot(snapshotPath, args, checked: true, recompile: () {
+    log.fine("Precompiled executable is out of date.");
+    return entrypoint.precompileExecutables();
+  });
 }
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart
index be861480..6f3da60e 100644
--- a/lib/src/global_packages.dart
+++ b/lib/src/global_packages.dart
@@ -18,9 +18,7 @@ import 'lock_file.dart';
 import 'log.dart' as log;
 import 'package.dart';
 import 'pubspec.dart';
-import 'package_graph.dart';
 import 'system_cache.dart';
-import 'sdk.dart' as sdk;
 import 'solver/version_solver.dart';
 import 'source/cached.dart';
 import 'source/git.dart';
@@ -149,9 +147,7 @@ class GlobalPackages {
   Future _precompileExecutables(Entrypoint entrypoint, String package) {
     return log.progress("Precompiling executables", () {
       var binDir = p.join(_directory, package, 'bin');
-      var sdkVersionPath = p.join(binDir, 'sdk-version');
       cleanDir(binDir);
-      writeTextFile(sdkVersionPath, "${sdk.version}\n");
 
       return AssetEnvironment.create(entrypoint, BarbackMode.RELEASE,
           useDart2JS: false).then((environment) {
@@ -314,18 +310,14 @@ class GlobalPackages {
       log.verbosity = log.Verbosity.WARNING;
     }
 
-    return syncFuture(() {
-      var sdkVersionPath = p.join(binDir, 'sdk-version');
-      var snapshotVersion = readTextFile(sdkVersionPath);
-      if (snapshotVersion == "${sdk.version}\n") return null;
-      log.fine("$package:$executable was compiled with Dart "
-          "${snapshotVersion.trim()} and needs to be recompiled.");
-
+    var snapshotPath = p.join(binDir, '$executable.dart.snapshot');
+    return exe.runSnapshot(snapshotPath, args, recompile: () {
+      log.fine("$package:$executable is out of date and needs to be "
+          "recompiled.");
       return find(package)
           .then((entrypoint) => entrypoint.loadPackageGraph())
           .then((graph) => _precompileExecutables(graph.entrypoint, package));
-    }).then((_) =>
-        exe.runSnapshot(p.join(binDir, '$executable.dart.snapshot'), args));
+    });
   }
 
   /// Gets the path to the lock file for an activated cached package with
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index 3abeefdb..b44a00d7 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -8,7 +8,10 @@ library pub.utils;
 import 'dart:async';
 import "dart:convert";
 import 'dart:io';
-@MirrorsUsed(targets: 'pub.io')
+
+// This is used by [libraryPath]. It must be kept up-to-date with all libraries
+// whose paths are looked up using that function.
+@MirrorsUsed(targets: ['pub.io', 'test_pub'])
 import 'dart:mirrors';
 
 import "package:crypto/crypto.dart";
diff --git a/test/asset/out-of-date.snapshot b/test/asset/out-of-date.snapshot
new file mode 100644
index 0000000000000000000000000000000000000000..38c3b9df5d1ac508ace259d37560ff1a2a36b4d5
GIT binary patch
literal 610
zcmezR_0F9*CI&EIgis83I|a@jJjlqv$b8^H=<d7hhZsr@I0?pQA4$v1Nww0~*DozD
z($C3HPR!9y&(BZKN!88INzu>9&rJpM^-~gyN+3i@YH^7kh`SeL%9R5SW+s*vCgzp~
zrgy6kJ_2HnLwD^DG90iyQhku&#9j~p3Z4Mrt$X(xf<zyIML`lv84jF000ayQd`$Zf
z8~~cgb3)HQvhGN3VrJgmtpePq4jtUTA1EX8z;-v#-PH#VK@Hlv_inEMP(2I9{<u4l
z;nv45jZN)e8^1Mm9%yLD1iA6pUjHK?=bYm4(6>2MP?VWha(61j50H&#iV95=j(Vi#
z<m4-sAJIPobiVo#u$Q*(1$vQn573DRKp5uDy@yOeMsgh5d*Hy%0SAtdhzSNc*%iXR
i+X_q|9FChI4h{tg9@^`DBoZPEa&#q#d0=lXgaQDCluOM3

literal 0
HcmV?d00001

diff --git a/test/descriptor.dart b/test/descriptor.dart
index aec82a36..67db4724 100644
--- a/test/descriptor.dart
+++ b/test/descriptor.dart
@@ -9,6 +9,7 @@ import 'package:oauth2/oauth2.dart' as oauth2;
 import 'package:scheduled_test/scheduled_server.dart';
 import 'package:scheduled_test/descriptor.dart';
 
+import '../lib/src/io.dart';
 import '../lib/src/utils.dart';
 import 'descriptor/git.dart';
 import 'descriptor/tar.dart';
@@ -35,6 +36,13 @@ Descriptor get validPackage => dir(appPath, [
   ])
 ]);
 
+/// Returns a descriptor of a snapshot that can't be run by the current VM.
+///
+/// This snapshot was generated by the VM on r39611, the revision immediately
+/// before snapshot versioning was added.
+FileDescriptor outOfDateSnapshot(String name) =>
+    binaryFile(name, readBinaryFile(testAssetPath('out-of-date.snapshot')));
+
 /// Describes a file named `pubspec.yaml` with the given YAML-serialized
 /// [contents], which should be a serializable object.
 ///
diff --git a/test/global/activate/snapshots_git_executables_test.dart b/test/global/activate/snapshots_git_executables_test.dart
index 35274c14..cfabaa9a 100644
--- a/test/global/activate/snapshots_git_executables_test.dart
+++ b/test/global/activate/snapshots_git_executables_test.dart
@@ -35,7 +35,6 @@ main() {
         d.dir('foo', [
           d.matcherFile('pubspec.lock', contains('1.0.0')),
           d.dir('bin', [
-            d.file('sdk-version', '0.1.2+3\n'),
             d.matcherFile('hello.dart.snapshot', contains('hello!')),
             d.matcherFile('goodbye.dart.snapshot', contains('goodbye!')),
             d.nothing('shell.sh.snapshot'),
diff --git a/test/global/activate/snaphots_hosted_executables_test.dart b/test/global/activate/snapshots_hosted_executables_test.dart
similarity index 96%
rename from test/global/activate/snaphots_hosted_executables_test.dart
rename to test/global/activate/snapshots_hosted_executables_test.dart
index 79ce0d7e..5bd0257e 100644
--- a/test/global/activate/snaphots_hosted_executables_test.dart
+++ b/test/global/activate/snapshots_hosted_executables_test.dart
@@ -33,7 +33,6 @@ main() {
         d.dir('foo', [
           d.matcherFile('pubspec.lock', contains('1.0.0')),
           d.dir('bin', [
-            d.file('sdk-version', '0.1.2+3\n'),
             d.matcherFile('hello.dart.snapshot', contains('hello!')),
             d.matcherFile('goodbye.dart.snapshot', contains('goodbye!')),
             d.nothing('shell.sh.snapshot'),
diff --git a/test/global/run/recompiles_if_sdk_is_out_of_date_test.dart b/test/global/run/recompiles_if_sdk_is_out_of_date_test.dart
index df4b0297..bb38dd7d 100644
--- a/test/global/run/recompiles_if_sdk_is_out_of_date_test.dart
+++ b/test/global/run/recompiles_if_sdk_is_out_of_date_test.dart
@@ -24,10 +24,7 @@ main() {
     d.dir(cachePath, [
       d.dir('global_packages', [
         d.dir('foo', [
-          d.dir('bin', [
-            d.file('sdk-version', '0.0.1\n'),
-            d.file('script.dart.snapshot', 'junk')
-          ])
+          d.dir('bin', [d.outOfDateSnapshot('script.dart.snapshot')])
         ])
       ])
     ]).create();
@@ -43,7 +40,6 @@ main() {
       d.dir('global_packages', [
         d.dir('foo', [
           d.dir('bin', [
-            d.file('sdk-version', '0.1.2+3\n'),
             d.matcherFile('script.dart.snapshot', contains('ok'))
           ])
         ])
diff --git a/test/snapshot/recompiles_if_the_sdk_is_out_of_date_test.dart b/test/snapshot/recompiles_if_the_sdk_is_out_of_date_test.dart
index 41870c7f..1d8d213c 100644
--- a/test/snapshot/recompiles_if_the_sdk_is_out_of_date_test.dart
+++ b/test/snapshot/recompiles_if_the_sdk_is_out_of_date_test.dart
@@ -26,8 +26,7 @@ main() {
     pubGet(output: contains("Precompiled foo:hello."));
 
     d.dir(p.join(appPath, '.pub', 'bin'), [
-      d.file('sdk-version', '0.0.1'),
-      d.dir('foo', [d.file('hello.dart.snapshot', 'junk')])
+      d.dir('foo', [d.outOfDateSnapshot('hello.dart.snapshot')])
     ]).create();
 
     var process = pubRun(args: ['foo:hello']);
diff --git a/test/test_pub.dart b/test/test_pub.dart
index d2e95893..a5ffe843 100644
--- a/test/test_pub.dart
+++ b/test/test_pub.dart
@@ -825,6 +825,10 @@ Map packageMap(String name, String version, [Map dependencies]) {
   return package;
 }
 
+/// Resolves [target] relative to the path to pub's `test/asset` directory.
+String testAssetPath(String target) =>
+    p.join(p.dirname(libraryPath('test_pub')), 'asset', target);
+
 /// Returns a Map in the format used by the pub.dartlang.org API to represent a
 /// package version.
 ///
-- 
GitLab