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

Revert "presubmit: emit errors instead of warnings for bad copyright headers"

Browse Source 
This reverts commit fa62515ecb.

Reason for revert: it seems that the existing files without valid copyright headers are causing linux-presubmit builds to fail. b/438791294 

Bug: 435696543,40237859
Original change's description:
> presubmit: emit errors instead of warnings for bad copyright headers
>
> 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>

Bug: 435696543,40237859
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Ibedf8d13e3742249947e29e625a14cceaf89879c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6852377
Commit-Queue: Scott Lee <ddoman@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
changes/77/6852377/2
Scott Lee 3 months ago committed by LUCI CQ
parent 526aace177
commit eb60ab38de
2 changed files with 23 additions and 79 deletions
Show Stats Download Patch File Download Diff File

44
presubmit_canned_checks.py
Unescape Escape View File

@ -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}: <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 == '.*':
@ -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}: <reason>" 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}: <reason>" 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))

58
tests/presubmit_unittest.py
Unescape Escape View File

@ -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 <reason> 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):

Write Preview
Loading…
Cancel
Save