Add presubmit check to prevent package boundary violations.

BUG=webrtc:6954
NOTRY=True

Review-Url: https://codereview.webrtc.org/2629723004
Cr-Commit-Position: refs/heads/master@{#16357}

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 = []

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<indent>\s*)\w+\("(?P<target_name>\w+)"\) {'
+                       r'(?P<target_contents>.*?)'
+                       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<sources>.*?)\]',
+                        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<line_number>\d+)\$\s*"(?P<source_file>(?P<subpackage>'
+  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())

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()

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",
+  ]
+}

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']]

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",
+  ]
+}

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",
+  ]
+}

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",
+  ]
+}

tools-webrtc/testdata/common_prefix/expected.pyl

@@ -0,0 +1 @@
+[]

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",
+  ]
+}

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']]

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",
+  ]
+}

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"]]

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",
+  ]
+}

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",
+  ]
+}

tools-webrtc/testdata/no_errors/expected.pyl

@@ -0,0 +1 @@
+[]