Merge WebKit at r70209: Initial merge by Git

Change-Id: Id23a68efa36e9d1126bcce0b137872db00892c8e

5 files changed, 117 insertions, 11 deletions

diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api.py b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
index ca28e32..72cad8d 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/api.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/api.py
@@ -83,13 +83,20 @@ class Checkout(object):
     def bug_id_for_revision(self, revision):
         return self.commit_info_for_revision(revision).bug_id()
 
-    def modified_changelogs(self, git_commit):
+    def _modified_files_matching_predicate(self, git_commit, predicate, changed_files=None):
         # SCM returns paths relative to scm.checkout_root
         # Callers (especially those using the ChangeLog class) may
         # expect absolute paths, so this method returns absolute paths.
-        changed_files = self._scm.changed_files(git_commit)
+        if not changed_files:
+            changed_files = self._scm.changed_files(git_commit)
         absolute_paths = [os.path.join(self._scm.checkout_root, path) for path in changed_files]
-        return [path for path in absolute_paths if self._is_path_to_changelog(path)]
+        return [path for path in absolute_paths if predicate(path)]
+
+    def modified_changelogs(self, git_commit, changed_files=None):
+        return self._modified_files_matching_predicate(git_commit, self._is_path_to_changelog, changed_files=changed_files)
+
+    def modified_non_changelogs(self, git_commit, changed_files=None):
+        return self._modified_files_matching_predicate(git_commit, lambda path: not self._is_path_to_changelog(path), changed_files=changed_files)
 
     def commit_message_for_this_commit(self, git_commit):
         changelog_paths = self.modified_changelogs(git_commit)
@@ -109,6 +116,17 @@ class Checkout(object):
         # FIXME: We should sort and label the ChangeLog messages like commit-log-editor does.
         return CommitMessage("".join(changelog_messages).splitlines())
 
+    def recent_commit_infos_for_files(self, paths):
+        revisions = set(sum(map(self._scm.revisions_changing_file, paths), []))
+        return set(map(self.commit_info_for_revision, revisions))
+
+    def suggested_reviewers(self, git_commit):
+        changed_files = self.modified_non_changelogs(git_commit)
+        commit_infos = self.recent_commit_infos_for_files(changed_files)
+        reviewers = [commit_info.reviewer() for commit_info in commit_infos if commit_info.reviewer()]
+        reviewers.extend([commit_info.author() for commit_info in commit_infos if commit_info.author() and commit_info.author().can_review])
+        return sorted(set(reviewers))
+
     def bug_id_for_this_commit(self, git_commit):
         try:
             return parse_bug_id(self.commit_message_for_this_commit(git_commit).message())
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
index fdfd879..d7bd95e 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
@@ -173,3 +173,24 @@ class CheckoutTest(unittest.TestCase):
         checkout = Checkout(scm)
         expected_changlogs = ["/foo/bar/ChangeLog", "/foo/bar/relative/path/ChangeLog"]
         self.assertEqual(checkout.modified_changelogs(git_commit=None), expected_changlogs)
+
+    def test_suggested_reviewers(self):
+        def mock_changelog_entries_for_revision(revision):
+            if revision % 2 == 0:
+                return [ChangeLogEntry(_changelog1entry1)]
+            return [ChangeLogEntry(_changelog1entry2)]
+
+        def mock_revisions_changing_file(path, limit=5):
+            if path.endswith("ChangeLog"):
+                return [3]
+            return [4, 8]
+
+        scm = Mock()
+        scm.checkout_root = "/foo/bar"
+        scm.changed_files = lambda git_commit: ["file1", "file2", "relative/path/ChangeLog"]
+        scm.revisions_changing_file = mock_revisions_changing_file
+        checkout = Checkout(scm)
+        checkout.changelog_entries_for_revision = mock_changelog_entries_for_revision
+        reviewers = checkout.suggested_reviewers(git_commit=None)
+        reviewer_names = [reviewer.full_name for reviewer in reviewers]
+        self.assertEqual(reviewer_names, [u'Tor Arne Vestb\xf8'])
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py b/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
index d8ebae6..a6ea756 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
@@ -33,9 +33,13 @@ import re
 
 _log = logging.getLogger("webkitpy.common.checkout.diff_parser")
 
+
+# FIXME: This is broken. We should compile our regexps up-front
+# instead of using a custom cache.
 _regexp_compile_cache = {}
 
 
+# FIXME: This function should be removed.
 def match(pattern, string):
     """Matches the string with the pattern, caching the compiled regexp."""
     if not pattern in _regexp_compile_cache:
