[NaCl SDK] update_nacl_manifest: Ignore the last digit of a canary version.

The final number in a canary build does not include any different
changes, it is just a different experiment (e.g. using ASAN, or using
aura). We should not compare these versions differently.

Note that the version mapping will show that 31.0.1608.0 is different
from 31.0.1608.1 -- this is because 31.0.1608.1 is built from the branch,
not from trunk. Inspecting the branch would show that there are no
changes (why would there be? No one has any reason to merge changes to a
canary branch.)

BUG=none
R=sbc@chromium.org

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

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

2 files changed, 65 insertions, 5 deletions

diff --git a/native_client_sdk/src/build_tools/tests/update_nacl_manifest_test.py b/native_client_sdk/src/build_tools/tests/update_nacl_manifest_test.py
index 7adb8f7..de292d1 100755
--- a/native_client_sdk/src/build_tools/tests/update_nacl_manifest_test.py
+++ b/native_client_sdk/src/build_tools/tests/update_nacl_manifest_test.py
@@ -31,6 +31,7 @@ OS_CR = ('cros',)
 OS_M = ('mac',)
 OS_ML = ('mac', 'linux')
 OS_MW = ('mac', 'win')
+OS_LW = ('linux', 'win')
 OS_MLW = ('mac', 'linux', 'win')
 OS_ALL = ('all',)
 POST_STABLE = 'post_stable'
@@ -238,6 +239,7 @@ V19_0_1084_67 = '19.0.1084.67'
 V21_0_1145_0 = '21.0.1145.0'
 V21_0_1166_0 = '21.0.1166.0'
 V26_0_1386_0 = '26.0.1386.0'
+V26_0_1386_1 = '26.0.1386.1'
 VTRUNK_140819 = 'trunk.140819'
 B18_0_1025_163_MLW = MakePlatformBundle(18, 132135, V18_0_1025_163, OS_MLW)
 B18_0_1025_184_MLW = MakePlatformBundle(18, 134900, V18_0_1025_184, OS_MLW)
@@ -250,6 +252,7 @@ B21_0_1145_0_MLW = MakePlatformBundle(21, 138079, V21_0_1145_0, OS_MLW)
 B21_0_1166_0_MW = MakePlatformBundle(21, 140819, V21_0_1166_0, OS_MW)
 B26_NONE = MakePlatformBundle(26)
 B26_0_1386_0_MLW = MakePlatformBundle(26, 177362, V26_0_1386_0, OS_MLW)
+B26_0_1386_1_MLW = MakePlatformBundle(26, 177439, V26_0_1386_1, OS_MLW)
 BTRUNK_140819_MLW = MakePlatformBundle(21, 140819, VTRUNK_140819, OS_MLW)
 NON_PEPPER_BUNDLE_NOARCHIVES = MakeNonPepperBundle('foo')
 NON_PEPPER_BUNDLE_ARCHIVES = MakeNonPepperBundle('bar', with_archives=True)
