diff options
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/common/checkout')
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() |