[NaCl SDK] update_nacl_manifest: Allow manual updating of a version.

* New argument --bundle-version, can be passed multiple times. The updater will
use the version passed in.
* The updater won't upload a manifest if it hasn't changed.
* The updater won't rollback a bundle (i.e. if the automatically determined
  revision is older, it will keep the current bundle).
* Switch --dryrun to use difflib instead of system diff.
* Add equality testing for manifest_util.SDKManifest. It already existed for
  Bundles.

BUG=none
R=sbc@chromium.org
TBR=noelallen@chromium.org
NOTRY=true


Review URL: https://chromiumcodereview.appspot.com/11421138

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

3 files changed, 197 insertions, 33 deletions

diff --git a/native_client_sdk/src/build_tools/manifest_util.py b/native_client_sdk/src/build_tools/manifest_util.py
index 1e055d7..478d7c3 100644
--- a/native_client_sdk/src/build_tools/manifest_util.py
+++ b/native_client_sdk/src/build_tools/manifest_util.py
@@ -543,6 +543,27 @@ class SDKManifest(object):
   def __str__(self):
     return self.GetDataAsString()
 
+  def __eq__(self, other):
+    # Access to protected member _manifest_data of a client class
+    # pylint: disable=W0212
+    if (self._manifest_data['manifest_version'] !=
+        other._manifest_data['manifest_version']):
+      return False
+
+    self_bundle_names = set(b.name for b in self.GetBundles())
+    other_bundle_names = set(b.name for b in other.GetBundles())
+    if self_bundle_names != other_bundle_names:
+      return False
+
+    for bundle_name in self_bundle_names:
+      if self.GetBundle(bundle_name) != other.GetBundle(bundle_name):
+        return False
+
+    return True
+
+  def __ne__(self, other):
+    return not (self == other)
+
   def GetDataAsString(self):
     """Returns the current JSON manifest object, pretty-printed"""
     return DictToJSON(self._manifest_data)
diff --git a/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py b/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
index e69f307..53f0e7b 100755
--- a/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
@@ -136,6 +136,9 @@ class MakeHistory(object):
 
 
 class MakeFiles(dict):
+  def AddOnlineManifest(self, manifest_string):
+    self['naclsdk_manifest2.json'] = manifest_string
+
   def Add(self, bundle, add_archive_for_os=OS_MLW, add_json_for_os=OS_MLW):
     for archive in bundle.GetArchives():
       if not archive.host_os in add_archive_for_os:
@@ -162,6 +165,7 @@ class TestDelegate(update_nacl_manifest.Delegate):
     self.files = files
     self.version_mapping = version_mapping
     self.dryrun = 0
+    self.called_gsutil_cp = False
 
   def GetRepoManifest(self):
     return self.manifest
@@ -187,6 +191,7 @@ class TestDelegate(update_nacl_manifest.Delegate):
     return self.files[path]
 
   def GsUtil_cp(self, src, dest, stdin=None):
+    self.called_gsutil_cp = True
     dest_path = GetPathFromGsUrl(dest)
     if src == '-':
       self.files[dest_path] = stdin
@@ -237,8 +242,8 @@ class TestUpdateManifest(unittest.TestCase):
     self.delegate = TestDelegate(self.manifest, self.history.history,
         self.files, self.version_mapping)
 
-  def _Run(self, host_oses):
-    update_nacl_manifest.Run(self.delegate, host_oses)
+  def _Run(self, host_oses, fixed_bundle_versions=None):
+    update_nacl_manifest.Run(self.delegate, host_oses, fixed_bundle_versions)
 
   def _HasUploadedManifest(self):
     return 'naclsdk_manifest2.json' in self.files
@@ -275,12 +280,12 @@ class TestUpdateManifest(unittest.TestCase):
     self.manifest = MakeManifest(B18_0_1025_163_R1_MLW)
     self._MakeDelegate()
     self._Run(OS_MLW)
-    self.assertEqual(self._HasUploadedManifest(), False)
+    self.assertFalse(self._HasUploadedManifest())
 
     # Add another bundle, make sure it still doesn't update
     self.manifest.AddBundle(B19_0_1084_41_R1_MLW)
     self._Run(OS_MLW)
-    self.assertEqual(self._HasUploadedManifest(), False)
+    self.assertFalse(self._HasUploadedManifest())
 
   def testSimpleUpdate(self):
     self.manifest = MakeManifest(B18_R1_NONE)
@@ -385,7 +390,7 @@ class TestUpdateManifest(unittest.TestCase):
         B18_0_1025_163_R1_MLW)
     self._MakeDelegate()
     self._Run(OS_MLW)
