Add LICENSE file checking
This patch adds license file checking in Open Screen, by adding a new
licenses.py that iterates through third_party dependencies and returns
a list of licensing errors. This runs as part of PRESUBMIT.
This patch also includes README.chromium and LICENSE fixes for
third_party dependencies.
Finally, as part of refactoring I fixed the DEPS checker--before this
patch it fails to load modules so never runs on pre-submission.
Bug: b/173625891
Change-Id: I417e61f878dab809cf959d69480be749f9229128
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2547807
Reviewed-by: Jordan Bayles <jophba@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index dafbc5e..07d567c 100755
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -2,7 +2,21 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+import os
import re
+import sys
+
+_REPO_PATH = os.path.dirname(os.path.realpath('__file__'))
+_IMPORT_SUBFOLDERS = ['tools', os.path.join('buildtools', 'checkdeps')]
+
+# git-cl upload is not compatible with __init__.py based subfolder imports, so
+# we extend the system path instead.
+sys.path.extend(os.path.join(_REPO_PATH, p) for p in _IMPORT_SUBFOLDERS)
+
+import licenses
+from checkdeps import DepsChecker
+from cpp_checker import CppChecker
+from rules import Rule
# Rather than pass this to all of the checks, we override the global excluded
# list with this one.
@@ -22,25 +36,19 @@
)
-def _CheckDeps(input_api, output_api):
- results = []
- import sys
- original_sys_path = sys.path
- try:
- sys.path = sys.path + [input_api.os_path.join(
- input_api.PresubmitLocalPath(), 'buildtools', 'checkdeps')]
- import checkdeps
- from cpp_checker import CppChecker
- from rules import Rule
- finally:
- sys.path = original_sys_path
+def _CheckLicenses(input_api, output_api):
+ """Checks third party licenses and returns a list of violations."""
+ return [
+ output_api.PresubmitError(v) for v in licenses.ScanThirdPartyDirs()
+ ]
- deps_checker = checkdeps.DepsChecker(input_api.PresubmitLocalPath())
- deps_checker.CheckDirectory(input_api.PresubmitLocalPath())
- deps_results = deps_checker.results_formatter.GetResults()
- for violation in deps_results:
- results.append(output_api.PresubmitError(violation))
- return results
+
+def _CheckDeps(input_api, output_api):
+ """Checks DEPS rules and returns a list of violations."""
+ deps_checker = DepsChecker(input_api.PresubmitLocalPath())
+ deps_checker.CheckDirectory(input_api.PresubmitLocalPath())
+ deps_results = deps_checker.results_formatter.GetResults()
+ return [output_api.PresubmitError(v) for v in deps_results]
# Matches Foo(Foo&&) when not followed by noexcept.
@@ -49,7 +57,7 @@
def _CheckNoexceptOnMove(filename, clean_lines, linenum, error):
- """Checks that move constructors are declared with 'noexcept'.
+ """Checks that move constructors are declared with 'noexcept'.
Args:
filename: The name of the current file.
@@ -57,17 +65,19 @@
linenum: The number of the line to check.
error: The function to call with any errors found.
"""
- # We only check headers as noexcept is meaningful on declarations, not
- # definitions. This may skip some definitions in .cc files though.
- if not filename.endswith('.h'):
- return
+ # We only check headers as noexcept is meaningful on declarations, not
+ # definitions. This may skip some definitions in .cc files though.
+ if not filename.endswith('.h'):
+ return
- line = clean_lines.elided[linenum]
- matched = _RE_PATTERN_MOVE_WITHOUT_NOEXCEPT.match(line)
- if matched:
- error(filename, linenum, 'runtime/noexcept', 4,
- 'Move constructor of %s not declared \'noexcept\' in %s' %
- (matched.group('classname'), matched.group(0).strip()))
+ line = clean_lines.elided[linenum]
+ matched = _RE_PATTERN_MOVE_WITHOUT_NOEXCEPT.match(line)
+ if matched:
+ error(
+ filename, linenum, 'runtime/noexcept', 4,
+ 'Move constructor of {} not declared \'noexcept\' in {}'.format(
+ matched.group('classname'),
+ matched.group(0).strip()))
# - We disable c++11 header checks since Open Screen allows them.
# - We disable whitespace/braces because of various false positives.
@@ -75,83 +85,89 @@
# enough to keep.
# - We add a custom check for 'noexcept' usage.
def _CheckChangeLintsClean(input_api, output_api):
- """Checks that all '.cc' and '.h' files pass cpplint.py."""
- result = []
+ """Checks that all '.cc' and '.h' files pass cpplint.py."""
+ cpplint = input_api.cpplint
+ # Access to a protected member _XX of a client class
+ # pylint: disable=protected-access
+ cpplint._cpplint_state.ResetErrorCounts()
- cpplint = input_api.cpplint
- # Access to a protected member _XX of a client class
- # pylint: disable=protected-access
- cpplint._cpplint_state.ResetErrorCounts()
+ cpplint._SetFilters('-build/c++11,-whitespace/braces')
+ files = [
+ f.AbsoluteLocalPath() for f in input_api.AffectedSourceFiles(None)
+ ]
+ CPPLINT_VERBOSE_LEVEL = 4
+ for file_name in files:
+ cpplint.ProcessFile(file_name, CPPLINT_VERBOSE_LEVEL,
+ [_CheckNoexceptOnMove])
- cpplint._SetFilters('-build/c++11,-whitespace/braces')
- files = [f.AbsoluteLocalPath() for f in input_api.AffectedSourceFiles(None)]
- for file_name in files:
- # 4 = verbose_level
- cpplint.ProcessFile(file_name, 4, [_CheckNoexceptOnMove])
+ if cpplint._cpplint_state.error_count:
+ if input_api.is_committing:
+ res_type = output_api.PresubmitError
+ else:
+ res_type = output_api.PresubmitPromptWarning
+ return [res_type('Changelist failed cpplint.py check.')]
- if cpplint._cpplint_state.error_count > 0:
- if input_api.is_committing:
- res_type = output_api.PresubmitError
- else:
- res_type = output_api.PresubmitPromptWarning
- result = [res_type('Changelist failed cpplint.py check.')]
-
- return result
+ return []
def _CommonChecks(input_api, output_api):
- results = []
- # PanProjectChecks include:
- # CheckLongLines (@ 80 cols)
- # CheckChangeHasNoTabs
- # CheckChangeHasNoStrayWhitespace
- # CheckLicense
- # CheckChangeWasUploaded (if committing)
- # CheckChangeHasDescription
- # CheckDoNotSubmitInDescription
- # CheckDoNotSubmitInFiles
- results.extend(input_api.canned_checks.PanProjectChecks(
- input_api, output_api, owners_check=False));
+ # PanProjectChecks include:
+ # CheckLongLines (@ 80 cols)
+ # CheckChangeHasNoTabs
+ # CheckChangeHasNoStrayWhitespace
+ # CheckLicense
+ # CheckChangeWasUploaded (if committing)
+ # CheckChangeHasDescription
+ # CheckDoNotSubmitInDescription
+ # CheckDoNotSubmitInFiles
+ results = input_api.canned_checks.PanProjectChecks(input_api,
+ output_api,
+ owners_check=False)
- # No carriage return characters, files end with one EOL (\n).
- results.extend(input_api.canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol(
- input_api, output_api));
+ # No carriage return characters, files end with one EOL (\n).
+ results.extend(
+ input_api.canned_checks.CheckChangeHasNoCrAndHasOnlyOneEol(
+ input_api, output_api))
- # Gender inclusivity
- results.extend(input_api.canned_checks.CheckGenderNeutral(
- input_api, output_api))
+ # Gender inclusivity
+ results.extend(
+ input_api.canned_checks.CheckGenderNeutral(input_api, output_api))
- # TODO(bug) format required
- results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
- input_api, output_api))
+ # TODO(bug) format required
+ results.extend(
+ input_api.canned_checks.CheckChangeTodoHasOwner(input_api, output_api))
- # Linter.
- results.extend(_CheckChangeLintsClean(input_api, output_api))
+ # Linter.
+ results.extend(_CheckChangeLintsClean(input_api, output_api))
- # clang-format
- results.extend(input_api.canned_checks.CheckPatchFormatted(
- input_api, output_api, bypass_warnings=False))
+ # clang-format
+ results.extend(
+ input_api.canned_checks.CheckPatchFormatted(input_api,
+ output_api,
+ bypass_warnings=False))
- # GN formatting
- results.extend(input_api.canned_checks.CheckGNFormatted(
- input_api, output_api))
+ # GN formatting
+ results.extend(
+ input_api.canned_checks.CheckGNFormatted(input_api, output_api))
- # buildtools/checkdeps
- results.extend(_CheckDeps(input_api, output_api))
- return results
+ # buildtools/checkdeps
+ results.extend(_CheckDeps(input_api, output_api))
+
+ # tools/licenses
+ results.extend(_CheckLicenses(input_api, output_api))
+
+ return results
def CheckChangeOnUpload(input_api, output_api):
- input_api.DEFAULT_FILES_TO_SKIP = _EXCLUDED_PATHS;
- results = []
- results.extend(_CommonChecks(input_api, output_api))
- results.extend(
- input_api.canned_checks.CheckChangedLUCIConfigs(input_api, output_api))
- return results
+ input_api.DEFAULT_FILES_TO_SKIP = _EXCLUDED_PATHS
+ # We always run the OnCommit checks, as well as some additional checks.
+ results = CheckChangeOnCommit(input_api, output_api)
+ results.extend(
+ input_api.canned_checks.CheckChangedLUCIConfigs(input_api, output_api))
+ return results
def CheckChangeOnCommit(input_api, output_api):
- input_api.DEFAULT_FILES_TO_SKIP = _EXCLUDED_PATHS;
- results = []
- results.extend(_CommonChecks(input_api, output_api))
- return results
+ input_api.DEFAULT_FILES_TO_SKIP = _EXCLUDED_PATHS
+ return _CommonChecks(input_api, output_api)