[NaCl SDK] Fix a bug in build_version.ChromeVersion()

native_client_sdk/src/build_tools/build_version.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Small utility library of python functions used during SDK building.
 """
 
 import os
 import re
 import sys
@@ skipping 12 matching lines @@
 
 
 def ChromeVersion():
   '''Extract chrome version from src/chrome/VERSION + svn.
 
   Returns:
     Chrome version string or trunk + svn rev.
   '''
   info = FetchVersionInfo()
   if info.url == 'git':
-    _, ref, revision = ParseCommitPosition(info)
+    _, ref, revision = ParseCommitPosition(info.revision)
     if ref == 'refs/heads/master':
       return 'trunk.%s' % revision
   return ChromeVersionNoTrunk()
 
 
 def ChromeVersionNoTrunk():
   '''Extract the chrome version from src/chrome/VERSION.
   Ignore whether this is a trunk build.
 
   Returns:
@@ skipping 61 matching lines @@
 
   version_info = (lastchange.FetchSVNRevision(directory, svn_url_regex) or
                   lastchange.FetchGitSVNRevision(directory, svn_url_regex) or
                   FetchGitCommitPosition(directory))
   if not version_info:
     version_info = lastchange.VersionInfo(None, None)
   return version_info
 
 
 def FetchGitCommitPosition(directory=None):
   '''

[Sam Clegg, 2014/08/29 11:51:45]

I think the first line of the docstring should be on the same
line as the opening tripple quote:

"""First line

Long desc
"""

[binji, 2014/08/29 16:01:28]

Done.

   Return the "commit-position" of the Chromium git repo. This should be
-  equivalent to the SVN revision if one eixsts.
+  equivalent to the SVN revision if one exists.
+  '''
+  SEARCH_LIMIT = 100
+  for i in xrange(SEARCH_LIMIT):
+    cmd = ['show', '-s', '--format=%H%n%B', 'HEAD~%d' % i]
+    proc = lastchange.RunGitCommand(directory, cmd)
+    if not proc:
+      break
 
-  This is a copy of the (recently reverted) change in lastchange.py.
-  TODO(binji): Move this logic to lastchange.py when the dust settles.
-  (see crbug.com/406783)
-  '''
-  hsh = ''
-  proc = lastchange.RunGitCommand(directory, ['rev-parse', 'HEAD'])
-  if proc:
-    output = proc.communicate()[0].strip()
-    if proc.returncode == 0 and output:
-      hsh = output
-  if not hsh:
-    return None
-  pos = ''
-  proc = lastchange.RunGitCommand(directory,
-                                  ['show', '-s', '--format=%B', 'HEAD'])
-  if proc:
     output = proc.communicate()[0]
-    if proc.returncode == 0 and output:
-      for line in reversed(output.splitlines()):
-        if line.startswith('Cr-Commit-Position:'):
-          pos = line.rsplit()[-1].strip()
-          break
-  if not pos:
-    return lastchange.VersionInfo('git', hsh)
-  return lastchange.VersionInfo('git', '%s-%s' % (hsh, pos))
+    if not (proc.returncode == 0 and output):
+      break
+
+    lines = output.splitlines()
+
+    # First line is the hash.
+    hsh = lines[0]
+    if not re.match(r'[0-9a-fA-F]+', hsh):
+      break
+
+    for line in reversed(lines):
+      if line.startswith('Cr-Commit-Position:'):
+        pos = line.rsplit()[-1].strip()
+        return lastchange.VersionInfo('git', '%s-%s' % (hsh, pos))
+
+  return lastchange.VersionInfo(None, None)

[Sam Clegg, 2014/08/29 11:51:45]

Should we assert / raise here?

[binji, 2014/08/29 16:01:28]

Done.

+
 
 
 def ParseCommitPosition(commit_position):
   '''
   Parse a Chrome commit position into its components.
 
   Given a commit position like:
     0178d4831bd36b5fb9ff477f03dc43b11626a6dc-refs/heads/master@{#292238}
   Returns:
     ("0178d4831bd36b5fb9ff477f03dc43b11626a6dc", "refs/heads/master", "292238")
   '''
-  m = re.match(r'([0-9a-fA-F]+)-([^@]+)@{#(\d+)}', commit_position)
+  m = re.match(r'([0-9a-fA-F]+)(?:-([^@]+)@{#(\d+)})?', commit_position)
   if m:
     return m.groups()
   return None