Check added files have current year in license header
Changed the hook to return a list of errors in the course of the change.
It will be helpful for users to find as many errors in a single run of
linter.
TEST=./pre-upload_unittest.py
TEST=tried to add new file with wrong year and checked linter complains.
BUG=chromium:980061
Change-Id: I2cd0ee95493f47114d62fcb81d72fccc627b8ca1
Reviewed-on: https://chromium-review.googlesource.com/1687034
Tested-by: Keigo Oka <oka@chromium.org>
Commit-Ready: Keigo Oka <oka@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index 7bcadb2..a96f52e 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -14,6 +14,7 @@
import argparse
import collections
import ConfigParser
+import datetime
import fnmatch
import functools
import json
@@ -1070,8 +1071,8 @@
"""
# For older years, be a bit more flexible as our policy says leave them be.
LICENSE_HEADER = (
- r'.*Copyright( \(c\))? 20[-0-9]{2,7} The Chromium OS Authors\. '
- r'All rights reserved\.' r'\n'
+ r'.*Copyright(?: \(c\))? (20[0-9]{2})(?:-20[0-9]{2})? The Chromium OS '
+ r'Authors\. All rights reserved\.\n'
r'.*Use of this source code is governed by a BSD-style license that can '
r'be\n'
r'.*found in the LICENSE file\.'
@@ -1080,42 +1081,58 @@
license_re = re.compile(LICENSE_HEADER, re.MULTILINE)
# For newer years, be stricter.
- COPYRIGHT_LINE = (
+ BAD_COPYRIGHT_LINE = (
r'.*Copyright \(c\) 20(1[5-9]|[2-9][0-9]) The Chromium OS Authors\. '
r'All rights reserved\.' r'\n'
)
- copyright_re = re.compile(COPYRIGHT_LINE)
+ bad_copyright_re = re.compile(BAD_COPYRIGHT_LINE)
included, excluded = _parse_common_inclusion_options(options)
bad_files = []
bad_copyright_files = []
+ bad_year_files = []
+
files = _filter_files(
_get_affected_files(commit, relative=True),
included + COMMON_INCLUDED_PATHS,
excluded + COMMON_EXCLUDED_PATHS + LICENSE_EXCLUDED_PATHS)
+ existing_files = set(_get_affected_files(commit, relative=True,
+ include_adds=False))
+ current_year = str(datetime.datetime.now().year)
for f in files:
contents = _get_file_content(f, commit)
if not contents:
# Ignore empty files.
continue
- if not license_re.search(contents):
+ m = license_re.search(contents)
+ if not m:
bad_files.append(f)
- elif copyright_re.search(contents):
+ elif bad_copyright_re.search(contents):
bad_copyright_files.append(f)
+ if m and f not in existing_files:
+ year = m.group(1)
+ if year != current_year:
+ bad_year_files.append(f)
+
+ errors = []
if bad_files:
msg = '%s:\n%s\n%s' % (
'License must match', license_re.pattern,
'Found a bad header in these files:')
- return HookFailure(msg, bad_files)
-
+ errors.append(HookFailure(msg, bad_files))
if bad_copyright_files:
msg = 'Do not use (c) in copyright headers in new files:'
- return HookFailure(msg, bad_copyright_files)
+ errors.append(HookFailure(msg, bad_copyright_files))
+ if bad_year_files:
+ msg = 'Use current year (%s) in copyright headers in new files:' % (
+ current_year)
+ errors.append(HookFailure(msg, bad_year_files))
+ return errors
def _check_aosp_license(_project, commit, options=()):
"""Verifies the AOSP license/copyright header.
@@ -1846,7 +1863,10 @@
print(' ' * len(output), end='\r')
sys.stdout.flush()
if hook_error:
- error_list.append(hook_error)
+ if isinstance(hook_error, list):
+ error_list.extend(hook_error)
+ else:
+ error_list.append(hook_error)
error_found = True
if error_list:
PrintErrorsForCommit(project.name, commit, _get_commit_desc(commit),
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index feb53c4..7d32d3b 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -8,6 +8,7 @@
from __future__ import print_function
+import datetime
import os
import sys
@@ -727,15 +728,44 @@
'# Use of this source code is governed by a BSD-style license that'
' can be\n'
'# found in the LICENSE file.\n'),
- ('// Copyright 2010-13 The Chromium OS Authors. All rights reserved.\n'
- '// Use of this source code is governed by a BSD-style license that'
+ ('// Copyright 2010-2013 The Chromium OS 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'),
)
self.file_mock.return_value = ['file']
for header in HEADERS:
self.content_mock.return_value = header
- self.assertEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertFalse(pre_upload._check_cros_license('proj', 'sha1'))
+
+ def testNewFileYear(self):
+ """Added files should have the current year in license header."""
+ year = datetime.datetime.now().year
+ HEADERS = (
+ ('// Copyright 2016 The Chromium OS 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'),
+ ('// Copyright %d The Chromium OS 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') % year,
+ )
+ want_error = (True, False)
+ def fake_get_affected_files(_, relative, include_adds=True):
+ _ = relative
+ if include_adds:
+ return ['file']
+ else:
+ return []
+
+ self.file_mock.side_effect = fake_get_affected_files
+ for i, header in enumerate(HEADERS):
+ self.content_mock.return_value = header
+ if want_error[i]:
+ self.assertTrue(pre_upload._check_cros_license('proj', 'sha1'))
+ else:
+ self.assertFalse(pre_upload._check_cros_license('proj', 'sha1'))
def testRejectC(self):
"""Reject the (c) in newer headers."""
@@ -752,7 +782,7 @@
self.file_mock.return_value = ['file']
for header in HEADERS:
self.content_mock.return_value = header
- self.assertNotEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertTrue(pre_upload._check_cros_license('proj', 'sha1'))
def testNoLeadingSpace(self):
"""Allow headers without leading space (e.g., not a source comment)"""
@@ -765,25 +795,25 @@
self.file_mock.return_value = ['file']
for header in HEADERS:
self.content_mock.return_value = header
- self.assertEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertFalse(pre_upload._check_cros_license('proj', 'sha1'))
def testNoExcludedGolang(self):
"""Don't exclude .go files for license checks."""
self.file_mock.return_value = ['foo/main.go']
self.content_mock.return_value = ('package main\nfunc main() {}')
- self.assertNotEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertTrue(pre_upload._check_cros_license('proj', 'sha1'))
def testIgnoreExcludedPaths(self):
"""Ignores excluded paths for license checks."""
self.file_mock.return_value = ['foo/OWNERS']
self.content_mock.return_value = ('owner@chromium.org')
- self.assertEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertFalse(pre_upload._check_cros_license('proj', 'sha1'))
def testIgnoreTopLevelExcludedPaths(self):
"""Ignores excluded paths for license checks."""
self.file_mock.return_value = ['OWNERS']
self.content_mock.return_value = ('owner@chromium.org')
- self.assertEqual(None, pre_upload._check_cros_license('proj', 'sha1'))
+ self.assertFalse(pre_upload._check_cros_license('proj', 'sha1'))
class CheckAOSPLicenseCopyrightHeader(cros_test_lib.MockTestCase):
"""Tests for _check_aosp_license."""
