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

Side by Side Diff: PRESUBMIT.py

Issue 11779037: Warn when committing code with http:// URLs. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add back the blacklist to reduce false positives due to tests. Created 7 years, 11 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 | no next file » | 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 """Top-level presubmit script for Chromium. 5 """Top-level presubmit script for Chromium.
6 6
7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts 7 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
8 for more details about the presubmit API built into gcl. 8 for more details about the presubmit API built into gcl.
9 """ 9 """
10 10
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
50 ) 50 )
51 51
52 _TEST_ONLY_WARNING = ( 52 _TEST_ONLY_WARNING = (
53 'You might be calling functions intended only for testing from\n' 53 'You might be calling functions intended only for testing from\n'
54 'production code. It is OK to ignore this warning if you know what\n' 54 'production code. It is OK to ignore this warning if you know what\n'
55 'you are doing, as the heuristics used to detect the situation are\n' 55 'you are doing, as the heuristics used to detect the situation are\n'
56 'not perfect. The commit queue will not block on this warning.\n' 56 'not perfect. The commit queue will not block on this warning.\n'
57 'Email joi@chromium.org if you have questions.') 57 'Email joi@chromium.org if you have questions.')
58 58
59 59
60 _HTTPS_ONLY_WARNING = (
61 'You should prefer to refer to HTTPS URLs, rather than HTTP URLs.\n'
62 'Do not bypass this warning if there is an equivalent HTTPS endpoint\n'
63 'that you could refer to instead. (Contact: palmer@chromium.org)')
64
65
60 _INCLUDE_ORDER_WARNING = ( 66 _INCLUDE_ORDER_WARNING = (
61 'Your #include order seems to be broken. Send mail to\n' 67 'Your #include order seems to be broken. Send mail to\n'
62 'marja@chromium.org if this is not the case.') 68 'marja@chromium.org if this is not the case.')
63 69
64 70
65 _BANNED_OBJC_FUNCTIONS = ( 71 _BANNED_OBJC_FUNCTIONS = (
66 ( 72 (
67 'addTrackingRect:', 73 'addTrackingRect:',
68 ( 74 (
69 'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is' 75 'The use of -[NSView addTrackingRect:owner:userData:assumeInside:] is'
(...skipping 117 matching lines...) Expand 10 before | Expand all | Expand 10 after
187 _TEST_CODE_EXCLUDED_PATHS + 193 _TEST_CODE_EXCLUDED_PATHS +
188 input_api.DEFAULT_BLACK_LIST) 194 input_api.DEFAULT_BLACK_LIST)
189 return input_api.FilterSourceFile( 195 return input_api.FilterSourceFile(
190 affected_file, 196 affected_file,
191 white_list=(file_inclusion_pattern, ), 197 white_list=(file_inclusion_pattern, ),
192 black_list=black_list) 198 black_list=black_list)
193 199
194 problems = [] 200 problems = []
195 for f in input_api.AffectedSourceFiles(FilterFile): 201 for f in input_api.AffectedSourceFiles(FilterFile):
196 local_path = f.LocalPath() 202 local_path = f.LocalPath()
197 lines = input_api.ReadFile(f).splitlines() 203 for line_number, line in enumerate(input_api.ReadFile(f).splitlines()):
198 line_number = 0 204 if inclusion_pattern.search(line) and not exclusion_pattern.search(line):
199 for line in lines:
200 if (inclusion_pattern.search(line) and
201 not exclusion_pattern.search(line)):
202 problems.append( 205 problems.append(
203 '%s:%d\n %s' % (local_path, line_number, line.strip())) 206 '%s:%d\n %s' % (local_path, line_number + 1, line.strip()))
204 line_number += 1
205 207
206 if problems: 208 if problems:
207 if not input_api.is_committing: 209 if input_api.is_committing:
208 return [output_api.PresubmitPromptWarning(_TEST_ONLY_WARNING, problems)]
209 else:
210 # We don't warn on commit, to avoid stopping commits going through CQ. 210 # We don't warn on commit, to avoid stopping commits going through CQ.
211 return [output_api.PresubmitNotifyResult(_TEST_ONLY_WARNING, problems)] 211 return [output_api.PresubmitNotifyResult(_TEST_ONLY_WARNING, problems)]
212 else: 212 else:
213 return [] 213 return [output_api.PresubmitPromptWarning(_TEST_ONLY_WARNING, problems)]
214
215 return []
216
217
218 def _CheckNoHttpUrls(input_api, output_api):
219 """Attempts to prevent use of http:// URLs. Production Chrome code, and
220 Chromium infrastructure code, should strive to use https:// URLs (preferably
221 HSTS and with pinned public keys, as well). Some URLs will not be
222 translatable, but gradually the number of those should diminish.
223 """
224
225 # Include all sorts of files, both code and infrastructure scripts.
226 file_inclusion_pattern = r'.+\.(h|cc|cpp|cxx|mm|py|bat|sh)$'
227
228 inclusion_pattern = input_api.re.compile(r'''["']http:/''',
229 input_api.re.IGNORECASE)
230
231 # Allow http:/ in comments. https://chromium.org and
232 # https://cr{bug,osbug,rev}.com don't work yet. See e.g.
233 # https://code.google.com/p/chromium/issues/detail?id=106096.
234 # TODO(palmer): Remove this once 106096 and its dependent bugs are fixed.
235 exclusion_pattern = input_api.re.compile(r'(#|//|/\*| \*).*http:/',
236 input_api.re.IGNORECASE)
237
238 def FilterFile(affected_file):
239 black_list = (_TEST_CODE_EXCLUDED_PATHS +
240 input_api.DEFAULT_BLACK_LIST)
241 return input_api.FilterSourceFile(
242 affected_file,
243 white_list=(file_inclusion_pattern, ),
244 black_list=black_list)
245
246 problems = []
247 for f in input_api.AffectedSourceFiles(FilterFile):
248 local_path = f.LocalPath()
249 for line_number, line in enumerate(input_api.ReadFile(f).splitlines()):
250 if inclusion_pattern.search(line) and not exclusion_pattern.search(line):
251 problems.append(
252 '%s:%d\n %s' % (local_path, line_number + 1, line.strip()))
253
254 if problems:
255 if input_api.is_committing:
256 # We don't warn on commit, to avoid stopping commits going through CQ.
257 return [output_api.PresubmitNotifyResult(_HTTPS_ONLY_WARNING, problems)]
258 else:
259 return [output_api.PresubmitPromptWarning(_HTTPS_ONLY_WARNING, problems)]
260
261 return []
214 262
215 263
216 def _CheckNoIOStreamInHeaders(input_api, output_api): 264 def _CheckNoIOStreamInHeaders(input_api, output_api):
217 """Checks to make sure no .h files include <iostream>.""" 265 """Checks to make sure no .h files include <iostream>."""
218 files = [] 266 files = []
219 pattern = input_api.re.compile(r'^#include\s*<iostream>', 267 pattern = input_api.re.compile(r'^#include\s*<iostream>',
220 input_api.re.MULTILINE) 268 input_api.re.MULTILINE)
221 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile): 269 for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
222 if not f.LocalPath().endswith('.h'): 270 if not f.LocalPath().endswith('.h'):
223 continue 271 continue
(...skipping 190 matching lines...) Expand 10 before | Expand all | Expand 10 after
414 error_descriptions.append(description_with_path) 462 error_descriptions.append(description_with_path)
415 else: 463 else:
416 warning_descriptions.append(description_with_path) 464 warning_descriptions.append(description_with_path)
417 465
418 results = [] 466 results = []
419 if error_descriptions: 467 if error_descriptions:
420 results.append(output_api.PresubmitError( 468 results.append(output_api.PresubmitError(
421 'You added one or more #includes that violate checkdeps rules.', 469 'You added one or more #includes that violate checkdeps rules.',
422 error_descriptions)) 470 error_descriptions))
423 if warning_descriptions: 471 if warning_descriptions:
424 if not input_api.is_committing: 472 if input_api.is_committing:
425 warning_factory = output_api.PresubmitPromptWarning
426 else:
427 # We don't want to block use of the CQ when there is a warning 473 # We don't want to block use of the CQ when there is a warning
428 # of this kind, so we only show a message when committing. 474 # of this kind, so we only show a message when committing.
429 warning_factory = output_api.PresubmitNotifyResult 475 warning_factory = output_api.PresubmitNotifyResult
476 else:
477 warning_factory = output_api.PresubmitPromptWarning
430 results.append(warning_factory( 478 results.append(warning_factory(
431 'You added one or more #includes of files that are temporarily\n' 479 'You added one or more #includes of files that are temporarily\n'
432 'allowed but being removed. Can you avoid introducing the\n' 480 'allowed but being removed. Can you avoid introducing the\n'
433 '#include? See relevant DEPS file(s) for details and contacts.', 481 '#include? See relevant DEPS file(s) for details and contacts.',
434 warning_descriptions)) 482 warning_descriptions))
435 return results 483 return results
436 484
437 485
438 def _CheckFilePermissions(input_api, output_api): 486 def _CheckFilePermissions(input_api, output_api):
439 """Check that all files have their permissions properly set.""" 487 """Check that all files have their permissions properly set."""
(...skipping 157 matching lines...) Expand 10 before | Expand all | Expand 10 after
597 """ 645 """
598 646
599 warnings = [] 647 warnings = []
600 for f in input_api.AffectedFiles(): 648 for f in input_api.AffectedFiles():
601 if f.LocalPath().endswith(('.cc', '.h')): 649 if f.LocalPath().endswith(('.cc', '.h')):
602 changed_linenums = set(line_num for line_num, _ in f.ChangedContents()) 650 changed_linenums = set(line_num for line_num, _ in f.ChangedContents())
603 warnings.extend(_CheckIncludeOrderInFile(input_api, f, changed_linenums)) 651 warnings.extend(_CheckIncludeOrderInFile(input_api, f, changed_linenums))
604 652
605 results = [] 653 results = []
606 if warnings: 654 if warnings:
607 if not input_api.is_committing: 655 if input_api.is_committing:
608 results.append(output_api.PresubmitPromptWarning(_INCLUDE_ORDER_WARNING,
609 warnings))
610 else:
611 # We don't warn on commit, to avoid stopping commits going through CQ. 656 # We don't warn on commit, to avoid stopping commits going through CQ.
612 results.append(output_api.PresubmitNotifyResult(_INCLUDE_ORDER_WARNING, 657 results.append(output_api.PresubmitNotifyResult(_INCLUDE_ORDER_WARNING,
613 warnings)) 658 warnings))
659 else:
660 results.append(output_api.PresubmitPromptWarning(_INCLUDE_ORDER_WARNING,
661 warnings))
614 return results 662 return results
615 663
616 664
617 def _CheckForVersionControlConflictsInFile(input_api, f): 665 def _CheckForVersionControlConflictsInFile(input_api, f):
618 pattern = input_api.re.compile('^(?:<<<<<<<|>>>>>>>) |^=======$') 666 pattern = input_api.re.compile('^(?:<<<<<<<|>>>>>>>) |^=======$')
619 errors = [] 667 errors = []
620 for line_num, line in f.ChangedContents(): 668 for line_num, line in f.ChangedContents():
621 if pattern.match(line): 669 if pattern.match(line):
622 errors.append(' %s:%d %s' % (f.LocalPath(), line_num, line)) 670 errors.append(' %s:%d %s' % (f.LocalPath(), line_num, line))
623 return errors 671 return errors
(...skipping 27 matching lines...) Expand all
651 input_api.DEFAULT_BLACK_LIST)) 699 input_api.DEFAULT_BLACK_LIST))
652 700
653 pattern = input_api.re.compile('"[^"]*google\.com[^"]*"') 701 pattern = input_api.re.compile('"[^"]*google\.com[^"]*"')
654 problems = [] # items are (filename, line_number, line) 702 problems = [] # items are (filename, line_number, line)
655 for f in input_api.AffectedSourceFiles(FilterFile): 703 for f in input_api.AffectedSourceFiles(FilterFile):
656 for line_num, line in f.ChangedContents(): 704 for line_num, line in f.ChangedContents():
657 if pattern.search(line): 705 if pattern.search(line):
658 problems.append((f.LocalPath(), line_num, line)) 706 problems.append((f.LocalPath(), line_num, line))
659 707
660 if problems: 708 if problems:
661 if not input_api.is_committing: 709 if input_api.is_committing:
662 warning_factory = output_api.PresubmitPromptWarning
663 else:
664 # We don't want to block use of the CQ when there is a warning 710 # We don't want to block use of the CQ when there is a warning
665 # of this kind, so we only show a message when committing. 711 # of this kind, so we only show a message when committing.
666 warning_factory = output_api.PresubmitNotifyResult 712 warning_factory = output_api.PresubmitNotifyResult
713 else:
714 warning_factory = output_api.PresubmitPromptWarning
667 return [warning_factory( 715 return [warning_factory(
668 'Most layers below src/chrome/ should not hardcode service URLs.\n' 716 'Most layers below src/chrome/ should not hardcode service URLs.\n'
669 'Are you sure this is correct? (Contact: joi@chromium.org)', 717 'Are you sure this is correct? (Contact: joi@chromium.org)',
670 [' %s:%d: %s' % ( 718 [' %s:%d: %s' % (
671 problem[0], problem[1], problem[2]) for problem in problems])] 719 problem[0], problem[1], problem[2]) for problem in problems])]
672 else: 720
673 return [] 721 return []
674 722
675 723
676 def _CommonChecks(input_api, output_api): 724 def _CommonChecks(input_api, output_api):
677 """Checks common to both upload and commit.""" 725 """Checks common to both upload and commit."""
678 results = [] 726 results = []
679 results.extend(input_api.canned_checks.PanProjectChecks( 727 results.extend(input_api.canned_checks.PanProjectChecks(
680 input_api, output_api, excluded_paths=_EXCLUDED_PATHS)) 728 input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
681 results.extend(_CheckAuthorizedAuthor(input_api, output_api)) 729 results.extend(_CheckAuthorizedAuthor(input_api, output_api))
682 results.extend( 730 results.extend(
683 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api)) 731 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
684 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api)) 732 results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
685 results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api)) 733 results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
686 results.extend(_CheckNoNewWStrings(input_api, output_api)) 734 results.extend(_CheckNoNewWStrings(input_api, output_api))
687 results.extend(_CheckNoDEPSGIT(input_api, output_api)) 735 results.extend(_CheckNoDEPSGIT(input_api, output_api))
688 results.extend(_CheckNoBannedFunctions(input_api, output_api)) 736 results.extend(_CheckNoBannedFunctions(input_api, output_api))
689 results.extend(_CheckNoPragmaOnce(input_api, output_api)) 737 results.extend(_CheckNoPragmaOnce(input_api, output_api))
690 results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api)) 738 results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
691 results.extend(_CheckUnwantedDependencies(input_api, output_api)) 739 results.extend(_CheckUnwantedDependencies(input_api, output_api))
692 results.extend(_CheckFilePermissions(input_api, output_api)) 740 results.extend(_CheckFilePermissions(input_api, output_api))
693 results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api)) 741 results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api))
694 results.extend(_CheckIncludeOrder(input_api, output_api)) 742 results.extend(_CheckIncludeOrder(input_api, output_api))
695 results.extend(_CheckForVersionControlConflicts(input_api, output_api)) 743 results.extend(_CheckForVersionControlConflicts(input_api, output_api))
696 results.extend(_CheckPatchFiles(input_api, output_api)) 744 results.extend(_CheckPatchFiles(input_api, output_api))
697 results.extend(_CheckHardcodedGoogleHostsInLowerLayers(input_api, output_api)) 745 results.extend(_CheckHardcodedGoogleHostsInLowerLayers(input_api, output_api))
746 results.extend(_CheckNoHttpUrls(input_api, output_api))
698 747
699 if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()): 748 if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
700 results.extend(input_api.canned_checks.RunUnitTestsInDirectory( 749 results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
701 input_api, output_api, 750 input_api, output_api,
702 input_api.PresubmitLocalPath(), 751 input_api.PresubmitLocalPath(),
703 whitelist=[r'.+_test\.py$'])) 752 whitelist=[r'.+_test\.py$']))
704 return results 753 return results
705 754
706 755
707 def _CheckSubversionConfig(input_api, output_api): 756 def _CheckSubversionConfig(input_api, output_api):
(...skipping 68 matching lines...) Expand 10 before | Expand all | Expand 10 after
776 author)] 825 author)]
777 return [] 826 return []
778 827
779 828
780 def _CheckPatchFiles(input_api, output_api): 829 def _CheckPatchFiles(input_api, output_api):
781 problems = [f.LocalPath() for f in input_api.AffectedFiles() 830 problems = [f.LocalPath() for f in input_api.AffectedFiles()
782 if f.LocalPath().endswith(('.orig', '.rej'))] 831 if f.LocalPath().endswith(('.orig', '.rej'))]
783 if problems: 832 if problems:
784 return [output_api.PresubmitError( 833 return [output_api.PresubmitError(
785 "Don't commit .rej and .orig files.", problems)] 834 "Don't commit .rej and .orig files.", problems)]
786 else: 835
787 return [] 836 return []
788 837
789 838
790 def CheckChangeOnUpload(input_api, output_api): 839 def CheckChangeOnUpload(input_api, output_api):
791 results = [] 840 results = []
792 results.extend(_CommonChecks(input_api, output_api)) 841 results.extend(_CommonChecks(input_api, output_api))
793 return results 842 return results
794 843
795 844
796 def CheckChangeOnCommit(input_api, output_api): 845 def CheckChangeOnCommit(input_api, output_api):
797 results = [] 846 results = []
798 results.extend(_CommonChecks(input_api, output_api)) 847 results.extend(_CommonChecks(input_api, output_api))
799 # TODO(thestig) temporarily disabled, doesn't work in third_party/ 848 # TODO(thestig) temporarily disabled, doesn't work in third_party/
800 #results.extend(input_api.canned_checks.CheckSvnModifiedDirectories( 849 #results.extend(input_api.canned_checks.CheckSvnModifiedDirectories(
801 # input_api, output_api, sources)) 850 # input_api, output_api, sources))
802 # Make sure the tree is 'open'. 851 # Make sure the tree is 'open'.
803 results.extend(input_api.canned_checks.CheckTreeIsOpen( 852 results.extend(input_api.canned_checks.CheckTreeIsOpen(
804 input_api, 853 input_api,
805 output_api, 854 output_api,
806 json_url='http://chromium-status.appspot.com/current?format=json')) 855 json_url='https://chromium-status.appspot.com/current?format=json'))
807 results.extend(input_api.canned_checks.CheckRietveldTryJobExecution(input_api, 856 results.extend(input_api.canned_checks.CheckRietveldTryJobExecution(input_api,
808 output_api, 'http://codereview.chromium.org', 857 output_api, 'https://codereview.chromium.org',
809 ('win_rel', 'linux_rel', 'mac_rel, win:compile'), 858 ('win_rel', 'linux_rel', 'mac_rel, win:compile'),
810 'tryserver@chromium.org')) 859 'tryserver@chromium.org'))
811 860
812 results.extend(input_api.canned_checks.CheckChangeHasBugField( 861 results.extend(input_api.canned_checks.CheckChangeHasBugField(
813 input_api, output_api)) 862 input_api, output_api))
814 results.extend(input_api.canned_checks.CheckChangeHasDescription( 863 results.extend(input_api.canned_checks.CheckChangeHasDescription(
815 input_api, output_api)) 864 input_api, output_api))
816 results.extend(_CheckSubversionConfig(input_api, output_api)) 865 results.extend(_CheckSubversionConfig(input_api, output_api))
817 return results 866 return results
818 867
(...skipping 30 matching lines...) Expand all
849 'win7_aura', 898 'win7_aura',
850 'win_rel', 899 'win_rel',
851 ] 900 ]
852 901
853 # Match things like path/aura/file.cc and path/file_aura.cc. 902 # Match things like path/aura/file.cc and path/file_aura.cc.
854 # Same for chromeos. 903 # Same for chromeos.
855 if any(re.search('[/_](aura|chromeos)', f) for f in files): 904 if any(re.search('[/_](aura|chromeos)', f) for f in files):
856 trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan'] 905 trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan']
857 906
858 return trybots 907 return trybots
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698