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

native_client_sdk/src/build_tools/build_version.py

Index: native_client_sdk/src/build_tools/build_version.py
diff --git a/native_client_sdk/src/build_tools/build_version.py b/native_client_sdk/src/build_tools/build_version.py
index 91e1381bd49a031fa12ba8bf613033958dd8a84e..c2eb561b998292db1d77485b9f49db6bd68911de 100644
--- a/native_client_sdk/src/build_tools/build_version.py
+++ b/native_client_sdk/src/build_tools/build_version.py
@@ -30,7 +30,7 @@ def ChromeVersion():
   '''
   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()
@@ -114,33 +114,33 @@ def FetchVersionInfo(directory=None,
 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.
-
-  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)
+  equivalent to the SVN revision if one exists.
   '''
-  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:
+  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
+
     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):
@@ -152,7 +152,7 @@ def ParseCommitPosition(commit_position):
   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