[release-tools] Tool to find related commits

tools/release/search_related_commits.py

+#!/usr/bin/env python
+# Copyright 2015 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import argparse
+import os
+import sys
+import re
+import operator
+from subprocess import Popen, PIPE
+
+def search_all_related_commits(
+    git_working_dir, start_hash, until, deadline, verbose=False):
+
+  all_commits_raw = _find_commits_inbetween(
+      start_hash, until, git_working_dir, verbose)
+  if verbose:
+    print "All commits between <of> and <until>: " + all_commits_raw
+
+  #Adding start hash too

[Michael Achenbach, 2015/05/04 08:57:53]

nit: space after # - also below

[Michael Hablich, 2015/05/04 12:17:31]

Done.

+  all_commits = [start_hash]
+  all_commits.extend(all_commits_raw.splitlines())
+  all_related_commits = {}
+  already_treated_commits = []

[Michael Achenbach, 2015/05/04 08:57:53]

Make already_treated_commits a set as it has lots of inclusion checks and order
doesn't matter?

[Michael Hablich, 2015/05/04 12:17:31]

Acknowledged.

+  for commit in all_commits:
+    if commit in already_treated_commits:
+      continue
+
+    related_commits = search_related_commits(
+        git_working_dir, commit, until, deadline, verbose)
+    if len(related_commits) > 0:
+      all_related_commits[commit] = related_commits
+      already_treated_commits.extend(related_commits)
+
+    already_treated_commits.append(commit)
+
+  return all_related_commits
+
+def search_related_commits(
+    git_working_dir, start_hash, until, deadline, verbose=False):
+
+  if deadline:
+    commits_between = _find_commits_inbetween(
+        start_hash, deadline, git_working_dir, verbose)
+    if commits_between == "":
+      return []
+
+  #Extract commit position
+  original_message = _git_execute(
+      git_working_dir,
+      ["show", "-s", "--format=%B", start_hash],
+      verbose)
+  title = original_message.splitlines()[0]
+
+  matches = re.search("(\{#)([0-9]*)(\})", original_message)
+  commit_position = matches.group(2)
+  if verbose:
+    print "1.) Commit position to look for: " + commit_position
+
+  search_range = start_hash + ".." + until
+
+  def git_args(grep_pattern):
+    return [
+      "log",
+      "--reverse",
+      "--grep=" + grep_pattern,
+      "--format=%H",
+      search_range,
+    ]
+
+  found_by_hash = _git_execute(
+      git_working_dir, git_args(start_hash), verbose).strip()
+
+  if verbose:
+    print "2.) Found by hash: " + found_by_hash
+
+  found_by_commit_pos = _git_execute(
+      git_working_dir, git_args(commit_position), verbose).strip()
+
+  if verbose:
+    print "3.) Found by commit position: " + found_by_commit_pos
+
+  # Replace brackets or else they are wrongly interpreted by --grep

[Michael Achenbach, 2015/05/04 08:57:53]

Are there no other things that need to be escaped? Is there a general regexp
escape method in python that could be applied?

[Michael Hablich, 2015/05/04 12:17:31]

The String is already escaped when it is given to Popen automatically. So
re.escape is not producing the right results as in that case the curly braces
are wrong.

When the I have only found that the square brackets need some special handling. 

So this seems to be ok.