-    self.assertEqual(self._HasUploadedManifest(), False)
+    self.assertFalse(self._HasUploadedManifest())
 
   def testUpdateWithHistoryWithExtraneousPlatforms(self):
     self.manifest = MakeManifest(B18_R1_NONE)
@@ -530,6 +535,54 @@ mac,canary,21.0.1156.0,2012-05-30 12:14:21.305090"""
     p19_bundle = self.uploaded_manifest.GetBundle(B19_R1_NONE.name)
     self.assertEqual(p19_bundle.stability, STABLE)
 
+  def testDontPushIfNoChange(self):
+    # Make an online manifest that already has this bundle.
+    online_manifest = MakeManifest(B18_0_1025_163_R1_MLW)
+    self.files.AddOnlineManifest(online_manifest.GetDataAsString())
+
+    self.manifest = MakeManifest(B18_R1_NONE)
+    self.history.Add(OS_MLW, DEV, V18_0_1025_163)
+    self.files.Add(B18_0_1025_163_R1_MLW)
+
+    self._MakeDelegate()
+    self._Run(OS_MLW)
+    self.assertFalse(self.delegate.called_gsutil_cp)
+
+  def testDontPushIfRollback(self):
+    # Make an online manifest that has a newer bundle
+    online_manifest = MakeManifest(B18_0_1025_184_R1_MLW)
+    self.files.AddOnlineManifest(online_manifest.GetDataAsString())
+
+    self.manifest = MakeManifest(B18_R1_NONE)
+    self.history.Add(OS_MLW, DEV, V18_0_1025_163)
+    self.files.Add(B18_0_1025_163_R1_MLW)
+
+    self._MakeDelegate()
+    self._Run(OS_MLW)
+    self.assertFalse(self.delegate.called_gsutil_cp)
+
+  def testRunWithFixedBundleVersions(self):
+    self.manifest = MakeManifest(B18_R1_NONE)
+    self.history.Add(OS_MLW, BETA, V18_0_1025_163)
+    self.files.Add(B18_0_1025_163_R1_MLW)
+    self.files.Add(B18_0_1025_184_R1_MLW)
+
+    self._MakeDelegate()
+    self._Run(OS_MLW, [('pepper_18', '18.0.1025.184')])
+    self._ReadUploadedManifest()
+    self._AssertUploadedManifestHasBundle(B18_0_1025_184_R1_MLW, BETA)
+    self.assertEqual(len(self.uploaded_manifest.GetBundles()), 1)
+
+  def testRunWithMissingFixedBundleVersions(self):
+    self.manifest = MakeManifest(B18_R1_NONE)
+    self.history.Add(OS_MLW, BETA, V18_0_1025_163)
+    self.files.Add(B18_0_1025_163_R1_MLW)
+
+    self._MakeDelegate()
+    self._Run(OS_MLW, [('pepper_18', '18.0.1025.184')])
+    # Nothing should be uploaded if the user gives a missing fixed version.
+    self.assertFalse(self.delegate.called_gsutil_cp)
+
 
 class TestUpdateVitals(unittest.TestCase):
   def setUp(self):
diff --git a/native_client_sdk/src/build_tools/update_nacl_manifest.py b/native_client_sdk/src/build_tools/update_nacl_manifest.py
index 154f827..52349c4 100755
--- a/native_client_sdk/src/build_tools/update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/update_nacl_manifest.py
@@ -13,6 +13,7 @@ in manifest.
 import buildbot_common
 import csv
 import cStringIO
+import difflib
 import email
 import json
 import manifest_util
@@ -308,6 +309,38 @@ class VersionFinder(object):
     return self._DoGetMostRecentSharedVersion(platforms,
         shared_version_generator, allow_trunk_revisions=True)
 
+  def GetAvailablePlatformArchivesFor(self, version, platforms,
+                                      allow_trunk_revisions):
+    """Returns a sequence of archives that exist for a given version, on the
+    given platforms.
+
+    The second element of the returned tuple is a list of all platforms that do
+    not have an archive for the given version.
+
+    Args:
+      version: The version to find archives for. (e.g. "18.0.1025.164")
+      platforms: A sequence of platforms to consider, e.g.
+          ('mac', 'linux', 'win')
+      allow_trunk_revisions: If True, will search for archives using the
+          trunk revision that matches the branch version.
+    Returns:
+      A tuple (archives, missing_platforms). |archives| is a list of archive
+      URLs, |missing_platforms| is a list of platform names.
+    """
+    archives = self._GetAvailableArchivesFor(version)
+    missing_platforms = set(platforms) - set(GetPlatformsFromArchives(archives))
+    if allow_trunk_revisions and missing_platforms:
+      # Try to find trunk archives for platforms that are missing archives.
+      trunk_version = self.delegate.GetTrunkRevision(version)
+      trunk_archives = self._GetAvailableArchivesFor(trunk_version)
+      for trunk_archive in trunk_archives:
+        trunk_archive_platform = GetPlatformFromArchiveUrl(trunk_archive)
+        if trunk_archive_platform in missing_platforms:
+          archives.append(trunk_archive)
+          missing_platforms.discard(trunk_archive_platform)
+
+    return archives, missing_platforms
+
   def _DoGetMostRecentSharedVersion(self, platforms, shared_version_generator,
       allow_trunk_revisions):
     """Returns the most recent version of a pepper bundle that exists on all
