Add --merge-base-ref option to lastchange.py

This option allows the chromecast platform team to remove some custom
logic in favor of a general flag that anyone can use. The
--merge-base-ref flag takes a ref, such as
"origin/chromium/src" and limits lastchange's calculations to the merge
base between HEAD and that ref

As part of this commit I simplified the VersionInfo class because it
didn't need the aditional complexity of not being named-tuple. After a
review of the various uses of RunGitCommand it seemed prudent to fold
in the returncode and error checking into the RunGitCommand function
to simplify the rest of the code.

This commit also moves towards a model of failing early and providing
messaging to the user about why the command failed. This led to
factoring out the FetchVersionInfo function which no longer does useful
work and to calculating the git top directory early in the process and
using it throughout.

Bug: 917159
Test: None
Change-Id: If58a956f51ccc019c7be860227a0039cdc0709b8
Reviewed-on: https://chromium-review.googlesource.com/c/1387690Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Eli Ribble <eliribble@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620447}

build/util/lastchange.py

@@ -7,28 +7,27 @@
 lastchange.py -- Chromium revision fetching utility.
 """
 
-import re
-import logging
 import argparse
+import collections
+import logging
 import os
 import subprocess
 import sys
 
-class VersionInfo(object):
-  def __init__(self, revision_id, full_revision_string, timestamp):
-    self.revision_id = revision_id
-    self.revision = full_revision_string
-    self.timestamp = timestamp
+VersionInfo = collections.namedtuple("VersionInfo",
+                                     ("revision_id", "revision", "timestamp"))
 
+class GitError(Exception):
+  pass
 
 def RunGitCommand(directory, command):
   """
   Launches git subcommand.
 
-  Errors are swallowed.
-
   Returns:
-    A process object or None.
+    The stripped stdout of the git command.
+  Raises:
+    GitError on failure, including a nonzero return code.
   """
   command = ['git'] + command
   # Force shell usage under cygwin. This is a workaround for
@@ -38,61 +37,81 @@ def RunGitCommand(directory, command):
   if sys.platform == 'cygwin':
     command = ['sh', '-c', ' '.join(command)]
   try:
+    logging.info("Executing '%s' in %s", ' '.join(command), directory)
     proc = subprocess.Popen(command,
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE,
                             cwd=directory,
                             shell=(sys.platform=='win32'))
-    return proc
+    stdout, stderr = proc.communicate()
+    stdout = stdout.strip()
+    logging.debug("returncode: %d", proc.returncode)
+    logging.debug("stdout: %s", stdout)
+    logging.debug("stderr: %s", stderr)
+    if proc.returncode != 0 or not stdout:
+      raise GitError((
+          "Git command 'git {}' in {} failed: "
+          "rc={}, stdout='{}' stderr='{}'").format(
+          " ".join(command), directory, proc.returncode, stdout, stderr))
+    return stdout
   except OSError as e:
-    logging.error('Command %r failed: %s' % (' '.join(command), e))
-    return None
-
+    raise GitError("Git command 'git {}' in {} failed: {}".format(
+        " ".join(command), directory, e))
 
-def FetchGitRevision(directory, filter):
+def GetMergeBase(directory, ref):
   """
-  Fetch the Git hash (and Cr-Commit-Position if any) for a given directory.
-
-  Errors are swallowed.
+  Return the merge-base of HEAD and ref.
 
+  Args:
+    directory: The directory containing the .git directory.
+    ref: The ref to use to find the merge base.
   Returns:
-    A VersionInfo object or None on error.
+    The git commit SHA of the merge-base as a string.
   """
-  hsh = ''
-  git_args = ['log', '-1', '--format=%H %ct']
-  if filter is not None:
-    git_args.append('--grep=' + filter)
-  proc = RunGitCommand(directory, git_args)
-  if proc:
-    output = proc.communicate()[0].strip()
-    if proc.returncode == 0 and output:
-      hsh, ct = output.split()
-    else:
-      logging.error('Git error: rc=%d, output=%r' %
-                    (proc.returncode, output))
-  if not hsh:
-    return None
-  pos = ''
-  proc = RunGitCommand(directory, ['cat-file', 'commit', hsh])
-  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
-  return VersionInfo(hsh, '%s-%s' % (hsh, pos), int(ct))
-
-
-def FetchVersionInfo(directory=None, filter=None):
+  logging.debug("Calculating merge base between HEAD and %s in %s",
+                ref, directory)
+  command = ['merge-base', 'HEAD', ref]
+  return RunGitCommand(directory, command)
+
+def FetchGitRevision(directory, commit_filter, start_commit="HEAD"):
   """
   Returns the last change (as a VersionInfo object)
   from some appropriate revision control system.