@@ -703,6 +706,45 @@ mac,canary,21.0.1156.0,2012-05-30 12:14:21.305090"""
     self.assertRaises(update_nacl_manifest.UnknownLockedBundleException,
                       self._Run, OS_MLW)
 
+  def testIgnoreLastDigitOnCanary(self):
+    # The final number in a canary build does not include any different
+    # changes, it is just a different experiment (e.g. using ASAN, or using
+    # aura). We should not compare these versions differently.
+    #
+    # Note that the version mapping will show that 31.0.1608.0 is different
+    # from 31.0.1608.1 -- this is because 31.0.1608.1 is built from the branch,
+    # not from trunk. Inspecting the branch would show that there are no
+    # changes (why would there be? No one has any reason to merge changes to a
+    # canary branch.)
+    self.manifest = MakeManifest(copy.deepcopy(BCANARY_NONE))
+    history = """win,canary,31.0.1608.1,2013-08-22 09:33:24.469760
+mac,canary,31.0.1608.0,2013-08-22 07:18:09.762600"""
+    self._AddCsvHistory(history)
+    self.version_mapping['31.0.1608.1'] = 'trunk.218914'
+    self.version_mapping['31.0.1608.0'] = 'trunk.218872'
+    my_bundle = MakePlatformBundle(31, 218872, '31.0.1608.0', OS_MLW)
+    self.files.Add(my_bundle)
+    self._MakeDelegate()
+    self._Run(OS_MLW)
+    self._ReadUploadedManifest()
+    self._AssertUploadedManifestHasBundle(my_bundle, CANARY)
+
+  def testDontIgnoreLastDigitForNonCanary(self):
+    self.manifest = MakeManifest(B26_NONE)
+    self.history.Add(OS_M, BETA, V26_0_1386_1)  # Only Mac
+    self.history.Add(OS_LW, BETA, V26_0_1386_0)  # Only Linux, Windows.
+    self.files.Add(B26_0_1386_0_MLW)
+
+    self._MakeDelegate()
+    # This raises because pepper_26 is not found in the history, and therefore
+    # "locked", but it also doesn't have an online version, therefore there is
+    # no good version number to upload.
+    #
+    # Basically we're asserting that 26.0.1386.1 != 26.0.1386.0, which would be
+    # true if it were canary.
+    self.assertRaises(update_nacl_manifest.UnknownLockedBundleException,
+                      self._Run, OS_MLW)
+
 
 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 f0db23a..07430f4 100755
--- a/native_client_sdk/src/build_tools/update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/update_nacl_manifest.py
@@ -368,11 +368,17 @@ class VersionFinder(object):
       A tuple (version, channel, archives). The version is a string such as
       "19.0.1084.41". The channel is always 'canary'. |archives| is a list of
       archive URLs."""
+    # Canary versions that differ in the last digit shouldn't be considered
+    # different; this number is typically used to represent an experiment, e.g.
+    # using ASAN or aura.
+    def CanaryKey(version):
+      return version[:-1]
+
     # We don't ship canary on Linux, so it won't appear in self.history.
     # Instead, we can use the matching Linux trunk build for that version.
     shared_version_generator = self._FindNextSharedVersion(
         set(self.platforms) - set(('linux',)),
-        self._GetPlatformCanaryHistory)
+        self._GetPlatformCanaryHistory, CanaryKey)
     return self._DoGetMostRecentSharedVersion(shared_version_generator,
                                               allow_trunk_revisions=True)
 
@@ -499,7 +505,7 @@ class VersionFinder(object):
         yield channel, version
 
 
-  def _FindNextSharedVersion(self, platforms, generator_func):
+  def _FindNextSharedVersion(self, platforms, generator_func, key_func=None):
     """Yields versions of Chrome that exist on all given platforms, in order of
        newest to oldest.
 
@@ -511,11 +517,17 @@ class VersionFinder(object):
           ('mac', 'linux', 'win')
       generator_func: A function which takes a platform and returns a
           generator that yields (channel, version) tuples.
+      key_func: A function to convert the version into a value that should be
+          used for comparison. See python built-in sorted() or min(), for
+          an example.
     Returns:
       A generator that yields a tuple (version, channel) for each version that
       matches all platforms and the major version. The version returned is a
       string (e.g. "18.0.1025.164").
     """
+    if not key_func:
+      key_func = lambda x: x
+
     platform_generators = []
     for platform in platforms:
       platform_generators.append(generator_func(platform))
@@ -533,13 +545,19 @@ class VersionFinder(object):
               platform, JoinVersion(platform_versions[i][1])))
         logger.info('Checking versions: %s' % ', '.join(msg_info))
 
-      shared_version = min(v for c, v in platform_versions)
+      shared_version = min((v for c, v in platform_versions), key=key_func)
+
+      if all(key_func(v) == key_func(shared_version)
+             for c, v in platform_versions):
+        # The real shared_version should be the real minimum version. This will
+        # be different from shared_version above only if key_func compares two
+        # versions with different values as equal.
+        min_version = min((v for c, v in platform_versions))
 
-      if all(v == shared_version for c, v in platform_versions):
         # grab the channel from an arbitrary platform
         first_platform = platform_versions[0]
         channel = first_platform[0]
-        yield JoinVersion(shared_version), channel
+        yield JoinVersion(min_version), channel
 
         # force increment to next version for all platforms
         shared_version = None