@@ -321,7 +354,7 @@ class VersionFinder(object):
       shared_version_generator: A generator that will yield (version, channel)
           tuples in order of most recent to least recent.
       allow_trunk_revisions: If True, will search for archives using the
-            trunk revision that matches the branch version.
+          trunk revision that matches the branch version.
     Returns:
       A tuple (version, channel, archives). The version is a string such as
       "19.0.1084.41". The channel is one of ('stable', 'beta', 'dev',
@@ -345,18 +378,8 @@ class VersionFinder(object):
           msg += '  %s (%s) %s\n' % (version, channel, archive_msg)
         raise Exception(msg)
 
-      archives = self._GetAvailableNaClSDKArchivesFor(version)
-      missing_platforms = set(platforms) - \
-          set(GetPlatformsFromArchives(archives))
-      if allow_trunk_revisions and missing_platforms:
-        # Try to find trunk archives for platforms that are missing archives.
-        trunk_version = self.delegate.GetTrunkRevision(version)
-        trunk_archives = self._GetAvailableNaClSDKArchivesFor(trunk_version)
-        for trunk_archive in trunk_archives:
-          trunk_archive_platform = GetPlatformFromArchiveUrl(trunk_archive)
-          if trunk_archive_platform in missing_platforms:
-            archives.append(trunk_archive)
-            missing_platforms.discard(trunk_archive_platform)
+      archives, missing_platforms = self.GetAvailablePlatformArchivesFor(
+          version, platforms, allow_trunk_revisions)
 
       if not missing_platforms:
         return version, channel, archives
@@ -438,7 +461,7 @@ class VersionFinder(object):
         # force increment to next version for all platforms
         shared_version = None
 
-  def _GetAvailableNaClSDKArchivesFor(self, version_string):
+  def _GetAvailableArchivesFor(self, version_string):
     """Downloads a list of all available archives for a given version.
 
     Args:
@@ -464,6 +487,8 @@ class Updater(object):
   def __init__(self, delegate):
     self.delegate = delegate
     self.versions_to_update = []
+    self.online_manifest = manifest_util.SDKManifest()
+    self._FetchOnlineManifest()
 
   def AddVersionToUpdate(self, bundle_name, version, channel, archives):
     """Add a pepper version to update in the uploaded manifest.
@@ -478,6 +503,10 @@ class Updater(object):
   def Update(self, manifest):
     """Update a manifest and upload it.
 
+    Note that bundles will not be updated if the current version is newer.
+    That is, the updater will never automatically update to an older version of
+    a bundle.
+
     Args:
       manifest: The manifest used as a template for updating. Only pepper
       bundles that contain no archives will be considered for auto-updating."""
@@ -499,6 +528,16 @@ class Updater(object):
         platform_bundle.name = bundle_name
         bundle.MergeWithBundle(platform_bundle)
 
+      # Check to ensure this bundle is newer than the online bundle.
+      online_bundle = self.online_manifest.GetBundle(bundle_name)
+      if online_bundle and online_bundle.revision >= bundle.revision:
+        self.delegate.Print(
+            '  Revision %s is not newer than than online revision %s. '
+            'Skipping.' % (bundle.revision, online_bundle.revision))
+
+        manifest.MergeBundle(online_bundle)
+        continue
+
       major_version = SplitVersion(version)[0]
       if major_version < max_stable_version and channel == 'stable':
         bundle.stability = 'post_stable'
@@ -541,18 +580,21 @@ class Updater(object):
     Args:
       manifest: The new manifest to upload.
     """
+    new_manifest_string = manifest.GetDataAsString()
+    online_manifest_string = self.online_manifest.GetDataAsString()
+
     if self.delegate.dryrun:
-      name = MANIFEST_BASENAME + ".new"
-      self.delegate.Print("Writing new manifest: %s" % name)
-      with open(name, 'w') as f:
-        f.write(manifest.GetDataAsString())
-      stdout = self.delegate.GsUtil_cat(GS_SDK_MANIFEST)
-
-      online = MANIFEST_BASENAME + ".online"
-      self.delegate.Print("Writing online manifest: %s" % online)
-      with open(online, 'w') as f:
-        f.write(stdout)
-      os.system('diff -u %s %s' % (online, name))
+      self.delegate.Print(''.join(list(difflib.unified_diff(
+          online_manifest_string.splitlines(1),
+          new_manifest_string.splitlines(1)))))
+    else:
+      online_manifest = manifest_util.SDKManifest()
+      online_manifest.LoadDataFromString(online_manifest_string)
+
+      if online_manifest == manifest:
+        self.delegate.Print('New manifest doesn\'t differ from online manifest.'
+            'Skipping upload.')
+        return
 
     timestamp_manifest_path = GS_MANIFEST_BACKUP_DIR + \
         GetTimestampManifestName()
@@ -562,9 +604,32 @@ class Updater(object):
     # copy from timestampped copy over the official manifest.
     self.delegate.GsUtil_cp(timestamp_manifest_path, GS_SDK_MANIFEST)
 
+  def _FetchOnlineManifest(self):
+    try:
+      online_manifest_string = self.delegate.GsUtil_cat(GS_SDK_MANIFEST)
+    except subprocess.CalledProcessError:
+      # It is not a failure if the online manifest doesn't exist.
+      online_manifest_string = ''
+
+    if online_manifest_string:
+      self.online_manifest.LoadDataFromString(online_manifest_string)
+
+
+def Run(delegate, platforms, fixed_bundle_versions=None):
+  """Entry point for the auto-updater.
+
+  Args:
+    delegate: The Delegate object to use for reading Urls, files, etc.
+    platforms: A sequence of platforms to consider, e.g.
+        ('mac', 'linux', 'win')
+    fixed_bundle_versions: A sequence of tuples (bundle_name, version_string).
+        e.g. ('pepper_21', '21.0.1145.0')
+  """
+  if fixed_bundle_versions:
+    fixed_bundle_versions = dict(fixed_bundle_versions)
+  else:
+    fixed_bundle_versions = {}
 
-def Run(delegate, platforms):
-  """Entry point for the auto-updater."""
   manifest = delegate.GetRepoManifest()
   auto_update_bundles = []
   for bundle in manifest.GetBundles():
@@ -589,6 +654,21 @@ def Run(delegate, platforms):
       version, channel, archives = version_finder.GetMostRecentSharedVersion(
           bundle.version, platforms)
 
+    if bundle.name in fixed_bundle_versions:
+      # Ensure this version is valid for all platforms.
+      # If it is, use the channel found above (because the channel for this
+      # version may not be in the history.)
+      version = fixed_bundle_versions[bundle.name]
+      delegate.Print('Fixed bundle version: %s, %s' % (bundle.name, version))
+      allow_trunk_revisions = bundle.name == CANARY_BUNDLE_NAME
+      archives, missing = version_finder.GetAvailablePlatformArchivesFor(
+          version, platforms, allow_trunk_revisions)
+      if missing:
+        delegate.Print(
+            'An archive for version %s of bundle %s doesn\'t exist '
+            'for platform(s): %s' % (version, bundle.name, ', '.join(missing)))
+        return
+
     updater.AddVersionToUpdate(bundle.name, version, channel, archives)
 
   updater.Update(manifest)
@@ -642,6 +722,9 @@ def main(args):
       action='store_true')
   parser.add_option('-v', '--verbose', help='print more diagnotic messages.',
       action='store_true')
+  parser.add_option('--bundle-version',
+      help='Manually set a bundle version. This can be passed more than once. '
+      'format: --bundle-version pepper_24=24.0.1312.25', action='append')
   options, args = parser.parse_args(args[1:])
 
   if (options.mailfrom is None) != (not options.mailto):
@@ -650,6 +733,13 @@ def main(args):
     sys.stderr.write('warning: Disabling email, one of --mailto or --mailfrom '
         'was missing.\n')
 
+  # Parse bundle versions.
+  fixed_bundle_versions = {}
+  if options.bundle_version:
+    for bundle_version_string in options.bundle_version:
+      bundle_name, version = bundle_version_string.split('=')
+      fixed_bundle_versions[bundle_name] = version
+
   if options.mailfrom and options.mailto:
     # Capture stderr so it can be emailed, if necessary.
     sys.stderr = CapturedFile(sys.stderr)
@@ -657,7 +747,7 @@ def main(args):
   try:
     try:
       delegate = RealDelegate(options.dryrun, options.gsutil, options.verbose)
-      Run(delegate, ('mac', 'win', 'linux'))
+      Run(delegate, ('mac', 'win', 'linux'), fixed_bundle_versions)
     except Exception:
       if options.mailfrom and options.mailto:
         traceback.print_exc()