OLD | NEW |
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 Loading... |
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 Loading... |
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 |
OLD | NEW |