[NaCl SDK] Updating a bundle shouldn't merge the new and old archives.

And reinstall shouldn't try to download the entire list of archives, either.

BUG=241174
R=sbc@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200882 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 75 insertions, 12 deletions

diff --git a/native_client_sdk/src/build_tools/manifest_util.py b/native_client_sdk/src/build_tools/manifest_util.py
index 51cf24c..4156ef4 100644
--- a/native_client_sdk/src/build_tools/manifest_util.py
+++ b/native_client_sdk/src/build_tools/manifest_util.py
@@ -338,6 +338,10 @@ class Bundle(dict):
     """Add an archive to this bundle."""
     self[ARCHIVES_KEY].append(archive)
 
+  def RemoveAllArchives(self):
+    """Remove all archives from this Bundle."""
+    del self[ARCHIVES_KEY][:]
+
   def RemoveAllArchivesForHostOS(self, host_os_name):
     """Remove an archive from this Bundle."""
     if host_os_name == 'all':
diff --git a/native_client_sdk/src/build_tools/sdk_tools/command/update.py b/native_client_sdk/src/build_tools/sdk_tools/command/update.py
index f485804..29f84c8 100644
--- a/native_client_sdk/src/build_tools/sdk_tools/command/update.py
+++ b/native_client_sdk/src/build_tools/sdk_tools/command/update.py
@@ -253,6 +253,25 @@ def Reinstall(delegate, local_manifest, bundle_names):
 
   for bundle_name in valid_bundles:
     bundle = copy.deepcopy(local_manifest.GetBundle(bundle_name))
+
+    # HACK(binji): There was a bug where we'd merge the bundles from the old
+    # archive and the new archive when updating. As a result, some users may
+    # have a cache manifest that contains duplicate archives. Remove all
+    # archives with the same basename except for the most recent.
+    # Because the archives are added to a list, we know the most recent is at
+    # the end.
+    archives = {}
+    for archive in bundle.GetArchives():
+      url = archive.url
+      path = urlparse.urlparse(url)[2]
+      basename = os.path.basename(path)
+      archives[basename] = archive
+
+    # Update the bundle with these new archives.
+    bundle.RemoveAllArchives()
+    for _, archive in archives.iteritems():
+      bundle.AddArchive(archive)
+
     _UpdateBundle(delegate, bundle, local_manifest)
 
 
@@ -351,7 +370,8 @@ def _UpdateBundle(delegate, bundle, local_manifest):
                            rename_to_dir)
 
   logging.info('Updating local manifest to include bundle %s' % (bundle.name))
-  local_manifest.MergeBundle(bundle)
+  local_manifest.RemoveBundle(bundle.name)
+  local_manifest.SetBundle(bundle)
   delegate.CleanupCache()
 
 
diff --git a/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py b/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
index d5e9841..2fae323 100755
--- a/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
+++ b/native_client_sdk/src/build_tools/tests/sdktools_commands_test.py
@@ -217,6 +217,10 @@ class TestCommands(SdkToolsTestCase):
     output = self._Run(['update', 'pepper_23', '--force'])
     self.assertTrue('Updating bundle' in output)
 
+    cache_manifest = self._ReadCacheManifest()
+    num_archives = len(cache_manifest.GetBundle('pepper_23').GetArchives())
+    self.assertEqual(num_archives, 1)
+
   def testUpdateUnknownBundles(self):
     """The update command should ignore unknown bundles and notify the user."""
     self._WriteManifest()
@@ -317,6 +321,42 @@ class TestCommands(SdkToolsTestCase):
     with open(dummy_txt) as f:
       self.assertEqual(f.read(), 'Dummy stuff for pepper_23')
 