+  title = title.replace("[", "\\[")
+  title = title.replace("]", "\\]")
+
+  found_by_title = _git_execute(
+      git_working_dir, git_args(title), verbose).strip()
+
+  if verbose:
+    print "4.) Found by title: " + found_by_title
+
+  hits = (
+      _convert_to_array(found_by_hash) +
+      _convert_to_array(found_by_commit_pos) +
+      _convert_to_array(found_by_title))
+  hits = _remove_duplicates(hits)
+
+  if deadline:
+    for current_hit in hits:
+      commits_between = _find_commits_inbetween(
+          deadline, current_hit, git_working_dir, verbose)
+      if commits_between != "":
+        return hits
+    return []
+
+  return hits
+
+def _find_commits_inbetween(start_hash, end_hash, git_working_dir, verbose):
+  commits_between = _git_execute(
+        git_working_dir,
+        ["rev-list", "--reverse", start_hash + ".." + end_hash],
+        verbose)
+  return commits_between.strip()
+
+def _convert_to_array(string_of_hashes):
+  return string_of_hashes.splitlines()
+
+def _remove_duplicates(array):
+   no_duplicates = []
+   for current in array:
+    if not current in no_duplicates:
+      no_duplicates.append(current)
+   return no_duplicates
+
+def _git_execute(working_dir, args, verbose=False):
+  command = ["git", "-C", working_dir] + args
+  if verbose:
+    print "Git working dir: " + working_dir
+    print "Executing git command:" + str(command)
+  p = Popen(args=command, stdin=PIPE,
+            stdout=PIPE, stderr=PIPE)
+  output, err = p.communicate()
+  rc = p.returncode
+  if rc != 0:
+    raise Exception(err)
+  if verbose:
+    print "Git return value: " + output
+  return output
+
+def _pretty_print_entry(hash, git_dir, pre_text, verbose):
+  text_to_print = _git_execute(
+      git_dir,
+      ["show",
+       "--quiet",
+       "--date=iso",
+       hash,
+       "--format=%ad # %H # %s"],
+      verbose)
+  return pre_text + text_to_print.strip()
+
+def main(options):
+
+  output = []
+  if options.all:
+    all_related_commits = search_all_related_commits(
+        options.git_dir,
+        options.of[0],
+        options.until[0],
+        options.deadline,
+        options.verbose)
+
+    sort_key = lambda x: (
+        _git_execute(
+            options.git_dir,
+            ["show", "--quiet", "--date=iso", x, "--format=%ad"],
+            options.verbose)).strip()
+
+    high_level_commits = sorted(all_related_commits.keys(), key=sort_key)
+
+    for current_key in high_level_commits:
+      if options.prettyprint:
+        output.append(_pretty_print_entry(
+            current_key, options.git_dir, "+", options.verbose))
+      else:
+        output.append("+" + current_key)
+
+      found_commits = all_related_commits[current_key]
+      for current_commit in found_commits:
+        if options.prettyprint:
+          output.append(_pretty_print_entry(
+              current_commit, options.git_dir, "| ", options.verbose))
+        else:
+          output.append("| " + current_commit)
+  else:
+    hits = search_related_commits(
+        options.git_dir, options.of[0],
+        options.until[0], options.deadline, options.verbose)
+    if hits:
+      output.append("\n".join(hits))

[Michael Achenbach, 2015/05/04 08:57:53]

As you _use_ main() like a generator, how about making it a generator. E.g.
instead of 
output.append("\n".join(hits))
write:
yield "\n".join(hits)

and remove "output = []" and "return output"

[Michael Hablich, 2015/05/04 12:17:31]

sgtm

+
+  return output
+
+if __name__ == "__main__":  # pragma: no cover
+  parser = argparse.ArgumentParser(
+      "This tool searches the git repository for "

[Michael Achenbach, 2015/05/04 08:57:53]

Is this description accurate? Isn't the tool searching through all commits
between of and until?

[Michael Hablich, 2015/05/04 12:17:31]

Updated description.

+      "commits which are related to the commit <of>.")
+  parser.add_argument("-g", "--git-dir", required=False, default=".",
+                        help="The path to your git working directory.")
+  parser.add_argument("--verbose", action="store_true",

[Michael Achenbach, 2015/05/04 08:57:53]

This is quite chatty. How about calling this "--debug"? I don't think it is
useful for the end user, i.e. in the sense of "give me some more details".

[Michael Hablich, 2015/05/04 12:17:31]

The intention is to keep in line with the rest of the tools which only have
--verbose

+      help="Enables verbose output")
+  parser.add_argument("of", nargs=1,
+      help="Hash of the commit to be searched.")
+  parser.add_argument("until", nargs=1,
+      help="Commit when searching should stop")
+  parser.add_argument("--all", action="store_true",
+      help="Searches for related commits in all "
+            "commits between <of> and <until>")
+  parser.add_argument("--deadline", required=False,

[Michael Achenbach, 2015/05/04 08:57:53]

"deadline" and "until" are very similar in their word meaning. Maybe name
"deadline" "separator" or "branch-point" or something?

[Michael Hablich, 2015/05/04 12:17:31]

separator sounds good. 

+      help="The script will only list related commits "
+            "which are separated by hash <--deadline>.")
+  parser.add_argument("--prettyprint", action="store_true",
+      help="Pretty prints the output")
+
+  args = sys.argv[1:]
+  options = parser.parse_args(args)
+  for current_line in main(options):
+    print current_line