@@ -43,12 +47,15 @@ def match(pattern, string):
     return _regexp_compile_cache[pattern].match(string)
 
 
+# FIXME: This belongs on DiffParser (e.g. as to_svn_diff()).
 def git_diff_to_svn_diff(line):
     """Converts a git formatted diff line to a svn formatted line.
 
     Args:
       line: A string representing a line of the diff.
     """
+    # FIXME: This list should be a class member on DiffParser.
+    # These regexp patterns should be compiled once instead of every time.
     conversion_patterns = (("^diff --git \w/(.+) \w/(?P<FilePath>.+)", lambda matched: "Index: " + matched.group('FilePath') + "\n"),
                            ("^new file.*", lambda matched: "\n"),
                            ("^index [0-9a-f]{7}\.\.[0-9a-f]{7} [0-9]{6}", lambda matched: "===================================================================\n"),
@@ -62,6 +69,7 @@ def git_diff_to_svn_diff(line):
     return line
 
 
+# FIXME: This method belongs on DiffParser
 def get_diff_converter(first_diff_line):
     """Gets a converter function of diff lines.
 
@@ -80,7 +88,7 @@ _DECLARED_FILE_PATH = 2
 _PROCESSING_CHUNK = 3
 
 
-class DiffFile:
+class DiffFile(object):
     """Contains the information for one file in a patch.
 
     The field "lines" is a list which contains tuples in this format:
@@ -88,6 +96,13 @@ class DiffFile:
     If deleted_line_number is zero, it means this line is newly added.
     If new_line_number is zero, it means this line is deleted.
     """
+    # FIXME: Tuples generally grow into classes.  We should consider
+    # adding a DiffLine object.
+
+    def added_or_modified_line_numbers(self):
+        # This logic was moved from patchreader.py, but may not be
+        # the right API for this object long-term.
+        return [line[1] for line in self.lines if not line[0]]
 
     def __init__(self, filename):
         self.filename = filename
@@ -103,13 +118,14 @@ class DiffFile:
         self.lines.append((deleted_line_number, new_line_number, line))
 
 
-class DiffParser:
+class DiffParser(object):
     """A parser for a patch file.
 
     The field "files" is a dict whose key is the filename and value is
     a DiffFile object.
     """
 
+    # FIXME: This function is way too long and needs to be broken up.
     def __init__(self, diff_input):
         """Parses a diff.
 
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py
index 793d96d..4bd9ed6 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/scm.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm.py
@@ -245,7 +245,10 @@ class SCM:
     def changed_files(self, git_commit=None):
         self._subclass_must_implement()
 
-    def changed_files_for_revision(self):
+    def changed_files_for_revision(self, revision):
+        self._subclass_must_implement()
+
+    def revisions_changing_file(self, path, limit=5):
         self._subclass_must_implement()
 
     def added_files(self):
@@ -257,7 +260,7 @@ class SCM:
     def display_name(self):
         self._subclass_must_implement()
 
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         self._subclass_must_implement()
 
     def committer_email_for_revision(self, revision):
@@ -427,6 +430,16 @@ class SVN(SCM):
         status_command = ["svn", "diff", "--summarize", "-c", revision]
         return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))
 
+    def revisions_changing_file(self, path, limit=5):
+        revisions = []
+        log_command = ['svn', 'log', '--quiet', '--limit=%s' % limit, path]
+        for line in self.run(log_command, cwd=self.checkout_root).splitlines():
+            match = re.search('^r(?P<revision>\d+) ', line)
+            if not match:
+                continue
+            revisions.append(int(match.group('revision')))
+        return revisions
+
     def conflicted_files(self):
         return self.run_status_and_extract_filenames(self.status_command(), self._status_regexp("C"))
 
@@ -444,11 +457,11 @@ class SVN(SCM):
         return "svn"
 
     # FIXME: This method should be on Checkout.
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         """Returns a byte array (str()) representing the patch file.
         Patch files are effectively binary since they may contain
         files of multiple different encodings."""
