diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index a53fab56be..0fd69e6679 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -740,8 +740,6 @@ def CheckLongLines(input_api, output_api, maxlen, source_file_filter=None): return [] -_BYPASS_CHECK_LICENSE_FOOTER = 'Bypass-Check-License' - def CheckLicense(input_api, output_api, license_re_param=None, @@ -749,20 +747,6 @@ def CheckLicense(input_api, source_file_filter=None, accept_empty_files=True): """Verifies the license header.""" - # The CL is ignoring the license check - reasons = input_api.change.GitFootersFromDescription().get( - _BYPASS_CHECK_LICENSE_FOOTER, []) - - if len(reasons): - if ''.join(reasons).strip() == '': - return [ - output_api.PresubmitError( - '{key} is specified without the reason. Please provide the reason ' - 'in "{key}: "'.format( - key=_BYPASS_CHECK_LICENSE_FOOTER)) - ] - input_api.logging.info('License check is being ignored') - return [] # Early-out if the license_re is guaranteed to match everything. if license_re_param and license_re_param == '.*': @@ -843,9 +827,11 @@ def CheckLicense(input_api, bad_files.append(f.LocalPath()) results = [] if bad_new_files: + # We can't distinguish between Google and thirty-party files, so this has to be a + # warning rather than an error. if license_re_param: - error_message = ('License on new files must match:\n\n%s\n' % - license_re_param) + warning_message = ('License on new files must match:\n\n%s\n' % + license_re_param) else: # Verbatim text that can be copy-pasted into new files (possibly # adjusting the leading comment delimiter). @@ -857,28 +843,28 @@ def CheckLicense(input_api, 'project': project_name, 'key_line': key_line, } - error_message = ( + warning_message = ( 'License on new files must be:\n\n%s\n' % new_license_text + '(adjusting the comment delimiter accordingly).\n\n' + 'If this is a moved file, then update the license but do not ' + 'update the year.\n\n' + - 'If this is a third-party file, then add a footer ' + - 'with "{key}: " to skip this check.'.format( - key=_BYPASS_CHECK_LICENSE_FOOTER)) - error_message += 'Found a bad license header in these new or moved files:' + 'If this is a third-party file then ignore this warning.\n\n') + warning_message += 'Found a bad license header in these new or moved files:' results.append( - output_api.PresubmitError(error_message, items=bad_new_files)) + output_api.PresubmitPromptWarning(warning_message, + items=bad_new_files)) if wrong_year_new_files: + # We can't distinguish between new and moved files, so this has to be a + # warning rather than an error. results.append( - output_api.PresubmitError( + output_api.PresubmitPromptWarning( 'License doesn\'t list the current year. If this is a new file, ' - 'use the current year. If this is a moved file, then add ' - 'a footer with "{key}: " to skip this check.'.format( - key=_BYPASS_CHECK_LICENSE_FOOTER), + 'use the current year. If this is a moved file then ignore this ' + 'warning.', items=wrong_year_new_files)) if bad_files: results.append( - output_api.PresubmitError( + output_api.PresubmitPromptWarning( 'License must match:\n%s\n' % license_re.pattern + 'Found a bad license header in these files:', items=bad_files)) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 05e0b6bcd5..6359031721 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2689,17 +2689,9 @@ the current line as well! committing, expected_result, new_file=False, - description='this is description', **kwargs): - change = presubmit.Change( - 'author', - description, - '/path/root', - None, - 0, - 0, - None, - ) + change = mock.MagicMock(presubmit.GitChange) + change.scm = 'svn' input_api = self.MockInputApi(change, committing) affected_file = mock.MagicMock(presubmit.GitAffectedFile) if new_file: @@ -2747,41 +2739,7 @@ the current line as well! license_text = (r".*? Copyright \(c\) 0007 Nobody.\n" r".*? All Rights Reserved\.\n") self._LicenseCheck(text, license_text, True, - presubmit.OutputApi.PresubmitError) - - def testBypassCheckLicense(self): - text = '\n'.join([ - '#!/bin/python', - '# this is not a CopyRight.', - '# No Rights Reserved.', - ]) - description = '\n'.join([ - 'this is a change', - '', - '', - 'Bypass-Check-License: a third-party', - ]) - self._LicenseCheck(text, None, True, None, description=description) - - def testBypassCheckLicenseWithoutReason(self): - text = '\n'.join([ - '#!/bin/python', - '# this is not a CopyRight.', - '# No Rights Reserved.', - ]) - description = '\n'.join([ - 'this is a change', - '', - '', - 'Bypass-Check-License: ', - ]) - self._LicenseCheck( - text, - None, - True, - # must error out that should be given. - presubmit.OutputApi.PresubmitError, - description=description) + presubmit.OutputApi.PresubmitPromptWarning) def testCheckLicenseFailUpload(self): text = ("#!/bin/python\n" @@ -2791,7 +2749,7 @@ the current line as well! license_text = (r".*? Copyright \(c\) 0007 Nobody.\n" r".*? All Rights Reserved\.\n") self._LicenseCheck(text, license_text, False, - presubmit.OutputApi.PresubmitError) + presubmit.OutputApi.PresubmitPromptWarning) def testCheckLicenseEmptySuccess(self): text = '' @@ -2822,7 +2780,7 @@ the current line as well! self._LicenseCheck(text, license_text, False, - presubmit.OutputApi.PresubmitError, + presubmit.OutputApi.PresubmitPromptWarning, new_file=True) def _GetLicenseText(self, current_year): @@ -2834,15 +2792,15 @@ the current line as well! "# found in the LICENSE file.\n" "print('foo')\n" % current_year) - def testCheckLicenseNewFileError(self): - # Check that we error on new files with wrong year. Test with first + def testCheckLicenseNewFileWarn(self): + # Check that we warn on new files with wrong year. Test with first # allowed year. text = self._GetLicenseText(2006) license_text = None self._LicenseCheck(text, license_text, False, - presubmit.OutputApi.PresubmitError, + presubmit.OutputApi.PresubmitPromptWarning, new_file=True) def testCheckLicenseNewCSSFilePass(self):