aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

presubmit: emit errors instead of warnings for bad copyright headers

Browse Source 
This is an effective revert of https://crrev.com/c/4895337 with
additional patches to support a footer.

https://crrev.com/c/3887721 updated CheckLicense() to emit errors
for bad copyright headers. However, https://crrev.com/c/4895337
was changed the finding type from error to warning, claiming that
the check is N/A for moved third files, but it's not so easy
to programatiically distinguish moved third-party files.

After discussions, it was decided to change the finding type back
to error to prevent accidental submissions for new files without
correct CopyRight headers.

To mitigate moved, third-party files, this CL adds support for
"Bypass-Check-License: <reason>" footer.

If the check should be ignored in a given CL, CL authors should
use the footer instead.

Bug: 435696543,40237859
Change-Id: I177915c65932a3d76ea60ee6a0e396f726bc400d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6842238
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
changes/38/6842238/8
Scott Lee 3 months ago committed by LUCI CQ
parent 1f33a15831
commit fa62515ecb
2 changed files with 79 additions and 23 deletions
Show Stats Download Patch File Download Diff File

44
presubmit_canned_checks.py
Unescape Escape View File

@ -740,6 +740,8 @@ 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,
@ -747,6 +749,20 @@ 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}: <reason>"'.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 == '.*':
@ -827,11 +843,9 @@ 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:
warning_message = ('License on new files must match:\n\n%s\n' %
license_re_param)
error_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).
@ -843,28 +857,28 @@ def CheckLicense(input_api,
'project': project_name,
'key_line': key_line,
}
warning_message = (
error_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 ignore this warning.\n\n')
warning_message += 'Found a bad license header in these new or moved files:'
'If this is a third-party file, then add a footer ' +
'with "{key}: <reason>" to skip this check.'.format(
key=_BYPASS_CHECK_LICENSE_FOOTER))
error_message += 'Found a bad license header in these new or moved files:'
results.append(
output_api.PresubmitPromptWarning(warning_message,
items=bad_new_files))
output_api.PresubmitError(error_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.PresubmitPromptWarning(
output_api.PresubmitError(
'License doesn\'t list the current year. If this is a new file, '
'use the current year. If this is a moved file then ignore this '
'warning.',
'use the current year. If this is a moved file, then add '
'a footer with "{key}: <reason>" to skip this check.'.format(
key=_BYPASS_CHECK_LICENSE_FOOTER),
items=wrong_year_new_files))
if bad_files:
results.append(
output_api.PresubmitPromptWarning(
output_api.PresubmitError(
'License must match:\n%s\n' % license_re.pattern +
'Found a bad license header in these files:',
items=bad_files))

58
tests/presubmit_unittest.py
Unescape Escape View File

@ -2689,9 +2689,17 @@ the current line as well!
committing,
expected_result,
new_file=False,
description='this is description',
**kwargs):
change = mock.MagicMock(presubmit.GitChange)
change.scm = 'svn'
change = presubmit.Change(
'author',
description,
'/path/root',
None,
0,
0,
None,
)
input_api = self.MockInputApi(change, committing)
affected_file = mock.MagicMock(presubmit.GitAffectedFile)
if new_file:
@ -2739,7 +2747,41 @@ 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.PresubmitPromptWarning)
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 <reason> should be given.
presubmit.OutputApi.PresubmitError,
description=description)
def testCheckLicenseFailUpload(self):
text = ("#!/bin/python\n"
@ -2749,7 +2791,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.PresubmitPromptWarning)
presubmit.OutputApi.PresubmitError)
def testCheckLicenseEmptySuccess(self):
text = ''
@ -2780,7 +2822,7 @@ the current line as well!
self._LicenseCheck(text,
license_text,
False,
presubmit.OutputApi.PresubmitPromptWarning,
presubmit.OutputApi.PresubmitError,
new_file=True)
def _GetLicenseText(self, current_year):
@ -2792,15 +2834,15 @@ the current line as well!
"# found in the LICENSE file.\n"
"print('foo')\n" % current_year)
def testCheckLicenseNewFileWarn(self):
# Check that we warn on new files with wrong year. Test with first
def testCheckLicenseNewFileError(self):
# Check that we error 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.PresubmitPromptWarning,
presubmit.OutputApi.PresubmitError,
new_file=True)
def testCheckLicenseNewCSSFilePass(self):

Write Preview
Loading…
Cancel
Save