Verify that cache unlinked and linked bundles are majorVersion compatible.

pkg/analyzer/lib/src/summary/pub_summary.dart

Index: pkg/analyzer/lib/src/summary/pub_summary.dart
diff --git a/pkg/analyzer/lib/src/summary/pub_summary.dart b/pkg/analyzer/lib/src/summary/pub_summary.dart
index ed7221a3bd6ee43d410c962e0b5a1cce3ca3383b..e77ca7e469fe6f364f0f22eab831c3e717ecb8f4 100644
--- a/pkg/analyzer/lib/src/summary/pub_summary.dart
+++ b/pkg/analyzer/lib/src/summary/pub_summary.dart
@@ -80,6 +80,12 @@ class PubSummaryManager {
   static const UNLINKED_NAME = 'unlinked.ds';
   static const UNLINKED_SPEC_NAME = 'unlinked_spec.ds';
 
+  /**
+   * See [PackageBundleAssembler.currentMajorVersion].
+   */
+  @visibleForTesting
+  int majorVersion = PackageBundleAssembler.currentMajorVersion;

[Brian Wilkerson, 2016/09/08 16:42:28]

Should this be final?

[scheglov, 2016/09/08 16:44:24]

No.
The only reason it was added is to be able to change it in tests.
In theory it could be final, and passed in the constructor.
But that seems like an overhead for the test-only feature.

[Paul Berry, 2016/09/08 16:45:16]

I was just about to make a similar comment.  I'd prefer if we made this final
and initialized it in the constructor using a named argument whose default value
is PackageBundleAssembler.currentMajorVersion.  That way we can still create a
PubSummaryManager during tests that uses a different major version, but there's
no worry about this field getting modified after the class is created and
causing weird behaviors.

[scheglov, 2016/09/08 17:02:37]

OK, it seems that we have a consensus here :-)
Done.

+
   final ResourceProvider resourceProvider;
 
   /**
@@ -342,7 +348,9 @@ class PubSummaryManager {
 
     try {
       addDartFiles(libFolder);
-      List<int> bytes = assembler.assemble().toBuffer();
+      PackageBundleBuilder bundleWriter = assembler.assemble();
+      bundleWriter.majorVersion = majorVersion;
+      List<int> bytes = bundleWriter.toBuffer();
       String fileName = _getUnlinkedName(strong);
       _writeAtomic(package.folder, fileName, bytes);
     } on FileSystemException {
@@ -382,27 +390,36 @@ class PubSummaryManager {
     if (bundle != null) {
       return bundle;
     }
+
     // Try to read from the file system.
     String fileName = _getUnlinkedName(strong);
-    File unlinkedFile = package.folder.getChildAssumingFile(fileName);
-    if (unlinkedFile.exists) {
+    File file = package.folder.getChildAssumingFile(fileName);
+    if (file.exists) {
       try {
-        List<int> bytes = unlinkedFile.readAsBytesSync();
+        List<int> bytes = file.readAsBytesSync();
         bundle = new PackageBundle.fromBuffer(bytes);
-        unlinkedBundleMap[package] = bundle;
-        // TODO(scheglov) if not in the pub cache, check for consistency
-        return bundle;
       } on FileSystemException {
         // Ignore file system exceptions.
       }
     }
+
+    bool isInPubCache = isPathInPubCache(pathContext, package.folder.path);
+
+    // Verify compatibility.
+    // TODO(scheglov) if not in the pub cache, check for consistency
+    if (bundle != null && bundle.majorVersion == majorVersion) {
+      unlinkedBundleMap[package] = bundle;
+      return bundle;
+    }
+
     // Schedule computation in the background, if in the pub cache.
-    if (isPathInPubCache(pathContext, package.folder.path)) {
+    if (isInPubCache) {
       if (seenPackages.add(package)) {
         _scheduleUnlinked(package);
       }
     }
-    // The bundle is for available.
+
+    // The bundle is not available.
     return null;
   }