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

Side by Side Diff: presubmit_canned_checks.py

Issue 1927773002: Reland Implement owners check in presubmit for Gerrit. (Closed) Base URL: https://chromium.googlesource.com/chromium/tools/depot_tools.git@P300
Patch Set: gerrit fix Created 4 years, 7 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
« no previous file with comments | « git_cl.py ('k') | presubmit_support.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 # Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 # Use of this source code is governed by a BSD-style license that can be 2 # Use of this source code is governed by a BSD-style license that can be
3 # found in the LICENSE file. 3 # found in the LICENSE file.
4 4
5 """Generic presubmit checks that can be reused by other presubmit checks.""" 5 """Generic presubmit checks that can be reused by other presubmit checks."""
6 6
7 import os as _os 7 import os as _os
8 _HERE = _os.path.dirname(_os.path.abspath(__file__)) 8 _HERE = _os.path.dirname(_os.path.abspath(__file__))
9 9
10 # Justifications for each filter: 10 # Justifications for each filter:
(...skipping 857 matching lines...) Expand 10 before | Expand all | Expand 10 after
868 needed = 'LGTM from an OWNER' 868 needed = 'LGTM from an OWNER'
869 output = output_api.PresubmitError 869 output = output_api.PresubmitError
870 else: 870 else:
871 needed = 'OWNER reviewers' 871 needed = 'OWNER reviewers'
872 output = output_api.PresubmitNotifyResult 872 output = output_api.PresubmitNotifyResult
873 873
874 affected_files = set([f.LocalPath() for f in 874 affected_files = set([f.LocalPath() for f in
875 input_api.change.AffectedFiles(file_filter=source_file_filter)]) 875 input_api.change.AffectedFiles(file_filter=source_file_filter)])
876 876
877 owners_db = input_api.owners_db 877 owners_db = input_api.owners_db
878 # TODO(tandrii): this will always return None, set() in case of Gerrit. 878 owner_email, reviewers = _CodereviewOwnersAndReviewers(
879 owner_email, reviewers = _RietveldOwnerAndReviewers(
880 input_api, 879 input_api,
881 owners_db.email_regexp, 880 owners_db.email_regexp,
882 approval_needed=input_api.is_committing) 881 approval_needed=input_api.is_committing)
883 882
884 owner_email = owner_email or input_api.change.author_email 883 owner_email = owner_email or input_api.change.author_email
885 884
886 if owner_email: 885 if owner_email:
887 reviewers_plus_owner = set([owner_email]).union(reviewers) 886 reviewers_plus_owner = set([owner_email]).union(reviewers)
888 missing_files = owners_db.files_not_covered_by(affected_files, 887 missing_files = owners_db.files_not_covered_by(affected_files,
889 reviewers_plus_owner) 888 reviewers_plus_owner)
890 else: 889 else:
891 missing_files = owners_db.files_not_covered_by(affected_files, reviewers) 890 missing_files = owners_db.files_not_covered_by(affected_files, reviewers)
892 891
893 if missing_files: 892 if missing_files:
894 output_list = [ 893 output_list = [
895 output('Missing %s for these files:\n %s' % 894 output('Missing %s for these files:\n %s' %
896 (needed, '\n '.join(sorted(missing_files))))] 895 (needed, '\n '.join(sorted(missing_files))))]
897 if not input_api.is_committing: 896 if not input_api.is_committing:
898 suggested_owners = owners_db.reviewers_for(missing_files, owner_email) 897 suggested_owners = owners_db.reviewers_for(missing_files, owner_email)
899 output_list.append(output('Suggested OWNERS: ' + 898 output_list.append(output('Suggested OWNERS: ' +
900 '(Use "git-cl owners" to interactively select owners.)\n %s' % 899 '(Use "git-cl owners" to interactively select owners.)\n %s' %
901 ('\n '.join(suggested_owners or [])))) 900 ('\n '.join(suggested_owners or []))))
902 return output_list 901 return output_list
903 902
904 if input_api.is_committing and not reviewers: 903 if input_api.is_committing and not reviewers:
905 return [output('Missing LGTM from someone other than %s' % owner_email)] 904 return [output('Missing LGTM from someone other than %s' % owner_email)]
906 return [] 905 return []
907 906
907 def _CodereviewOwnersAndReviewers(input_api, email_regexp, approval_needed):
908 """Return the owner and reviewers of a change, if any.
909
910 If approval_needed is True, only reviewers who have approved the change
911 will be returned.
912 """
913 if input_api.change.issue:
914 if input_api.gerrit:
915 res = _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed)
916 else:
917 # Rietveld is default.
918 res = _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed)
919 if res:
920 return res
921
922 reviewers = set()
923 if not approval_needed:
924 reviewers = _ReviewersFromChange(input_api.change)
925 return None, reviewers
926
908 927
909 def _GetRietveldIssueProps(input_api, messages): 928 def _GetRietveldIssueProps(input_api, messages):
910 """Gets the issue properties from rietveld.""" 929 """Gets the issue properties from rietveld."""
911 issue = input_api.change.issue 930 issue = input_api.change.issue
912 if issue and input_api.rietveld: 931 if issue and input_api.rietveld:
913 return input_api.rietveld.get_issue_properties( 932 return input_api.rietveld.get_issue_properties(
914 issue=int(issue), messages=messages) 933 issue=int(issue), messages=messages)
915 934
916 935
917 def _ReviewersFromChange(change): 936 def _ReviewersFromChange(change):
918 """Return the reviewers specified in the |change|, if any.""" 937 """Return the reviewers specified in the |change|, if any."""
919 reviewers = set() 938 reviewers = set()
920 if change.R: 939 if change.R:
921 reviewers.update(set([r.strip() for r in change.R.split(',')])) 940 reviewers.update(set([r.strip() for r in change.R.split(',')]))
922 if change.TBR: 941 if change.TBR:
923 reviewers.update(set([r.strip() for r in change.TBR.split(',')])) 942 reviewers.update(set([r.strip() for r in change.TBR.split(',')]))
924 943
925 # Drop reviewers that aren't specified in email address format. 944 # Drop reviewers that aren't specified in email address format.
926 return set(reviewer for reviewer in reviewers if '@' in reviewer) 945 return set(reviewer for reviewer in reviewers if '@' in reviewer)
927 946
928 947
948 def _match_reviewer_email(r, owner_email, email_regexp):
949 return email_regexp.match(r) and r != owner_email
950
929 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False): 951 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
930 """Return the owner and reviewers of a change, if any. 952 """Return the owner and reviewers of a change, if any.
931 953
932 If approval_needed is True, only reviewers who have approved the change 954 If approval_needed is True, only reviewers who have approved the change
933 will be returned. 955 will be returned.
956 Returns None if can't fetch issue properties from codereview.
934 """ 957 """
935 issue_props = _GetRietveldIssueProps(input_api, True) 958 issue_props = _GetRietveldIssueProps(input_api, True)
936 if not issue_props: 959 if not issue_props:
937 reviewers = set() 960 return None
938 if not approval_needed:
939 reviewers = _ReviewersFromChange(input_api.change)
940 return None, reviewers
941 961
942 if not approval_needed: 962 if not approval_needed:
943 return issue_props['owner_email'], set(issue_props['reviewers']) 963 return issue_props['owner_email'], set(issue_props['reviewers'])
944 964
945 owner_email = issue_props['owner_email'] 965 owner_email = issue_props['owner_email']
946 966
947 def match_reviewer(r):
948 return email_regexp.match(r) and r != owner_email
949
950 messages = issue_props.get('messages', []) 967 messages = issue_props.get('messages', [])
951 approvers = set( 968 approvers = set(
952 m['sender'] for m in messages 969 m['sender'] for m in messages
953 if m.get('approval') and match_reviewer(m['sender'])) 970 if m.get('approval') and _match_reviewer_email(m['sender'], owner_email,
971 email_regexp))
972 return owner_email, approvers
954 973
955 return owner_email, approvers 974
975 def _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
976 """Return the owner and reviewers of a change, if any.
977
978 If approval_needed is True, only reviewers who have approved the change
979 will be returned.
980 Returns None if can't fetch issue properties from codereview.
981 """
982 issue = input_api.change.issue
983 if not issue:
984 return None
985
986 owner_email = input_api.gerrit.GetChangeOwner(issue)
987 reviewers = set(
988 r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed)
989 if _match_reviewer_email(r, owner_email, email_regexp))
990 return owner_email, reviewers
956 991
957 992
958 def _CheckConstNSObject(input_api, output_api, source_file_filter): 993 def _CheckConstNSObject(input_api, output_api, source_file_filter):
959 """Checks to make sure no objective-c files have |const NSSomeClass*|.""" 994 """Checks to make sure no objective-c files have |const NSSomeClass*|."""
960 pattern = input_api.re.compile( 995 pattern = input_api.re.compile(
961 r'(?<!reinterpret_cast<)' 996 r'(?<!reinterpret_cast<)'
962 r'const\s+NS(?!(Point|Range|Rect|Size)\s*\*)\w*\s*\*') 997 r'const\s+NS(?!(Point|Range|Rect|Size)\s*\*)\w*\s*\*')
963 998
964 def objective_c_filter(f): 999 def objective_c_filter(f):
965 return (source_file_filter(f) and 1000 return (source_file_filter(f) and
(...skipping 162 matching lines...) Expand 10 before | Expand all | Expand 10 after
1128 for f in affected_files: 1163 for f in affected_files:
1129 cmd = ['gn', 'format', '--dry-run', f.AbsoluteLocalPath()] 1164 cmd = ['gn', 'format', '--dry-run', f.AbsoluteLocalPath()]
1130 rc = gn.main(cmd) 1165 rc = gn.main(cmd)
1131 if rc == 2: 1166 if rc == 2:
1132 warnings.append(output_api.PresubmitPromptWarning( 1167 warnings.append(output_api.PresubmitPromptWarning(
1133 '%s requires formatting. Please run `gn format --in-place %s`.' % ( 1168 '%s requires formatting. Please run `gn format --in-place %s`.' % (
1134 f.AbsoluteLocalPath(), f.LocalPath()))) 1169 f.AbsoluteLocalPath(), f.LocalPath())))
1135 # It's just a warning, so ignore other types of failures assuming they'll be 1170 # It's just a warning, so ignore other types of failures assuming they'll be
1136 # caught elsewhere. 1171 # caught elsewhere.
1137 return warnings 1172 return warnings
OLDNEW
« no previous file with comments | « git_cl.py ('k') | presubmit_support.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698