From a7949c2545d46330ac4972ab169247a244da48e9 Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Thu, 20 Oct 2022 01:33:11 +0000 Subject: [PATCH] Fix license checks In crrev.com/c/3955701 the license checks were made stricter on new files. Unfortunately moved and new files are indistinguishable in the presubmit system, and moved files are not supposed to update their copyright year. So, CheckLicense() now emits three different types of presubmit results: 1) If a file is new/moved and the license is malformed then an error is reported. 2) If a file is new/moved and the year is not current then a warning is issued, with advice to ignore the warning if the file was moved. 3) If a file is not new/moved and it has a bad license then a warning is issued. This will ensure that new files do not have bad licenses, and will usually get the year correct. The new output looks like this (for one moved file with an old date, one file with a bad license, and one new file with a bad license): ** Presubmit Warnings: 2 ** 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. base\win\moved.cc License must match: .*? Copyright (\(c\) )?(2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors(\. All rights reserved\.)?\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?: \*/)?\n Found a bad license header in these files: base\win\bad_license.cc ** Presubmit ERRORS: 1 ** License on new files must match: .*? Copyright (2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.\n Found a bad license header in these new files: base\win\new_file.cc Bug: 1098010 Change-Id: Ia62b0591ee416c55566427bba9fdd91d74a26349 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3967210 Commit-Queue: Bruce Dawson Reviewed-by: Mike Dougherty --- presubmit_canned_checks.py | 45 +++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 369e43a45..f74f1ee62 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -669,7 +669,7 @@ def CheckLicense(input_api, output_api, license_re=None, project_name=None, new_license_re = (r'.*? Copyright %(year)s The %(project)s Authors\n' r'.*? %(key_line)s\n' r'.*? found in the LICENSE file\.\n') % { - 'year': current_year, + 'year': years_re, 'project': project_name, 'key_line': key_line, } @@ -677,7 +677,8 @@ def CheckLicense(input_api, output_api, license_re=None, project_name=None, license_re = input_api.re.compile(license_re, input_api.re.MULTILINE) new_license_re = input_api.re.compile(new_license_re, input_api.re.MULTILINE) bad_files = [] - bad_new_files = False + wrong_year_new_files = [] + bad_new_files = [] for f in input_api.AffectedSourceFiles(source_file_filter): # Only examine the first 1,000 bytes of the file to avoid expensive and # fruitless regex searches over long files with no license. @@ -691,25 +692,39 @@ def CheckLicense(input_api, output_api, license_re=None, project_name=None, if accept_empty_files and not contents: continue if f.Action() == 'A': - # Stricter checking for new files. - if not new_license_re.search(contents): - bad_files.append(f.LocalPath()) - bad_new_files = True + # Stricter checking for new files (but might actually be moved). + match = new_license_re.search(contents) + if not match: + # License is totally wrong. + bad_new_files.append(f.LocalPath()) + elif match.groups()[0] != current_year: + # License does not list this year. + wrong_year_new_files.append(f.LocalPath()) elif not license_re.search(contents): bad_files.append(f.LocalPath()) + results = [] if bad_new_files: - return [ + results.append( output_api.PresubmitError( 'License on new files must match:\n%s\n\n' % new_license_re.pattern - + - 'Found a bad license header in these files, some of which are new:', - items=bad_files) - ] + + 'Found a bad license header in these new files:', + 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( + '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.', + items=wrong_year_new_files)) if bad_files: - return [output_api.PresubmitPromptWarning( - 'License must match:\n%s\n' % license_re.pattern + - 'Found a bad license header in these files:', items=bad_files)] - return [] + results.append( + output_api.PresubmitPromptWarning( + 'License must match:\n%s\n' % license_re.pattern + + 'Found a bad license header in these files:', + items=bad_files)) + return results ### Other checks