+
+
+  Args:
+    directory: The directory containing the .git directory.
+    commit_filter: A filter to supply to grep to filter commits
+    start_commit: A commit identifier. The result of this function
+      will be limited to only consider commits before the provided
+      commit.
+  Returns:
+    A VersionInfo object. On error all values will be 0.
   """
-  version_info = FetchGitRevision(directory, filter)
-  if not version_info:
-    version_info = VersionInfo('0', '0', 0)
-  return version_info
+  hash_ = ''
+
+  git_args = ['log', '-1', '--format=%H %ct']
+  if commit_filter is not None:
+    git_args.append('--grep=' + commit_filter)
+
+  git_args.append(start_commit)
+
+  output = RunGitCommand(directory, git_args)
+  hash_, commit_timestamp = output.split()
+  if not hash_:
+    return VersionInfo('0', '0', 0)
+
+  revision = hash_
+  output = RunGitCommand(directory, ['cat-file', 'commit', hash_])
+  for line in reversed(output.splitlines()):
+    if line.startswith('Cr-Commit-Position:'):
+      pos = line.rsplit()[-1].strip()
+      logging.debug("Found Cr-Commit-Position '%s'", pos)
+      revision = "{}-{}".format(hash_, pos)
+      break
+  return VersionInfo(hash_, revision, int(commit_timestamp))
+
+
 
 
 def GetHeaderGuard(path):
@@ -133,6 +152,16 @@ def GetHeaderContents(path, define, version):
   return header_contents
 
 
+def GetGitTopDirectory(source_dir):
+  """Get the top git directory - the directory that contains the .git directory.
+
+  Args:
+    source_dir: The directory to search.
+  Returns:
+    The output of "git rev-parse --show-toplevel" as a string
+  """
+  return RunGitCommand(source_dir, ['rev-parse', '--show-toplevel'])
+
 def WriteIfChanged(file_name, contents):
   """
   Writes the specified contents to the specified file_name
@@ -157,20 +186,23 @@ def main(argv=None):
 
   parser = argparse.ArgumentParser(usage="lastchange.py [options]")
   parser.add_argument("-m", "--version-macro",
-                    help="Name of C #define when using --header. Defaults to " +
-                    "LAST_CHANGE.",
-                    default="LAST_CHANGE")
+                    help=("Name of C #define when using --header. Defaults to "
+                          "LAST_CHANGE."))
   parser.add_argument("-o", "--output", metavar="FILE",
-                    help="Write last change to FILE. " +
-                    "Can be combined with --header to write both files.")
+                    help=("Write last change to FILE. "
+                          "Can be combined with --header to write both files."))
   parser.add_argument("--header", metavar="FILE",
                     help=("Write last change to FILE as a C/C++ header. "
                           "Can be combined with --output to write both files."))
+  parser.add_argument("--merge-base-ref",
+                    default=None,
+                    help=("Only consider changes since the merge "
+                          "base between HEAD and the provided ref"))
   parser.add_argument("--revision-id-only", action='store_true',
                     help=("Output the revision as a VCS revision ID only (in "
                           "Git, a 40-character commit hash, excluding the "
                           "Cr-Commit-Position)."))
-  parser.add_argument("--print-only", action='store_true',
+  parser.add_argument("--print-only", action="store_true",
                     help=("Just print the revision string. Overrides any "
                           "file-output-related options."))
   parser.add_argument("-s", "--source-dir", metavar="DIR",
@@ -180,13 +212,14 @@ def main(argv=None):
                           "matches the supplied filter regex. Defaults to "
                           "'^Change-Id:' to suppress local commits."),
                     default='^Change-Id:')
+
   args, extras = parser.parse_known_args(argv[1:])
 
   logging.basicConfig(level=logging.WARNING)
 
   out_file = args.output
   header = args.header
-  filter=args.filter
+  commit_filter = args.filter
 
   while len(extras) and out_file is None:
     if out_file is None:
@@ -196,18 +229,37 @@ def main(argv=None):
     parser.print_help()
     sys.exit(2)
 
-  if args.source_dir:
-    src_dir = args.source_dir
+  source_dir = args.source_dir or os.path.dirname(os.path.abspath(__file__))
+  try:
+    git_top_dir = GetGitTopDirectory(source_dir)
+  except GitError as e:
+    logging.error("Failed to get git top directory from '%s': %s",
+                  source_dir, e)
+    return 2
+
+  if args.merge_base_ref:
+    try:
+      merge_base_sha = GetMergeBase(git_top_dir, args.merge_base_ref)
+    except GitError as e:
+      logging.error("You requested a --merge-base-ref value of '%s' but no "
+                    "merge base could be found between it and HEAD. Git "
+                    "reports: %s", args.merge_base_ref, e)
+      return 3
   else:
-    src_dir = os.path.dirname(os.path.abspath(__file__))
+    merge_base_sha = 'HEAD'
+
+  try:
+    version_info = FetchGitRevision(git_top_dir, commit_filter, merge_base_sha)
+  except GitError as e:
+    logging.error("Failed to get version info: %s")
+    return 1
 
-  version_info = FetchVersionInfo(directory=src_dir, filter=filter)
   revision_string = version_info.revision
   if args.revision_id_only:
     revision_string = version_info.revision_id
 
   if args.print_only:
-    print revision_string
+    print(revision_string)
   else:
     contents = "LASTCHANGE=%s\n" % revision_string
     if not out_file and not args.header: