Handle updated .packages in incremental compiler

Change-Id: I6de6cb643b14b14ce54944f57b9e15f502750206
Reviewed-on: https://dart-review.googlesource.com/49901
Reviewed-by: Peter von der Ahé <ahe@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>

Author: jensjoha

pkg/front_end/lib/src/base/processed_options.dart

@@ -392,7 +392,11 @@ class ProcessedOptions {
   ///
   /// This is an asynchronous method since file system operations may be
   /// required to locate/read the packages file as well as SDK metadata.
-  Future<UriTranslatorImpl> getUriTranslator() async {
+  Future<UriTranslatorImpl> getUriTranslator({bool bypassCache: false}) async {
+    if (bypassCache) {
+      _uriTranslator = null;
+      _packages = null;
+    }
     if (_uriTranslator == null) {
       ticker.logMs("Started building UriTranslator");
       var libraries = await _computeLibrarySpecification();

pkg/front_end/lib/src/fasta/incremental_compiler.dart

@@ -63,22 +63,27 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
     entryPoint ??= context.options.inputs.single;
     return context.runInContext<Future<Component>>((CompilerContext c) async {
       IncrementalCompilerData data = new IncrementalCompilerData();
-      if (dillLoadedData == null) {
-        UriTranslator uriTranslator = await c.options.getUriTranslator();
-        ticker.logMs("Read packages file");
 
+      // TODO(jensj): We should only bypass the cache if .packages has been
+      // invalidated, but Flutter does not currently invalidate .packages.
+      UriTranslator uriTranslator =
+          await c.options.getUriTranslator(bypassCache: true);
+      ticker.logMs("Read packages file");
+
+      if (dillLoadedData == null) {
         List<int> summaryBytes = await c.options.loadSdkSummaryBytes();
         int bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
         if (initializeFromDillUri != null) {
           try {
-            bytesLength += await initializeFromDill(summaryBytes, c, data);
+            bytesLength +=
+                await initializeFromDill(summaryBytes, uriTranslator, c, data);
           } catch (e) {
             // We might have loaded x out of y libraries into the component.
             // To avoid any unforeseen problems start over.
             bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
           }
         }
-        appendLibraries(data, bytesLength, uriTranslator);
+        appendLibraries(data, bytesLength);
 
         try {
           await dillLoadedData.buildOutlines();
@@ -89,7 +94,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           initializedFromDill = false;
           data.reset();
           bytesLength = prepareSummary(summaryBytes, uriTranslator, c, data);
-          appendLibraries(data, bytesLength, uriTranslator);
+          appendLibraries(data, bytesLength);
           await dillLoadedData.buildOutlines();
         }
         summaryBytes = null;
@@ -112,7 +117,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       }
 
       List<LibraryBuilder> reusedLibraries =
-          computeReusedLibraries(invalidatedUris);
+          computeReusedLibraries(invalidatedUris, uriTranslator);
       Set<Uri> reusedLibraryUris =
           new Set<Uri>.from(reusedLibraries.map((b) => b.uri));
       for (Uri uri in new Set<Uri>.from(dillLoadedData.loader.builders.keys)
@@ -128,7 +133,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
       KernelIncrementalTarget userCodeOld = userCode;
       userCode = new KernelIncrementalTarget(
-          c.fileSystem, false, dillLoadedData, dillLoadedData.uriTranslator,
+          c.fileSystem, false, dillLoadedData, uriTranslator,
           uriToSource: c.uriToSource);
 
       for (LibraryBuilder library in reusedLibraries) {
@@ -258,7 +263,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   }
 
   // This procedure will try to load the dill file and will crash if it cannot.
-  Future<int> initializeFromDill(List<int> summaryBytes, CompilerContext c,
+  Future<int> initializeFromDill(
+      List<int> summaryBytes,
+      UriTranslator uriTranslator,
+      CompilerContext c,
       IncrementalCompilerData data) async {
     int bytesLength = 0;
     FileSystemEntity entity =
@@ -277,6 +285,20 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         new BinaryBuilder(initializationBytes, disableLazyReading: true)
             .readComponent(data.component);
 
+        // Check the any package-urls still point to the same file
+        // (e.g. the package still exists and hasn't been updated).
+        for (Library lib in data.component.libraries) {
+          if (lib.importUri.scheme == "package" &&
+              uriTranslator.translate(lib.importUri, false) != lib.fileUri) {
+            // Package has been removed or updated.
+            // This library should be thrown away.
+            // Everything that depends on it should be thrown away.
+            // TODO(jensj): Anything that doesn't depend on it can be kept.
+            // For now just don't initialize from this dill.
+            throw "Changed package";
+          }
+        }
+
         initializedFromDill = true;
         bytesLength += initializationBytes.length;
         for (Library lib in data.component.libraries) {
@@ -294,27 +316,16 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
     return bytesLength;
   }
 
-  void appendLibraries(IncrementalCompilerData data, int bytesLength,
-      UriTranslator uriTranslator) {
+  void appendLibraries(IncrementalCompilerData data, int bytesLength) {
     if (data.component != null) {
-      List<Library> keepLibraries = <Library>[];
-      for (Library lib in data.component.libraries) {
-        if (lib.importUri.scheme != "package" ||
-            uriTranslator.translate(lib.importUri, false) != null) {
-          keepLibraries.add(lib);
-        }
-      }
-      data.component.libraries
-        ..clear()
-        ..addAll(keepLibraries);
-
       dillLoadedData.loader
           .appendLibraries(data.component, byteCount: bytesLength);
     }
     ticker.logMs("Appended libraries");
   }
 
-  List<LibraryBuilder> computeReusedLibraries(Iterable<Uri> invalidatedUris) {
+  List<LibraryBuilder> computeReusedLibraries(
+      Iterable<Uri> invalidatedUris, UriTranslator uriTranslator) {
     if (userCode == null && userBuilders == null) {
       return <LibraryBuilder>[];
     }
@@ -338,7 +349,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
               invalidatedFileUris.contains(library.fileUri)) ||
           (library is DillLibraryBuilder &&
               uri != library.library.fileUri &&
-              invalidatedFileUris.contains(library.library.fileUri))) {
+              invalidatedFileUris.contains(library.library.fileUri)) ||
+          (library.uri.scheme == "package" &&
+              uriTranslator.translate(library.uri, false) !=
+                  library.target.fileUri)) {
         invalidatedImportUris.add(uri);
       }
       if (!recursive) return;

pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart

@@ -68,6 +68,7 @@ void testDart2jsCompile() async {
     checkIsEqual(normalDillData, initializedDillData);
 
     // Also try without invalidating anything.
+    stopwatch.reset();
     initializeResult = await initializedCompile(
         dart2jsUrl, fullDillFromInitialized, initializeWith, []);
     Expect.equals(initializeExpect, initializeResult);

pkg/front_end/test/incremental_load_from_dill_test.dart

@@ -174,15 +174,33 @@ void newWorldTest(bool strong, List worlds) async {
       await new File.fromUri(platformUri).readAsBytes();
 
   List<int> newestWholeComponent;
+  MemoryFileSystem fs;
+  Map<String, String> sourceFiles;
+  CompilerOptions options;
+  TestIncrementalCompiler compiler;
   for (var world in worlds) {
-    MemoryFileSystem fs = new MemoryFileSystem(base);
+    bool brandNewWorld = true;
+    if (world["worldType"] == "updated") {
+      brandNewWorld = false;
+    }
+
+    if (brandNewWorld) {
+      fs = new MemoryFileSystem(base);
+    }
     fs.entityForUri(sdkSummary).writeAsBytesSync(sdkSummaryData);
     bool expectInitializeFromDill = false;
     if (newestWholeComponent != null && newestWholeComponent.isNotEmpty) {
       fs.entityForUri(initializeFrom).writeAsBytesSync(newestWholeComponent);
       expectInitializeFromDill = true;
     }
-    Map<String, String> sourceFiles = world["sources"];
+    if (world["expectInitializeFromDill"] != null) {
+      expectInitializeFromDill = world["expectInitializeFromDill"];
+    }
+    if (brandNewWorld) {
+      sourceFiles = new Map<String, String>.from(world["sources"]);
+    } else {
+      sourceFiles.addAll(world["sources"]);
+    }
     Uri packagesUri;
     for (String filename in sourceFiles.keys) {
       String data = sourceFiles[filename] ?? "";
@@ -194,12 +212,14 @@ void newWorldTest(bool strong, List worlds) async {
       fs.entityForUri(uri).writeAsStringSync(data);
     }
 
-    CompilerOptions options = getOptions(strong);
-    options.fileSystem = fs;
-    options.sdkRoot = null;
-    options.sdkSummary = sdkSummary;
-    if (packagesUri != null) {
-      options.packagesFileUri = packagesUri;
+    if (brandNewWorld) {
+      options = getOptions(strong);
+      options.fileSystem = fs;
+      options.sdkRoot = null;
+      options.sdkSummary = sdkSummary;
+      if (packagesUri != null) {
+        options.packagesFileUri = packagesUri;
+      }
     }
     bool gotError = false;
     List<String> formattedErrors = <String>[];
@@ -218,8 +238,9 @@ void newWorldTest(bool strong, List worlds) async {
     };
 
     Uri entry = base.resolve(world["entry"]);
-    TestIncrementalCompiler compiler =
-        new TestIncrementalCompiler(options, entry, initializeFrom);
+    if (brandNewWorld) {
+      compiler = new TestIncrementalCompiler(options, entry, initializeFrom);
+    }
 
     if (world["invalidate"] != null) {
       for (var filename in world["invalidate"]) {
@@ -228,7 +249,7 @@ void newWorldTest(bool strong, List worlds) async {
     }
 
     Stopwatch stopwatch = new Stopwatch()..start();
-    Component component = await compiler.computeDelta();
+    Component component = await compiler.computeDelta(fullComponent: true);
     if (world["errors"] == true && !gotError) {
       throw "Expected error, but didn't get any.";
     } else if (world["errors"] != true && gotError) {
@@ -254,19 +275,27 @@ void newWorldTest(bool strong, List worlds) async {
       throw "Expected that initializedFromDill would be "
           "$expectInitializeFromDill but was ${compiler.initializedFromDill}";
     }
-    if (world["invalidate"] != null) {
-      Expect.equals(world["invalidate"].length,
-          compiler.invalidatedImportUrisForTesting.length);
-      List expectedInvalidatedUri = world["expectedInvalidatedUri"];
-      if (expectedInvalidatedUri != null) {
-        Expect.setEquals(
-            expectedInvalidatedUri
-                .map((s) => Uri.parse(substituteVariables(s, base))),
-            compiler.invalidatedImportUrisForTesting);
+    if (world["checkInvalidatedFiles"] != false) {
+      if (world["invalidate"] != null) {
+        Expect.equals(world["invalidate"].length,
+            compiler.invalidatedImportUrisForTesting.length);
+        List expectedInvalidatedUri = world["expectedInvalidatedUri"];
+        if (expectedInvalidatedUri != null) {
+          Expect.setEquals(
+              expectedInvalidatedUri
+                  .map((s) => Uri.parse(substituteVariables(s, base))),
+              compiler.invalidatedImportUrisForTesting);
+        }
+      } else {
+        Expect.isNull(compiler.invalidatedImportUrisForTesting);
+        Expect.isNull(world["expectedInvalidatedUri"]);
       }
-    } else {
-      Expect.isNull(compiler.invalidatedImportUrisForTesting);
-      Expect.isNull(world["expectedInvalidatedUri"]);
+    }
+
+    {
+      Component component2 = await compiler.computeDelta(fullComponent: true);
+      List<int> thisWholeComponent = serializeComponent(component2);
+      checkIsEqual(newestWholeComponent, thisWholeComponent);
     }
   }
 }

pkg/front_end/testcases/incremental_initialize_from_dill/disappearing_package.yaml

@@ -25,8 +25,11 @@ worlds:
       .packages: dummy:${outDirUri}
     expectedLibraryCount: 2
   - entry: main.dart
-    invalidate:
-      - main.dart
+    # TODO(jensj): For now we don't initialize from dill when a package was
+    # changed.
+    #invalidate:
+    #  - main.dart
+    expectInitializeFromDill: false
     sources:
       main.dart: |
         main() {

pkg/front_end/testcases/incremental_initialize_from_dill/updated_package.yaml

@@ -0,0 +1,65 @@
+# Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE.md file.
+
+# Compile an application with a package and use it.
+# Then update the package from .packages.
+# The old package is still included in the dill file we initialize from,
+# but shouldn't cause trouble, nor be included in the output.
+
+type: newworld
+strong: false
+worlds:
+  - entry: main.dart
+    sources:
+      main.dart: |
+        import "package:dummy/b.dart";
+        main() {
+          print("hello");
+          b();
+        }
+      package_0.1.0/a.dart: |
+        a() {
+          la1();
+        }
+        la1() {
+          print("v0.1.0");
+        }
+      package_0.1.0/b.dart: |
+        import "a.dart";
+        b() {
+          a();
+        }
+      .packages: dummy:${outDirUri}package_0.1.0
+    expectedLibraryCount: 3
+  - entry: main.dart
+    # TODO(jensj): For now we don't initialize from dill when a package was
+    # changed.
+    #invalidate:
+    #  - main.dart
+    expectInitializeFromDill: false
+    sources:
+      main.dart: |
+        import "package:dummy/b.dart";
+        main() {
+          print("hello");
+          b();
+        }
+      package_0.1.0/a.dart: |
+        a() {
+          la1();
+        }
+        la1() {
+          print("v0.1.0");
+        }
+      package_0.1.0/b.dart: |
+        import "a.dart";
+        b() {
+          a();
+        }
+      package_0.1.1/b.dart: |
+        b() {
+          print("hello from v0.1.1");
+        }
+      .packages: dummy:${outDirUri}package_0.1.1
+    expectedLibraryCount: 2