-        return self.run([self.script_path("svn-create-patch")],
+        return self.run([self.script_path("svn-create-patch")] + changed_files,
             cwd=self.checkout_root, return_stderr=False,
             decode_output=False)
 
@@ -653,6 +666,10 @@ class Git(SCM):
         commit_id = self.git_commit_from_svn_revision(revision)
         return self._changes_files_for_commit(commit_id)
 
+    def revisions_changing_file(self, path, limit=5):
+        commit_ids = self.run(["git", "log", "--pretty=format:%H", "-%s" % limit, path]).splitlines()
+        return filter(lambda revision: revision, map(self.svn_revision_from_git_commit, commit_ids))
+
     def conflicted_files(self):
         # We do not need to pass decode_output for this diff command
         # as we're passing --name-status which does not output any data.
@@ -672,12 +689,12 @@ class Git(SCM):
     def display_name(self):
         return "git"
 
-    def create_patch(self, git_commit=None):
+    def create_patch(self, git_commit=None, changed_files=[]):
         """Returns a byte array (str()) representing the patch file.
         Patch files are effectively binary since they may contain
         files of multiple different encodings."""
         # FIXME: This should probably use cwd=self.checkout_root
-        return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit)], decode_output=False)
+        return self.run(['git', 'diff', '--binary', "--no-ext-diff", "--full-index", "-M", self.merge_base(git_commit), "--"] + changed_files, decode_output=False)
 
     @classmethod
     def git_commit_from_svn_revision(cls, revision):
@@ -688,6 +705,12 @@ class Git(SCM):
             raise ScriptError(message='Failed to find git commit for revision %s, your checkout likely needs an update.' % revision)
         return git_commit
 
+    def svn_revision_from_git_commit(self, commit_id):
+        try:
+            return int(self.run(['git', 'svn', 'find-rev', commit_id]).rstrip())
+        except ValueError, e:
+            return None
+
     def contents_at_revision(self, path, revision):
         """Returns a byte array (str()) containing the contents
         of path @ revision in the repository."""
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py b/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py
index 87d5539..4aa5279 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py
@@ -352,6 +352,10 @@ class SCMTest(unittest.TestCase):
         self.assertRaises(ScriptError, self.scm.contents_at_revision, "test_file2", 2)
         self.assertRaises(ScriptError, self.scm.contents_at_revision, "does_not_exist", 2)
 
+    def _shared_test_revisions_changing_file(self):
+        self.assertEqual(self.scm.revisions_changing_file("test_file"), [5, 4, 3, 2])
+        self.assertRaises(ScriptError, self.scm.revisions_changing_file, "non_existent_file")
+
     def _shared_test_committer_email_for_revision(self):
         self.assertEqual(self.scm.committer_email_for_revision(3), getpass.getuser())  # Committer "email" will be the current user
 
@@ -696,6 +700,9 @@ Q1dTBx0AAAB42itg4GlgYJjGwMDDyODMxMDw34GBgQEAJPQDJA==
     def test_contents_at_revision(self):
         self._shared_test_contents_at_revision()
 
+    def test_revisions_changing_file(self):
+        self._shared_test_revisions_changing_file()
+
     def test_committer_email_for_revision(self):
         self._shared_test_committer_email_for_revision()
 
@@ -964,6 +971,10 @@ class GitSVNTest(SCMTest):
         self.scm.commit_locally_with_message("another test commit")
         self._two_local_commits()
 
+    def test_revisions_changing_files_with_local_commit(self):
+        self._one_local_commit()
+        self.assertEquals(self.scm.revisions_changing_file('test_file_commit1'), [])
+
     def test_commit_with_message(self):
         self._one_local_commit_plus_working_copy_changes()
         scm = detect_scm_system(self.git_checkout_path)
@@ -1087,6 +1098,20 @@ class GitSVNTest(SCMTest):
         self.assertTrue(re.search(r'test_file_commit2', patch))
         self.assertTrue(re.search(r'test_file_commit1', patch))
 
+    def test_create_patch_with_changed_files(self):
+        self._one_local_commit_plus_working_copy_changes()
+        scm = detect_scm_system(self.git_checkout_path)
+        patch = scm.create_patch(changed_files=['test_file_commit2'])
+        self.assertTrue(re.search(r'test_file_commit2', patch))
+
+    def test_create_patch_with_rm_and_changed_files(self):
+        self._one_local_commit_plus_working_copy_changes()
+        scm = detect_scm_system(self.git_checkout_path)
+        os.remove('test_file_commit1')
+        patch = scm.create_patch()
+        patch_with_changed_files = scm.create_patch(changed_files=['test_file_commit1', 'test_file_commit2'])
+        self.assertEquals(patch, patch_with_changed_files)
+
     def test_create_patch_git_commit(self):
         self._two_local_commits()
         scm = detect_scm_system(self.git_checkout_path)
@@ -1199,6 +1224,9 @@ class GitSVNTest(SCMTest):
     def test_contents_at_revision(self):
         self._shared_test_contents_at_revision()
 
+    def test_revisions_changing_file(self):
+        self._shared_test_revisions_changing_file()
+
     def test_added_files(self):
         self._shared_test_added_files()