Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(32)

Side by Side Diff: gcl.py

Issue 6696010: Modify gcl to use suggested_reviewer output from presubmit_support. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/tools/depot_tools
Patch Set: update w/ review feedback Created 9 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | tests/gcl_unittest.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 #!/usr/bin/python 1 #!/usr/bin/python
2 # Copyright (c) 2010 The Chromium Authors. All rights reserved. 2 # Copyright (c) 2010 The Chromium Authors. All rights reserved.
3 # Use of this source code is governed by a BSD-style license that can be 3 # Use of this source code is governed by a BSD-style license that can be
4 # found in the LICENSE file. 4 # found in the LICENSE file.
5 5
6 """\ 6 """\
7 Wrapper script around Rietveld's upload.py that simplifies working with groups 7 Wrapper script around Rietveld's upload.py that simplifies working with groups
8 of files. 8 of files.
9 """ 9 """
10 10
(...skipping 21 matching lines...) Expand all
32 json.loads 32 json.loads
33 except (ImportError, AttributeError): 33 except (ImportError, AttributeError):
34 # Import the one included in depot_tools. 34 # Import the one included in depot_tools.
35 sys.path.append(os.path.join(os.path.dirname(__file__), 'third_party')) 35 sys.path.append(os.path.join(os.path.dirname(__file__), 'third_party'))
36 import simplejson as json # pylint: disable=F0401 36 import simplejson as json # pylint: disable=F0401
37 37
38 import breakpad # pylint: disable=W0611 38 import breakpad # pylint: disable=W0611
39 39
40 # gcl now depends on gclient. 40 # gcl now depends on gclient.
41 from scm import SVN 41 from scm import SVN
42
42 import gclient_utils 43 import gclient_utils
44 import owners
45 import presubmit_support
43 46
44 __version__ = '1.2' 47 __version__ = '1.2'
45 48
46 49
47 CODEREVIEW_SETTINGS = { 50 CODEREVIEW_SETTINGS = {
48 # To make gcl send reviews to a server, check in a file named 51 # To make gcl send reviews to a server, check in a file named
49 # "codereview.settings" (see |CODEREVIEW_SETTINGS_FILE| below) to your 52 # "codereview.settings" (see |CODEREVIEW_SETTINGS_FILE| below) to your
50 # project's base directory and add the following line to codereview.settings: 53 # project's base directory and add the following line to codereview.settings:
51 # CODE_REVIEW_SERVER: codereview.yourserver.org 54 # CODE_REVIEW_SERVER: codereview.yourserver.org
52 } 55 }
(...skipping 10 matching lines...) Expand all
63 # Warning message when the change appears to be missing tests. 66 # Warning message when the change appears to be missing tests.
64 MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!" 67 MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!"
65 68
66 # Global cache of files cached in GetCacheDir(). 69 # Global cache of files cached in GetCacheDir().
67 FILES_CACHE = {} 70 FILES_CACHE = {}
68 71
69 # Valid extensions for files we want to lint. 72 # Valid extensions for files we want to lint.
70 DEFAULT_LINT_REGEX = r"(.*\.cpp|.*\.cc|.*\.h)" 73 DEFAULT_LINT_REGEX = r"(.*\.cpp|.*\.cc|.*\.h)"
71 DEFAULT_LINT_IGNORE_REGEX = r"$^" 74 DEFAULT_LINT_IGNORE_REGEX = r"$^"
72 75
76 REVIEWERS_REGEX = r'\s*R=(.+)'
73 77
74 def CheckHomeForFile(filename): 78 def CheckHomeForFile(filename):
75 """Checks the users home dir for the existence of the given file. Returns 79 """Checks the users home dir for the existence of the given file. Returns
76 the path to the file if it's there, or None if it is not. 80 the path to the file if it's there, or None if it is not.
77 """ 81 """
78 home_vars = ['HOME'] 82 home_vars = ['HOME']
79 if sys.platform in ('cygwin', 'win32'): 83 if sys.platform in ('cygwin', 'win32'):
80 home_vars.append('USERPROFILE') 84 home_vars.append('USERPROFILE')
81 for home_var in home_vars: 85 for home_var in home_vars:
82 home = os.getenv(home_var) 86 home = os.getenv(home_var)
(...skipping 196 matching lines...) Expand 10 before | Expand all | Expand 10 after
279 rietveld: rietveld server for this change 283 rietveld: rietveld server for this change
280 """ 284 """
281 # Kept for unit test support. This is for the old format, it's deprecated. 285 # Kept for unit test support. This is for the old format, it's deprecated.
282 _SEPARATOR = "\n-----\n" 286 _SEPARATOR = "\n-----\n"
283 287
284 def __init__(self, name, issue, patchset, description, files, local_root, 288 def __init__(self, name, issue, patchset, description, files, local_root,
285 rietveld, needs_upload=False): 289 rietveld, needs_upload=False):
286 self.name = name 290 self.name = name
287 self.issue = int(issue) 291 self.issue = int(issue)
288 self.patchset = int(patchset) 292 self.patchset = int(patchset)
289 self.description = description 293 self._description = None
294 self._subject = None
295 self._reviewers = None
296 self._set_description(description)
290 if files is None: 297 if files is None:
291 files = [] 298 files = []
292 self._files = files 299 self._files = files
293 self.patch = None 300 self.patch = None
294 self._local_root = local_root 301 self._local_root = local_root
295 self.needs_upload = needs_upload 302 self.needs_upload = needs_upload
296 self.rietveld = rietveld 303 self.rietveld = rietveld
297 if not self.rietveld: 304 if not self.rietveld:
298 # Set the default value. 305 # Set the default value.
299 self.rietveld = GetCodeReviewSetting('CODE_REVIEW_SERVER') 306 self.rietveld = GetCodeReviewSetting('CODE_REVIEW_SERVER')
300 307
308 def _get_description(self):
309 return self._description
310
311 def _set_description(self, description):
312 # TODO(dpranke): Cloned from git_cl.py. These should be shared.
313 if not description:
314 self._description = description
315 return
316
317 parsed_lines = []
318 reviewers_re = re.compile(REVIEWERS_REGEX)
319 reviewers = ''
320 subject = ''
321 for l in description.splitlines():
322 if not subject:
323 subject = l
324 matched_reviewers = reviewers_re.match(l)
325 if matched_reviewers:
326 reviewers = matched_reviewers.group(1)
327 parsed_lines.append(l)
328
329 if len(subject) > 100:
330 subject = subject[:97] + '...'
331
332 self._subject = subject
333 self._reviewers = reviewers
334 self._description = '\n'.join(parsed_lines)
335
336 description = property(_get_description, _set_description)
337
338 @property
339 def reviewers(self):
340 return self._reviewers
341
342 @property
343 def subject(self):
344 return self._subject
345
301 def NeedsUpload(self): 346 def NeedsUpload(self):
302 return self.needs_upload 347 return self.needs_upload
303 348
304 def GetFileNames(self): 349 def GetFileNames(self):
305 """Returns the list of file names included in this change.""" 350 """Returns the list of file names included in this change."""
306 return [f[1] for f in self._files] 351 return [f[1] for f in self._files]
307 352
308 def GetFiles(self): 353 def GetFiles(self):
309 """Returns the list of files included in this change with their status.""" 354 """Returns the list of files included in this change with their status."""
310 return self._files 355 return self._files
(...skipping 396 matching lines...) Expand 10 before | Expand all | Expand 10 after
707 752
708 return editor 753 return editor
709 754
710 755
711 def GenerateDiff(files, root=None): 756 def GenerateDiff(files, root=None):
712 return SVN.GenerateDiff(files, root=root) 757 return SVN.GenerateDiff(files, root=root)
713 758
714 759
715 def OptionallyDoPresubmitChecks(change_info, committing, args): 760 def OptionallyDoPresubmitChecks(change_info, committing, args):
716 if FilterFlag(args, "--no_presubmit") or FilterFlag(args, "--force"): 761 if FilterFlag(args, "--no_presubmit") or FilterFlag(args, "--force"):
717 return True 762 return presubmit_support.PresubmitOutput()
718 return DoPresubmitChecks(change_info, committing, True) 763 return DoPresubmitChecks(change_info, committing, True)
719 764
720 765
766 def suggest_reviewers(change_info, affected_files):
767 owners_db = owners.Database(change_info.LocalRoot(), fopen=file,
768 os_path=os.path)
769 return owners_db.reviewers_for(affected_files)
770
771
721 def defer_attributes(a, b): 772 def defer_attributes(a, b):
722 """Copy attributes from an object (like a function) to another.""" 773 """Copy attributes from an object (like a function) to another."""
723 for x in dir(a): 774 for x in dir(a):
724 if not getattr(b, x, None): 775 if not getattr(b, x, None):
725 setattr(b, x, getattr(a, x)) 776 setattr(b, x, getattr(a, x))
726 777
727 778
728 def need_change(function): 779 def need_change(function):
729 """Converts args -> change_info.""" 780 """Converts args -> change_info."""
730 # pylint: disable=W0612,W0621 781 # pylint: disable=W0612,W0621
(...skipping 59 matching lines...) Expand 10 before | Expand all | Expand 10 after
790 def CMDupload(change_info, args): 841 def CMDupload(change_info, args):
791 """Uploads the changelist to the server for review. 842 """Uploads the changelist to the server for review.
792 843
793 This does not submit a try job; use gcl try to submit a try job. 844 This does not submit a try job; use gcl try to submit a try job.
794 """ 845 """
795 if '-s' in args or '--server' in args: 846 if '-s' in args or '--server' in args:
796 ErrorExit('Don\'t use the -s flag, fix codereview.settings instead') 847 ErrorExit('Don\'t use the -s flag, fix codereview.settings instead')
797 if not change_info.GetFiles(): 848 if not change_info.GetFiles():
798 print "Nothing to upload, changelist is empty." 849 print "Nothing to upload, changelist is empty."
799 return 0 850 return 0
800 if not OptionallyDoPresubmitChecks(change_info, False, args): 851
852 output = OptionallyDoPresubmitChecks(change_info, False, args)
853 if not output.should_continue():
801 return 1 854 return 1
802 no_watchlists = (FilterFlag(args, "--no_watchlists") or 855 no_watchlists = (FilterFlag(args, "--no_watchlists") or
803 FilterFlag(args, "--no-watchlists")) 856 FilterFlag(args, "--no-watchlists"))
804 857
805 # Map --send-mail to --send_mail 858 # Map --send-mail to --send_mail
806 if FilterFlag(args, "--send-mail"): 859 if FilterFlag(args, "--send-mail"):
807 args.append("--send_mail") 860 args.append("--send_mail")
808 861
809 upload_arg = ["upload.py", "-y"] 862 upload_arg = ["upload.py", "-y"]
810 upload_arg.append("--server=%s" % change_info.rietveld) 863 upload_arg.append("--server=%s" % change_info.rietveld)
864
865 reviewers = change_info.reviewers or output.reviewers
866 if (reviewers and
867 not any(arg.startswith('-r') or arg.startswith('--reviewer') for
868 arg in args)):
869 upload_arg.append('--reviewers=%s' % ','.join(reviewers))
870
811 upload_arg.extend(args) 871 upload_arg.extend(args)
812 872
813 desc_file = "" 873 desc_file = ""
814 if change_info.issue: # Uploading a new patchset. 874 if change_info.issue: # Uploading a new patchset.
815 found_message = False 875 found_message = False
816 for arg in args: 876 for arg in args:
817 if arg.startswith("--message") or arg.startswith("-m"): 877 if arg.startswith("--message") or arg.startswith("-m"):
818 found_message = True 878 found_message = True
819 break 879 break
820 880
(...skipping 13 matching lines...) Expand all
834 watchlist = watchlists.Watchlists(change_info.GetLocalRoot()) 894 watchlist = watchlists.Watchlists(change_info.GetLocalRoot())
835 watchers = watchlist.GetWatchersForPaths(change_info.GetFileNames()) 895 watchers = watchlist.GetWatchersForPaths(change_info.GetFileNames())
836 896
837 cc_list = GetCodeReviewSetting("CC_LIST") 897 cc_list = GetCodeReviewSetting("CC_LIST")
838 if not no_watchlists and watchers: 898 if not no_watchlists and watchers:
839 # Filter out all empty elements and join by ',' 899 # Filter out all empty elements and join by ','
840 cc_list = ','.join(filter(None, [cc_list] + watchers)) 900 cc_list = ','.join(filter(None, [cc_list] + watchers))
841 if cc_list: 901 if cc_list:
842 upload_arg.append("--cc=" + cc_list) 902 upload_arg.append("--cc=" + cc_list)
843 upload_arg.append("--description_file=" + desc_file + "") 903 upload_arg.append("--description_file=" + desc_file + "")
844 if change_info.description: 904 if change_info.subject:
845 subject = change_info.description[:77] 905 upload_arg.append("--message=" + change_info.subject)
846 if subject.find("\r\n") != -1:
847 subject = subject[:subject.find("\r\n")]
848 if subject.find("\n") != -1:
849 subject = subject[:subject.find("\n")]
850 if len(change_info.description) > 77:
851 subject = subject + "..."
852 upload_arg.append("--message=" + subject)
853 906
854 if GetCodeReviewSetting("PRIVATE") == "True": 907 if GetCodeReviewSetting("PRIVATE") == "True":
855 upload_arg.append("--private") 908 upload_arg.append("--private")
856 909
857 # Change the current working directory before calling upload.py so that it 910 # Change the current working directory before calling upload.py so that it
858 # shows the correct base. 911 # shows the correct base.
859 previous_cwd = os.getcwd() 912 previous_cwd = os.getcwd()
860 os.chdir(change_info.GetLocalRoot()) 913 os.chdir(change_info.GetLocalRoot())
861 # If we have a lot of files with long paths, then we won't be able to fit 914 # If we have a lot of files with long paths, then we won't be able to fit
862 # the command to "svn diff". Instead, we generate the diff manually for 915 # the command to "svn diff". Instead, we generate the diff manually for
(...skipping 111 matching lines...) Expand 10 before | Expand all | Expand 10 after
974 output = RunShell(commit_cmd, True) 1027 output = RunShell(commit_cmd, True)
975 os.remove(commit_filename) 1028 os.remove(commit_filename)
976 os.remove(targets_filename) 1029 os.remove(targets_filename)
977 if output.find("Committed revision") != -1: 1030 if output.find("Committed revision") != -1:
978 change_info.Delete() 1031 change_info.Delete()
979 1032
980 if change_info.issue: 1033 if change_info.issue:
981 revision = re.compile(".*?\nCommitted revision (\d+)", 1034 revision = re.compile(".*?\nCommitted revision (\d+)",
982 re.DOTALL).match(output).group(1) 1035 re.DOTALL).match(output).group(1)
983 viewvc_url = GetCodeReviewSetting("VIEW_VC") 1036 viewvc_url = GetCodeReviewSetting("VIEW_VC")
984 change_info.description = change_info.description + '\n' 1037 change_info.description += '\n'
985 if viewvc_url: 1038 if viewvc_url:
986 change_info.description += "\nCommitted: " + viewvc_url + revision 1039 change_info.description += "\nCommitted: " + viewvc_url + revision
987 change_info.CloseIssue() 1040 change_info.CloseIssue()
988 os.chdir(previous_cwd) 1041 os.chdir(previous_cwd)
989 return 0 1042 return 0
990 1043
991 1044
992 def CMDchange(args): 1045 def CMDchange(args):
993 """Creates or edits a changelist. 1046 """Creates or edits a changelist.
994 1047
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
1036 description = change_info.description 1089 description = change_info.description
1037 1090
1038 other_files = GetFilesNotInCL() 1091 other_files = GetFilesNotInCL()
1039 1092
1040 # Edited files (as opposed to files with only changed properties) will have 1093 # Edited files (as opposed to files with only changed properties) will have
1041 # a letter for the first character in the status string. 1094 # a letter for the first character in the status string.
1042 file_re = re.compile(r"^[a-z].+\Z", re.IGNORECASE) 1095 file_re = re.compile(r"^[a-z].+\Z", re.IGNORECASE)
1043 affected_files = [x for x in other_files if file_re.match(x[0])] 1096 affected_files = [x for x in other_files if file_re.match(x[0])]
1044 unaffected_files = [x for x in other_files if not file_re.match(x[0])] 1097 unaffected_files = [x for x in other_files if not file_re.match(x[0])]
1045 1098
1099 suggested_reviewers = suggest_reviewers(change_info, affected_files)
1100 if suggested_reviewers:
1101 reviewers_re = re.compile(REVIEWERS_REGEX)
1102 if not any(
1103 reviewers_re.match(l) for l in description.splitlines()):
1104 description += '\nR=' + ','.join(suggested_reviewers) + '\n'
1105
1046 separator1 = ("\n---All lines above this line become the description.\n" 1106 separator1 = ("\n---All lines above this line become the description.\n"
1047 "---Repository Root: " + change_info.GetLocalRoot() + "\n" 1107 "---Repository Root: " + change_info.GetLocalRoot() + "\n"
1048 "---Paths in this changelist (" + change_info.name + "):\n") 1108 "---Paths in this changelist (" + change_info.name + "):\n")
1049 separator2 = "\n\n---Paths modified but not in any changelist:\n\n" 1109 separator2 = "\n\n---Paths modified but not in any changelist:\n\n"
1050 text = (description + separator1 + '\n' + 1110 text = (description + separator1 + '\n' +
1051 '\n'.join([f[0] + f[1] for f in change_info.GetFiles()])) 1111 '\n'.join([f[0] + f[1] for f in change_info.GetFiles()]))
1052 1112
1053 if change_info.Exists(): 1113 if change_info.Exists():
1054 text += (separator2 + 1114 text += (separator2 +
1055 '\n'.join([f[0] + f[1] for f in affected_files]) + '\n') 1115 '\n'.join([f[0] + f[1] for f in affected_files]) + '\n')
(...skipping 97 matching lines...) Expand 10 before | Expand all | Expand 10 after
1153 else: 1213 else:
1154 print "Skipping file %s" % filename 1214 print "Skipping file %s" % filename
1155 1215
1156 print "Total errors found: %d\n" % cpplint._cpplint_state.error_count 1216 print "Total errors found: %d\n" % cpplint._cpplint_state.error_count
1157 os.chdir(previous_cwd) 1217 os.chdir(previous_cwd)
1158 return 1 1218 return 1
1159 1219
1160 1220
1161 def DoPresubmitChecks(change_info, committing, may_prompt): 1221 def DoPresubmitChecks(change_info, committing, may_prompt):
1162 """Imports presubmit, then calls presubmit.DoPresubmitChecks.""" 1222 """Imports presubmit, then calls presubmit.DoPresubmitChecks."""
1163 # Need to import here to avoid circular dependency.
1164 import presubmit_support
1165 root_presubmit = GetCachedFile('PRESUBMIT.py', use_root=True) 1223 root_presubmit = GetCachedFile('PRESUBMIT.py', use_root=True)
1166 change = presubmit_support.SvnChange(change_info.name, 1224 change = presubmit_support.SvnChange(change_info.name,
1167 change_info.description, 1225 change_info.description,
1168 change_info.GetLocalRoot(), 1226 change_info.GetLocalRoot(),
1169 change_info.GetFiles(), 1227 change_info.GetFiles(),
1170 change_info.issue, 1228 change_info.issue,
1171 change_info.patchset) 1229 change_info.patchset)
1172 output = presubmit_support.DoPresubmitChecks(change=change, 1230 output = presubmit_support.DoPresubmitChecks(change=change,
1173 committing=committing, 1231 committing=committing,
1174 verbose=False, 1232 verbose=False,
1175 output_stream=sys.stdout, 1233 output_stream=sys.stdout,
1176 input_stream=sys.stdin, 1234 input_stream=sys.stdin,
1177 default_presubmit=root_presubmit, 1235 default_presubmit=root_presubmit,
1178 may_prompt=may_prompt) 1236 may_prompt=may_prompt,
1237 tbr=False,
1238 host_url=change_info.rietveld)
1179 if not output.should_continue() and may_prompt: 1239 if not output.should_continue() and may_prompt:
1180 # TODO(dpranke): move into DoPresubmitChecks(), unify cmd line args. 1240 # TODO(dpranke): move into DoPresubmitChecks(), unify cmd line args.
1181 print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)" 1241 print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)"
1182 1242
1183 # TODO(dpranke): Return the output object and make use of it. 1243 return output
1184 return output.should_continue()
1185 1244
1186 1245
1187 @no_args 1246 @no_args
1188 def CMDchanges(): 1247 def CMDchanges():
1189 """Lists all the changelists and their files.""" 1248 """Lists all the changelists and their files."""
1190 for cl in GetCLs(): 1249 for cl in GetCLs():
1191 change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True) 1250 change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
1192 print "\n--- Changelist " + change_info.name + ":" 1251 print "\n--- Changelist " + change_info.name + ":"
1193 for filename in change_info.GetFiles(): 1252 for filename in change_info.GetFiles():
1194 print "".join(filename) 1253 print "".join(filename)
(...skipping 203 matching lines...) Expand 10 before | Expand all | Expand 10 after
1398 if e.code != 500: 1457 if e.code != 500:
1399 raise 1458 raise
1400 print >> sys.stderr, ( 1459 print >> sys.stderr, (
1401 'AppEngine is misbehaving and returned HTTP %d, again. Keep faith ' 1460 'AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
1402 'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)) 1461 'and retry or visit go/isgaeup.\n%s') % (e.code, str(e))
1403 return 1 1462 return 1
1404 1463
1405 1464
1406 if __name__ == "__main__": 1465 if __name__ == "__main__":
1407 sys.exit(main(sys.argv[1:])) 1466 sys.exit(main(sys.argv[1:]))
OLDNEW
« no previous file with comments | « no previous file | tests/gcl_unittest.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698