InTheForest/chromium_depot_tools
1
0
Fork 0
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools.git synced 2026-01-11 18:51:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
This reverts commit eb60ab38de.

Reason for revert: re-landing with an additional patch

-------------
* Problem
Browser infra runs ci.*-presubmit builders, such as linux-presubmit,
with --all to ensure that the entire chromium/src passes
presubmit checks.

crrev.com/c/6842238 changed the finding type for License Check
from warning to error, but the CI presubmit builders failed because
there are many files without valid CopyRight.

Not only the existing files, all the new files that are added with
`Bypass-Check-License: <reason> footer could also cause the presubmit
builder to fail.

* This CL

In addition to the original patch from crrev.com/c/6842238,
this CL makes additional patches to turn the CopyRight errors into
warnings, if --file or --all is given.

The same approach is used in
https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_canned_checks.py;l=982-987;drc=eb60ab38deeda6975c9b0fef883978f2a9f69120


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

Bug: 435696543,40237859
Change-Id: Iafdb29b928c016eb3949e29fd43a2ba5f53e0ba0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6852108
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
This commit is contained in:
Scott Lee
2025-08-15 10:47:31 -07:00
committed by LUCI CQ
parent 407c50aa50
commit f88b25b610
2 changed files with 105 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
tests/presubmit_unittest.py
View File

@@ -2689,13 +2689,24 @@ the current line as well!
committing,
expected_result,
new_file=False,
description='this is description',
no_diffs=False,
**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)
input_api.no_diffs = no_diffs
if new_file:
affected_file.Action.return_value = 'A'
input_api.AffectedSourceFiles.return_value = [affected_file]
input_api.ReadFile.return_value = text
if expected_result:
@@ -2739,7 +2750,54 @@ 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 testCheckLicenseWarnCommitIfNoDiffs(self):
text = ("#!/bin/python\n"
"# Copyright (c) 2037 Nobody.\n"
"# All Rights Reserved.\n"
"print('foo')\n")
license_text = (r".*? Copyright \(c\) 0007 Nobody.\n"
r".*? All Rights Reserved\.\n")
self._LicenseCheck(text,
license_text,
True,
presubmit.OutputApi.PresubmitPromptWarning,
no_diffs=True)
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 +2807,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 +2838,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 +2850,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):