+    cache_manifest = self._ReadCacheManifest()
+    num_archives = len(cache_manifest.GetBundle('pepper_23').GetArchives())
+    self.assertEqual(num_archives, 1)
+
+  def testReinstallWithDuplicatedArchives(self):
+    """The reinstall command should only use the most recent archive if there
+    are duplicated archives.
+
+    NOTE: There was a bug where the sdk_cache/naclsdk_manifest2.json file was
+    duplicating archives from different revisions. Make sure that reinstall
+    ignores old archives in the bundle.
+    """
+    # First install the bundle.
+    self._AddDummyBundle(self.manifest, 'pepper_23')
+    self._WriteManifest()
+    self._Run(['update', 'pepper_23'])
+
+    manifest = self._ReadCacheManifest()
+    bundle = manifest.GetBundle('pepper_23')
+    self.assertEqual(len(bundle.GetArchives()), 1)
+
+    # Now add a bogus duplicate archive
+    archive2 = self._MakeDummyArchive('pepper_23', tarname='pepper_23',
+                                      filename='dummy2.txt')
+    bundle.AddArchive(archive2)
+    self._WriteCacheManifest(manifest)
+
+    output = self._Run(['reinstall', 'pepper_23'])
+    # When updating just one file, there is no (file 1/2 - "...") output.
+    self.assertFalse('file 1/' in output)
+    # Should be using the last archive.
+    self.assertFalse(os.path.exists(
+        os.path.join(self.basedir, 'nacl_sdk', 'pepper_23', 'dummy.txt')))
+    self.assertTrue(os.path.exists(
+        os.path.join(self.basedir, 'nacl_sdk', 'pepper_23', 'dummy2.txt')))
+
   def testReinstallDoesntUpdate(self):
     """The reinstall command should not update a bundle that has an update."""
     # First install the bundle.
diff --git a/native_client_sdk/src/build_tools/tests/sdktools_test.py b/native_client_sdk/src/build_tools/tests/sdktools_test.py
index 6f5a6ee..cc5e113 100755
--- a/native_client_sdk/src/build_tools/tests/sdktools_test.py
+++ b/native_client_sdk/src/build_tools/tests/sdktools_test.py
@@ -47,7 +47,8 @@ class SdkToolsTestCase(unittest.TestCase):
     # is greater than the version we are attempting to update to.
     self.current_revision = self._GetSdkToolsBundleRevision()
     self._BuildUpdater(self.basedir, self.current_revision)
-    self._LoadCacheManifest()
+    self.manifest = self._ReadCacheManifest()
+    self.sdk_tools_bundle = self.manifest.GetBundle('sdk_tools')
     self.server = test_server.LocalHTTPServer(self.basedir)
 
   def _GetSdkToolsBundleRevision(self):
@@ -61,16 +62,6 @@ class SdkToolsTestCase(unittest.TestCase):
     manifest.LoadDataFromString(open(manifest_filename, 'r').read())
     return manifest.GetBundle('sdk_tools').revision
 
-  def _LoadCacheManifest(self):
-    """Read the manifest from nacl_sdk/sdk_cache.
-
-    This manifest should only contain the sdk_tools bundle.
-    """
-    manifest_filename = os.path.join(self.cache_dir, MANIFEST_BASENAME)
-    self.manifest = manifest_util.SDKManifest()
-    self.manifest.LoadDataFromString(open(manifest_filename).read())
-    self.sdk_tools_bundle = self.manifest.GetBundle('sdk_tools')
-
   def _WriteConfig(self, config_data):
     config_filename = os.path.join(self.cache_dir, 'naclsdk_config.json')
     with open(config_filename, 'w') as stream:
@@ -85,6 +76,14 @@ class SdkToolsTestCase(unittest.TestCase):
     with open(manifest_filename, 'w') as stream:
       stream.write(manifest.GetDataAsString())
 
+  def _ReadCacheManifest(self):
+    """Read the manifest at nacl_sdk/sdk_cache."""
+    manifest_filename = os.path.join(self.cache_dir, MANIFEST_BASENAME)
+    manifest = manifest_util.SDKManifest()
+    with open(manifest_filename) as stream:
+      manifest.LoadDataFromString(stream.read())
+    return manifest
+
   def _WriteManifest(self):
     with open(os.path.join(self.basedir, MANIFEST_BASENAME), 'w') as stream:
       stream.write(self.manifest.GetDataAsString())