diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 276db1e83b..46be5ff06f 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -9,6 +9,7 @@ import json import os import re +import subprocess import sys @@ -84,6 +85,18 @@ LEGACY_API_DIRS = ( API_DIRS = NATIVE_API_DIRS[:] + LEGACY_API_DIRS[:] +def _RunCommand(command, cwd): + """Runs a command and returns the output from that command.""" + p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + cwd=cwd) + stdout = p.stdout.read() + stderr = p.stderr.read() + p.wait() + p.stdout.close() + p.stderr.close() + return p.returncode, stdout, stderr + + def _VerifyNativeApiHeadersListIsValid(input_api, output_api): """Ensures the list of native API header directories is up to date.""" non_existing_paths = [] @@ -285,6 +298,19 @@ def _CheckNoMixingCAndCCSources(input_api, gn_files, output_api): items=violating_gn_files.keys())] return [] +def _CheckNoPackageBoundaryViolations(input_api, gn_files, output_api): + cwd = input_api.PresubmitLocalPath() + script_path = os.path.join('tools-webrtc', 'check_package_boundaries.py') + webrtc_path = os.path.join('webrtc') + command = [sys.executable, script_path, webrtc_path] + command += [gn_file.LocalPath() for gn_file in gn_files] + returncode, _, stderr = _RunCommand(command, cwd) + if returncode: + return [output_api.PresubmitError( + 'There are package boundary violations in the following GN files:\n\n' + '%s' % stderr)] + return [] + def _CheckGnChanges(input_api, output_api): source_file_filter = lambda x: input_api.FilterSourceFile( x, white_list=(r'.+\.(gn|gni)$',)) @@ -298,6 +324,8 @@ def _CheckGnChanges(input_api, output_api): if gn_files: result.extend(_CheckNoSourcesAbove(input_api, gn_files, output_api)) result.extend(_CheckNoMixingCAndCCSources(input_api, gn_files, output_api)) + result.extend(_CheckNoPackageBoundaryViolations( + input_api, gn_files, output_api)) return result def _CheckUnwantedDependencies(input_api, output_api): @@ -405,8 +433,10 @@ def _RunPythonTests(input_api, output_api): return input_api.os_path.join(input_api.PresubmitLocalPath(), *args) test_directories = [ - join('tools-webrtc', 'autoroller', 'unittests'), - join('webrtc', 'tools', 'py_event_log_analyzer'), + join('webrtc', 'tools', 'py_event_log_analyzer') + ] + [ + root for root, _, files in os.walk(join('tools-webrtc')) + if any(f.endswith('_test.py') for f in files) ] tests = [] diff --git a/tools-webrtc/check_package_boundaries.py b/tools-webrtc/check_package_boundaries.py new file mode 100644 index 0000000000..75588fa7d8 --- /dev/null +++ b/tools-webrtc/check_package_boundaries.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python + +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +import argparse +import logging +import os +import re +import sys + + +DISPLAY_LEVEL = 1 +IGNORE_LEVEL = 0 + +# TARGET_RE matches a GN target, and extracts the target name and the contents. +TARGET_RE = re.compile(r'\d+\$(?P\s*)\w+\("(?P\w+)"\) {' + r'(?P.*?)' + r'\d+\$(?P=indent)}', + re.MULTILINE | re.DOTALL) + +# SOURCES_RE matches a block of sources inside a GN target. +SOURCES_RE = re.compile(r'sources \+?= \[(?P.*?)\]', + re.MULTILINE | re.DOTALL) + +LOG_FORMAT = '%(message)s' +ERROR_MESSAGE = ("{}:{} in target '{}':\n" + " Source file '{}'\n" + " crosses boundary of package '{}'.\n") + + +class Logger(object): + def __init__(self, messages_left=None): + self.log_level = DISPLAY_LEVEL + self.messages_left = messages_left + + def log(self, build_file_path, line_number, target_name, source_file, + subpackage): + if self.messages_left is not None: + if not self.messages_left: + self.log_level = IGNORE_LEVEL + else: + self.messages_left -= 1 + message = ERROR_MESSAGE.format(build_file_path, line_number, target_name, + source_file, subpackage) + logging.log(self.log_level, message) + + +def _BuildSubpackagesPattern(packages, query): + """Returns a regular expression that matches source files inside subpackages + of the given query.""" + query += '/' + length = len(query) + pattern = r'(?P\d+)\$\s*"(?P(?P' + pattern += '|'.join(package[length:] for package in packages + if package.startswith(query)) + pattern += r')/[\w\./]*)"' + return re.compile(pattern) + + +def _ReadFileAndPrependLines(file_path): + """Reads the contents of a file and prepends the line number to every line.""" + with open(file_path) as f: + return "".join("{}${}".format(line_number, line) + for line_number, line in enumerate(f, 1)) + + +def _CheckBuildFile(build_file_path, packages, logger): + """Iterates oven all the targets of the given BUILD.gn file, and verifies that + the source files referenced by it don't belong to any of it's subpackages. + Returns True if a package boundary violation was found. + """ + found_violations = False + package = os.path.dirname(build_file_path) + subpackages_re = _BuildSubpackagesPattern(packages, package) + + build_file_contents = _ReadFileAndPrependLines(build_file_path) + for target_match in TARGET_RE.finditer(build_file_contents): + target_name = target_match.group('target_name') + target_contents = target_match.group('target_contents') + for sources_match in SOURCES_RE.finditer(target_contents): + sources = sources_match.group('sources') + for subpackages_match in subpackages_re.finditer(sources): + subpackage = subpackages_match.group('subpackage') + source_file = subpackages_match.group('source_file') + line_number = subpackages_match.group('line_number') + if subpackage: + found_violations = True + logger.log(build_file_path, line_number, target_name, source_file, + subpackage) + + return found_violations + + +def CheckPackageBoundaries(root_dir, logger, build_files=None): + packages = [root for root, _, files in os.walk(root_dir) + if 'BUILD.gn' in files] + default_build_files = [os.path.join(package, 'BUILD.gn') + for package in packages] + + build_files = build_files or default_build_files + return any([_CheckBuildFile(build_file_path, packages, logger) + for build_file_path in build_files]) + + +def main(): + parser = argparse.ArgumentParser( + description='Script that checks package boundary violations in GN ' + 'build files.') + + parser.add_argument('root_dir', metavar='ROOT_DIR', + help='The root directory that contains all BUILD.gn ' + 'files to be processed.') + parser.add_argument('build_files', metavar='BUILD_FILE', nargs='*', + help='A list of BUILD.gn files to be processed. If no ' + 'files are given, all BUILD.gn files under ROOT_DIR ' + 'will be processed.') + parser.add_argument('--max_messages', type=int, default=None, + help='If set, the maximum number of violations to be ' + 'displayed.') + + args = parser.parse_args() + + logging.basicConfig(format=LOG_FORMAT) + logging.getLogger().setLevel(DISPLAY_LEVEL) + logger = Logger(args.max_messages) + + return CheckPackageBoundaries(args.root_dir, logger, args.build_files) + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/tools-webrtc/check_package_boundaries_test.py b/tools-webrtc/check_package_boundaries_test.py new file mode 100755 index 0000000000..437f6e6eac --- /dev/null +++ b/tools-webrtc/check_package_boundaries_test.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python + +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +import ast +import os +import unittest + +from check_package_boundaries import CheckPackageBoundaries + + +MSG_FORMAT = 'ERROR:check_package_boundaries.py: Unexpected %s.' +TESTDATA_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), + 'testdata') + + +def ReadPylFile(file_path): + with open(file_path) as f: + return ast.literal_eval(f.read()) + + +class Logger(object): + def __init__(self, test_dir): + self.messages = [] + self.test_dir = test_dir + + def log(self, build_file_path, line_number, target_name, source_file, + subpackage): + build_file_path = os.path.relpath(build_file_path, self.test_dir) + self.messages.append([build_file_path, line_number, target_name, + source_file, subpackage]) + + +class UnitTest(unittest.TestCase): + def RunTest(self, test_dir, check_all_build_files=False): + logger = Logger(test_dir) + build_files = [os.path.join(test_dir, 'BUILD.gn')] + if check_all_build_files: + build_files = None + CheckPackageBoundaries(test_dir, logger, build_files) + expected_messages = ReadPylFile(os.path.join(test_dir, 'expected.pyl')) + self.assertListEqual(sorted(expected_messages), sorted(logger.messages)) + + def test_no_errors(self): + self.RunTest(os.path.join(TESTDATA_DIR, 'no_errors')) + + def test_multiple_errors_single_target(self): + self.RunTest(os.path.join(TESTDATA_DIR, 'multiple_errors_single_target')) + + def test_multiple_errors_multiple_targets(self): + self.RunTest(os.path.join(TESTDATA_DIR, 'multiple_errors_multiple_targets')) + + def test_common_prefix(self): + self.RunTest(os.path.join(TESTDATA_DIR, 'common_prefix')) + + def test_all_build_files(self): + self.RunTest(os.path.join(TESTDATA_DIR, 'all_build_files'), True) + + +if __name__ == '__main__': + unittest.main() diff --git a/tools-webrtc/testdata/all_build_files/BUILD.gn b/tools-webrtc/testdata/all_build_files/BUILD.gn new file mode 100644 index 0000000000..46bd2bec8f --- /dev/null +++ b/tools-webrtc/testdata/all_build_files/BUILD.gn @@ -0,0 +1,14 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +target("dummy_target_ok") { + sources = [ + "dummy_source.cc", + "dummy_source.h", + ] +} diff --git a/tools-webrtc/testdata/all_build_files/expected.pyl b/tools-webrtc/testdata/all_build_files/expected.pyl new file mode 100644 index 0000000000..b067c3c5b0 --- /dev/null +++ b/tools-webrtc/testdata/all_build_files/expected.pyl @@ -0,0 +1,20 @@ +[['subpackage2/BUILD.gn', + '12', + 'error_2', + 'subsubpackage2/dummy_subsubpackage2.cc', + 'subsubpackage2'], + ['subpackage2/BUILD.gn', + '13', + 'error_2', + 'subsubpackage2/dummy_subsubpackage2.h', + 'subsubpackage2'], + ['subpackage1/BUILD.gn', + '12', + 'error_1', + 'subsubpackage1/dummy_subsubpackage1.cc', + 'subsubpackage1'], + ['subpackage1/BUILD.gn', + '13', + 'error_1', + 'subsubpackage1/dummy_subsubpackage1.h', + 'subsubpackage1']] diff --git a/tools-webrtc/testdata/all_build_files/subpackage1/BUILD.gn b/tools-webrtc/testdata/all_build_files/subpackage1/BUILD.gn new file mode 100644 index 0000000000..2653a2b607 --- /dev/null +++ b/tools-webrtc/testdata/all_build_files/subpackage1/BUILD.gn @@ -0,0 +1,15 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +target("error_1") { + sources = [ + "subpackage1.h", + "subsubpackage1/dummy_subsubpackage1.cc", + "subsubpackage1/dummy_subsubpackage1.h", + ] +} diff --git a/tools-webrtc/testdata/all_build_files/subpackage1/subsubpackage1/BUILD.gn b/tools-webrtc/testdata/all_build_files/subpackage1/subsubpackage1/BUILD.gn new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools-webrtc/testdata/all_build_files/subpackage2/BUILD.gn b/tools-webrtc/testdata/all_build_files/subpackage2/BUILD.gn new file mode 100644 index 0000000000..290036145d --- /dev/null +++ b/tools-webrtc/testdata/all_build_files/subpackage2/BUILD.gn @@ -0,0 +1,15 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +target("error_2") { + sources = [ + "subpackage2.h", + "subsubpackage2/dummy_subsubpackage2.cc", + "subsubpackage2/dummy_subsubpackage2.h", + ] +} diff --git a/tools-webrtc/testdata/all_build_files/subpackage2/subsubpackage2/BUILD.gn b/tools-webrtc/testdata/all_build_files/subpackage2/subsubpackage2/BUILD.gn new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools-webrtc/testdata/common_prefix/BUILD.gn b/tools-webrtc/testdata/common_prefix/BUILD.gn new file mode 100644 index 0000000000..b683408667 --- /dev/null +++ b/tools-webrtc/testdata/common_prefix/BUILD.gn @@ -0,0 +1,17 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +rtc_static_library("webrtc") { + sources = [ + "call.h", + "dummy_source.h", + ] + deps = [ + "call", + ] +} diff --git a/tools-webrtc/testdata/common_prefix/call/BUILD.gn b/tools-webrtc/testdata/common_prefix/call/BUILD.gn new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools-webrtc/testdata/common_prefix/expected.pyl b/tools-webrtc/testdata/common_prefix/expected.pyl new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tools-webrtc/testdata/common_prefix/expected.pyl @@ -0,0 +1 @@ +[] diff --git a/tools-webrtc/testdata/multiple_errors_multiple_targets/BUILD.gn b/tools-webrtc/testdata/multiple_errors_multiple_targets/BUILD.gn new file mode 100644 index 0000000000..db8c426588 --- /dev/null +++ b/tools-webrtc/testdata/multiple_errors_multiple_targets/BUILD.gn @@ -0,0 +1,28 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +target("dummy_target_ok") { + sources = [ + "dummy_source.cc", + "dummy_source.h", + ] +} + +target("error_1") { + sources = [ + "subpackage1/dummy_subpackage1.cc", + "subpackage1/dummy_subpackage1.h", + ] +} + +target("error_2") { + sources = [ + "subpackage1/dummy_subpackage2.cc", + "subpackage1/dummy_subpackage2.h", + ] +} diff --git a/tools-webrtc/testdata/multiple_errors_multiple_targets/expected.pyl b/tools-webrtc/testdata/multiple_errors_multiple_targets/expected.pyl new file mode 100644 index 0000000000..92b889cd17 --- /dev/null +++ b/tools-webrtc/testdata/multiple_errors_multiple_targets/expected.pyl @@ -0,0 +1,20 @@ +[['BUILD.gn', + '18', + 'error_1', + 'subpackage1/dummy_subpackage1.cc', + 'subpackage1'], + ['BUILD.gn', + '19', + 'error_1', + 'subpackage1/dummy_subpackage1.h', + 'subpackage1'], + ['BUILD.gn', + '25', + 'error_2', + 'subpackage1/dummy_subpackage2.cc', + 'subpackage1'], + ['BUILD.gn', + '26', + 'error_2', + 'subpackage1/dummy_subpackage2.h', + 'subpackage1']] diff --git a/tools-webrtc/testdata/multiple_errors_multiple_targets/subpackage1/BUILD.gn b/tools-webrtc/testdata/multiple_errors_multiple_targets/subpackage1/BUILD.gn new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools-webrtc/testdata/multiple_errors_multiple_targets/subpackage2/BUILD.gn b/tools-webrtc/testdata/multiple_errors_multiple_targets/subpackage2/BUILD.gn new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools-webrtc/testdata/multiple_errors_single_target/BUILD.gn b/tools-webrtc/testdata/multiple_errors_single_target/BUILD.gn new file mode 100644 index 0000000000..eb5ee40846 --- /dev/null +++ b/tools-webrtc/testdata/multiple_errors_single_target/BUILD.gn @@ -0,0 +1,14 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +target("dummy_target") { + sources = [ + "subpackage/dummy_subpackage_file.cc", + "subpackage/dummy_subpackage_file.h", + ] +} diff --git a/tools-webrtc/testdata/multiple_errors_single_target/expected.pyl b/tools-webrtc/testdata/multiple_errors_single_target/expected.pyl new file mode 100644 index 0000000000..94ec0bee12 --- /dev/null +++ b/tools-webrtc/testdata/multiple_errors_single_target/expected.pyl @@ -0,0 +1,10 @@ +[["BUILD.gn", + "11", + "dummy_target", + "subpackage/dummy_subpackage_file.cc", + "subpackage"], + ["BUILD.gn", + "12", + "dummy_target", + "subpackage/dummy_subpackage_file.h", + "subpackage"]] diff --git a/tools-webrtc/testdata/multiple_errors_single_target/subpackage/BUILD.gn b/tools-webrtc/testdata/multiple_errors_single_target/subpackage/BUILD.gn new file mode 100644 index 0000000000..700508155b --- /dev/null +++ b/tools-webrtc/testdata/multiple_errors_single_target/subpackage/BUILD.gn @@ -0,0 +1,14 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +group("dummy_supbackage") { + sources = [ + "dummy_subpackage.cc", + "dummy_subpackage.h", + ] +} diff --git a/tools-webrtc/testdata/no_errors/BUILD.gn b/tools-webrtc/testdata/no_errors/BUILD.gn new file mode 100644 index 0000000000..62fc42d6fe --- /dev/null +++ b/tools-webrtc/testdata/no_errors/BUILD.gn @@ -0,0 +1,22 @@ +# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +group("testdata") { + deps = [ + ":dummy_target", + ] +} + +static_library("dummy_target") { + sources = [ + "dummy.cc", + ] + deps = [ + "subdir", + ] +} diff --git a/tools-webrtc/testdata/no_errors/expected.pyl b/tools-webrtc/testdata/no_errors/expected.pyl new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/tools-webrtc/testdata/no_errors/expected.pyl @@ -0,0 +1 @@ +[]