Add presubmit check for changes in 3pp

Presubmit check will test will new changes be overriden by autoroll
or not. In more details presubmit will check:
1. Each dependency in third_party have to be specified in one of:
   a. THIRD_PARTY_CHROMIUM_DEPS.json
   b. THIRD_PARTY_WEBRTC_DEPS.json
2. Each dependency not specified in both files from #1
3. Changes won't be overriden by chromium third_party deps autoroll:
   a. Changes were made in WebRTC owned dependency
   b. Changes were addition of new Chromium owned dependency

Bug: webrtc:8366
Change-Id: Ic5db24289e7fa461e0959f75cfbe81ecc65af4b5
Reviewed-on: https://webrtc-review.googlesource.com/77421
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23301}

PRESUBMIT.py

@@ -13,7 +13,6 @@ import sys
 from collections import defaultdict
 from contextlib import contextmanager
 
-
 # Files and directories that are *skipped* by cpplint in the presubmit script.
 CPPLINT_BLACKLIST = [
   'api/video_codecs/video_decoder.h',
@@ -140,6 +139,7 @@ def VerifyNativeApiHeadersListIsValid(input_api, output_api):
         non_existing_paths)]
   return []
 
+
 API_CHANGE_MSG = """
 You seem to be changing native API header files. Please make sure that you:
   1. Make compatible changes that don't break existing clients. Usually
@@ -159,6 +159,7 @@ You seem to be changing native API header files. Please make sure that you:
 Related files:
 """
 
+
 def CheckNativeApiHeaderChanges(input_api, output_api):
   """Checks to remind proper changing of native APIs."""
   files = []
@@ -288,7 +289,7 @@ def CheckApprovedFilesLintClean(input_api, output_api,
   for f in input_api.AffectedSourceFiles(source_file_filter):
     # Note that moved/renamed files also count as added.
     if f.Action() == 'A' or not IsLintBlacklisted(blacklist_paths,
-                                                   f.LocalPath()):
+                                                  f.LocalPath()):
       files.append(f.AbsoluteLocalPath())
 
   for file_name in files:
@@ -303,6 +304,7 @@ def CheckApprovedFilesLintClean(input_api, output_api,
 
   return result
 
+
 def CheckNoSourcesAbove(input_api, gn_files, output_api):
   # Disallow referencing source files with paths above the GN file location.
   source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
@@ -331,11 +333,13 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api):
         items=violating_gn_files)]
   return []
 
+
 def CheckNoMixingSources(input_api, gn_files, output_api):
   """Disallow mixing C, C++ and Obj-C/Obj-C++ in the same target.
 
   See bugs.webrtc.org/7743 for more context.
   """
+
   def _MoreThanOneSourceUsed(*sources_lists):
     sources_used = 0
     for source_list in sources_lists:
@@ -397,6 +401,7 @@ def CheckNoMixingSources(input_api, gn_files, output_api):
                                        '\n'.join(errors.keys())))]
   return []
 
+
 def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
   cwd = input_api.PresubmitLocalPath()
   with _AddToPath(input_api.os_path.join(
@@ -456,6 +461,7 @@ def CheckNoStreamUsageIsAdded(input_api, output_api,
     return [output_api.PresubmitError(error_msg, errors)]
   return []
 
+
 def CheckPublicDepsIsNotUsed(gn_files, input_api, output_api):
   """Checks that public_deps is not used without a good reason."""
   result = []
@@ -477,6 +483,7 @@ def CheckPublicDepsIsNotUsed(gn_files, input_api, output_api):
                                                    line_number)))
   return result
 
+
 def CheckCheckIncludesIsNotUsed(gn_files, output_api):
   result = []
   error_msg = ('check_includes overrides are not allowed since it can cause '
@@ -494,6 +501,7 @@ def CheckCheckIncludesIsNotUsed(gn_files, output_api):
                                                    line_number)))
   return result
 
+
 def CheckGnChanges(input_api, output_api, source_file_filter):
   file_filter = lambda x: (input_api.FilterSourceFile(
       x, white_list=(r'.+\.(gn|gni)$',),
@@ -514,6 +522,7 @@ def CheckGnChanges(input_api, output_api, source_file_filter):
     result.extend(CheckCheckIncludesIsNotUsed(gn_files, output_api))
   return result
 
+
 def CheckGnGen(input_api, output_api):
   """Runs `gn gen --check` with default args to detect mismatches between
   #includes and dependencies in the BUILD.gn files, as well as general build
@@ -530,6 +539,7 @@ def CheckGnGen(input_api, output_api):
         long_text='\n\n'.join(errors))]
   return []
 
+
 def CheckUnwantedDependencies(input_api, output_api, source_file_filter):
   """Runs checkdeps on #include statements added in this
   change. Breaking - rules is an error, breaking ! rules is a
@@ -589,6 +599,7 @@ def CheckUnwantedDependencies(input_api, output_api, source_file_filter):
         warning_descriptions))
   return results
 
+
 def CheckCommitMessageBugEntry(input_api, output_api):
   """Check that bug entries are well-formed in commit message."""
   bogus_bug_msg = (
@@ -614,6 +625,7 @@ def CheckCommitMessageBugEntry(input_api, output_api):
       results.append(bogus_bug_msg % bug)
   return [output_api.PresubmitError(r) for r in results]
 
+
 def CheckChangeHasBugField(input_api, output_api):
   """Requires that the changelist is associated with a bug.
 
@@ -633,6 +645,7 @@ def CheckChangeHasBugField(input_api, output_api):
         ' * https://bugs.webrtc.org - reference it using Bug: webrtc:XXXX\n'
         ' * https://crbug.com - reference it using Bug: chromium:XXXXXX')]
 
+
 def CheckJSONParseErrors(input_api, output_api, source_file_filter):
   """Check that JSON files do not contain syntax errors."""
 
@@ -655,7 +668,8 @@ def CheckJSONParseErrors(input_api, output_api, source_file_filter):
                                     affected_file.AbsoluteLocalPath())
     if parse_error:
       results.append(output_api.PresubmitError('%s could not be parsed: %s' %
-          (affected_file.LocalPath(), parse_error)))
+                                               (affected_file.LocalPath(),
+                                                parse_error)))
   return results
 
 
@@ -778,14 +792,14 @@ def CommonChecks(input_api, output_api):
                  third_party_filter_list)
   hundred_char_sources = lambda x: input_api.FilterSourceFile(x,
       white_list=objc_filter_list)
+  non_third_party_sources = lambda x: input_api.FilterSourceFile(x,
+      black_list=third_party_filter_list)
+
   results.extend(input_api.canned_checks.CheckLongLines(
       input_api, output_api, maxlen=80, source_file_filter=eighty_char_sources))
   results.extend(input_api.canned_checks.CheckLongLines(
       input_api, output_api, maxlen=100,
       source_file_filter=hundred_char_sources))
-
-  non_third_party_sources = lambda x: input_api.FilterSourceFile(x,
-      black_list=third_party_filter_list)
   results.extend(input_api.canned_checks.CheckChangeHasNoTabs(
       input_api, output_api, source_file_filter=non_third_party_sources))
   results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
@@ -816,9 +830,17 @@ def CommonChecks(input_api, output_api):
       input_api, output_api, source_file_filter=non_third_party_sources))
   results.extend(CheckNoStreamUsageIsAdded(
       input_api, output_api, non_third_party_sources))
+  results.extend(CheckThirdPartyChanges(input_api, output_api))
   return results
 
 
+def CheckThirdPartyChanges(input_api, output_api):
+  with _AddToPath(input_api.os_path.join(
+      input_api.PresubmitLocalPath(), 'tools_webrtc', 'presubmit_checks_lib')):
+    from check_3pp import CheckThirdPartyDirectory
+  return CheckThirdPartyDirectory(input_api, output_api)
+
+
 def CheckChangeOnUpload(input_api, output_api):
   results = []
   results.extend(CommonChecks(input_api, output_api))
@@ -874,8 +896,7 @@ def CheckOrphanHeaders(input_api, output_api, source_file_filter):
   return results
 
 
-def CheckNewlineAtTheEndOfProtoFiles(input_api, output_api,
-                                     source_file_filter):
+def CheckNewlineAtTheEndOfProtoFiles(input_api, output_api, source_file_filter):
   """Checks that all .proto files are terminated with a newline."""
   error_msg = 'File {} must end with exactly one newline.'
   results = []

THIRD_PARTY_WEBRTC_DEPS.json

@@ -5,5 +5,6 @@
     "THIRD_PARTY_CHROMIUM_DEPS.json)."
   ],
   "dependencies": [
+    "OWNERS"
   ]
-}
+}

presubmit_test_mocks.py

@@ -9,6 +9,9 @@
 # This file is inspired to [1].
 # [1] - https://cs.chromium.org/chromium/src/PRESUBMIT_test_mocks.py
 
+import os.path
+import re
+
 
 class MockInputApi(object):
   """Mock class for the InputApi class.
@@ -20,10 +23,38 @@ class MockInputApi(object):
   def __init__(self):
     self.change = MockChange([], [])
     self.files = []
+    self.presubmit_local_path = os.path.dirname(__file__)
 
   def AffectedSourceFiles(self, file_filter=None):
-    # pylint: disable=unused-argument
-    return self.files
+    return self.AffectedFiles(file_filter=file_filter)
+
+  def AffectedFiles(self, file_filter=None, include_deletes=False):
+    for f in self.files:
+      if file_filter and not file_filter(f):
+        continue
+      if not include_deletes and f.Action() == 'D':
+        continue
+      yield f
+
+  @classmethod
+  def FilterSourceFile(cls, affected_file, white_list=(), black_list=()):
+    local_path = affected_file.LocalPath()
+    found_in_white_list = not white_list
+    if white_list:
+      for pattern in white_list:
+        compiled_pattern = re.compile(pattern)
+        if compiled_pattern.search(local_path):
+          found_in_white_list = True
+          break
+    if black_list:
+      for pattern in black_list:
+        compiled_pattern = re.compile(pattern)
+        if compiled_pattern.search(local_path):
+          return False
+    return found_in_white_list
+
+  def PresubmitLocalPath(self):
+    return self.presubmit_local_path
 
   def ReadFile(self, affected_file, mode='rU'):
     filename = affected_file.AbsoluteLocalPath()
@@ -79,11 +110,30 @@ class MockFile(object):
   MockInputApi for presubmit unittests.
   """
 
-  def __init__(self, local_path):
+  def __init__(self, local_path, new_contents=None, old_contents=None,
+      action='A'):
+    if new_contents is None:
+      new_contents = ["Data"]
     self._local_path = local_path
+    self._new_contents = new_contents
+    self._changed_contents = [(i + 1, l) for i, l in enumerate(new_contents)]
+    self._action = action
+    self._old_contents = old_contents
+
+  def Action(self):
+    return self._action
+
+  def ChangedContents(self):
+    return self._changed_contents
+
+  def NewContents(self):
+    return self._new_contents
 
   def LocalPath(self):
     return self._local_path
 
   def AbsoluteLocalPath(self):
     return self._local_path
+
+  def OldContents(self):
+    return self._old_contents

tools_webrtc/presubmit_checks_lib/check_3pp.py

@@ -0,0 +1,169 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2018 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 json
+import logging
+import os.path
+import subprocess
+import sys
+import re
+
+
+def CheckThirdPartyDirectory(input_api, output_api):
+  # We have to put something in black_list here that won't blacklist
+  # third_party/* because otherwise default black list will be used. Default
+  # list contains third_party, so source set will become empty.
+  third_party_sources = lambda x: (
+    input_api.FilterSourceFile(x, white_list=(r'^third_party[\\\/].+',),
+                               black_list=(r'^_',)))
+
+  webrtc_owned_deps_list_path = input_api.os_path.join(
+      input_api.PresubmitLocalPath(),
+      'THIRD_PARTY_WEBRTC_DEPS.json')
+  chromium_owned_deps_list_path = input_api.os_path.join(
+      input_api.PresubmitLocalPath(),
+      'THIRD_PARTY_CHROMIUM_DEPS.json')
+  webrtc_owned_deps = _LoadDepsList(webrtc_owned_deps_list_path)
+  chromium_owned_deps = _LoadDepsList(chromium_owned_deps_list_path)
+  chromium_added_deps = GetChromiumOwnedAddedDeps(input_api)
+
+  results = []
+  results.extend(CheckNoNotOwned3ppDeps(input_api, output_api,
+                                        webrtc_owned_deps, chromium_owned_deps))
+  results.extend(CheckNoBothOwned3ppDeps(output_api, webrtc_owned_deps,
+                                         chromium_owned_deps))
+  results.extend(CheckNoChangesInAutoImportedDeps(input_api, output_api,
+                                                  webrtc_owned_deps,
+                                                  chromium_owned_deps,
+                                                  chromium_added_deps,
+                                                  third_party_sources))
+  return results
+
+
+def GetChromiumOwnedAddedDeps(input_api):
+  """Return list of deps that were added into chromium owned deps list."""
+
+  chromium_owned_deps_list_source = lambda x: (
+    input_api.FilterSourceFile(x,
+                               white_list=('THIRD_PARTY_CHROMIUM_DEPS.json',),
+                               black_list=(r'^_',)))
+
+  chromium_owned_deps_list = input_api.AffectedFiles(
+      file_filter=chromium_owned_deps_list_source)
+  modified_deps_file = next(iter(chromium_owned_deps_list), None)
+  if not modified_deps_file:
+    return []
+  if modified_deps_file.Action() != 'M':
+    return []
+  prev_json = json.loads('\n'.join(modified_deps_file.OldContents()))
+  new_json = json.loads('\n'.join(modified_deps_file.NewContents()))
+  prev_deps_set = set(prev_json.get('dependencies', []))
+  new_deps_set = set(new_json.get('dependencies', []))
+  return list(new_deps_set.difference(prev_deps_set))
+
+
+def CheckNoNotOwned3ppDeps(input_api, output_api,
+    webrtc_owned_deps, chromium_owned_deps):
+  """Checks that there are no any not owned third_party deps."""
+  error_msg = ('Third party dependency [{}] have to be specified either in '
+               'THIRD_PARTY_WEBRTC_DEPS.json or in '
+               'THIRD_PARTY_CHROMIUM_DEPS.json.\n'
+               'If you want to add chromium-specific'
+               'dependency you can run this command (better in separate CL): \n'
+               './tools_webrtc/autoroller/checkin_chromium_dep.py -d {}\n'
+               'If you want to add WebRTC-specific dependency just add it into '
+               'THIRD_PARTY_WEBRTC_DEPS.json manually')
+
+  third_party_dir = os.path.join(input_api.PresubmitLocalPath(), 'third_party')
+  os.listdir(third_party_dir)
+  stdout, _ = _RunCommand(['git', 'ls-tree', '--name-only', 'HEAD'],
+                          working_dir=third_party_dir)
+  not_owned_deps = set()
+  results = []
+  for dep_name in stdout.split('\n'):
+    dep_name = dep_name.strip()
+    if len(dep_name) == 0:
+      continue
+    if dep_name == '.gitignore':
+      continue
+    if (dep_name not in webrtc_owned_deps
+        and dep_name not in chromium_owned_deps):
+      results.append(
+          output_api.PresubmitError(error_msg.format(dep_name, dep_name)))
+      not_owned_deps.add(dep_name)
+  return results
+
+
+def CheckNoBothOwned3ppDeps(output_api, webrtc_owned_deps, chromium_owned_deps):
+  """Checks that there are no any not owned third_party deps."""
+  error_msg = ('Third party dependencies {} can\'t be a WebRTC- and '
+               'Chromium-specific dependency at the same time. '
+               'Remove them from one of these files: '
+               'THIRD_PARTY_WEBRTC_DEPS.json or THIRD_PARTY_CHROMIUM_DEPS.json')
+
+  both_owned_deps = set(chromium_owned_deps).intersection(
+      set(webrtc_owned_deps))
+  results = []
+  if both_owned_deps:
+    results.append(output_api.PresubmitError(error_msg.format(
+        json.dumps(list(both_owned_deps)))))
+  return results
+
+
+def CheckNoChangesInAutoImportedDeps(input_api, output_api,
+    webrtc_owned_deps, chromium_owned_deps, chromium_added_deps,
+    third_party_sources):
+  """Checks that there are no changes in deps imported by autoroller."""
+
+  error_msg = ('Changes in [{}] will be overridden during chromium third_party '
+               'autoroll. If you really want to change this code you have to '
+               'do it upstream in Chromium\'s third_party.')
+  results = []
+  for f in input_api.AffectedFiles(file_filter=third_party_sources):
+    file_path = f.LocalPath()
+    split = re.split(r'[\\\/]', file_path)
+    dep_name = split[1]
+    if (dep_name not in webrtc_owned_deps
+        and dep_name in chromium_owned_deps
+        and dep_name not in chromium_added_deps):
+      results.append(output_api.PresubmitError(error_msg.format(file_path)))
+  return results
+
+
+def _LoadDepsList(file_name):
+  with open(file_name, 'rb') as f:
+    content = json.load(f)
+    return content.get('dependencies', [])
+
+
+def _RunCommand(command, working_dir):
+  """Runs a command and returns the output from that command.
+
+  If the command fails (exit code != 0), the function will exit the process.
+
+  Returns:
+    A tuple containing the stdout and stderr outputs as strings.
+  """
+  env = os.environ.copy()
+  p = subprocess.Popen(command,
+                       stdin=subprocess.PIPE,
+                       stdout=subprocess.PIPE,
+                       stderr=subprocess.PIPE, env=env,
+                       cwd=working_dir, universal_newlines=True)
+  std_output, err_output = p.communicate()
+  p.stdout.close()
+  p.stderr.close()
+  if p.returncode != 0:
+    logging.error('Command failed: %s\n'
+                  'stdout:\n%s\n'
+                  'stderr:\n%s\n', ' '.join(command), std_output, err_output)
+    sys.exit(p.returncode)
+  return std_output, err_output

tools_webrtc/presubmit_checks_lib/check_3pp_test.py

@@ -0,0 +1,126 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2018 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 os.path
+import sys
+import unittest
+import check_3pp
+
+SCRIPT_DIR = os.path.realpath(os.path.dirname(os.path.abspath(__file__)))
+CHECKOUT_SRC_DIR = os.path.realpath(os.path.join(SCRIPT_DIR, os.pardir,
+                                                 os.pardir))
+TEST_DATA_DIR = os.path.join(SCRIPT_DIR, 'testdata',
+                             'check_third_party_changes')
+sys.path.append(CHECKOUT_SRC_DIR)
+from presubmit_test_mocks import MockInputApi, MockOutputApi, MockFile
+
+
+class CheckThirdPartyChangesTest(unittest.TestCase):
+
+  def setUp(self):
+    self._input_api = MockInputApi()
+    self._output_api = MockOutputApi()
+
+  def testGetChromiumOwnedAddedDeps(self):
+    self._input_api.files = [
+      MockFile('THIRD_PARTY_CHROMIUM_DEPS.json',
+               new_contents=[
+                 """
+                 {
+                   "dependencies": [
+                     "foo",
+                     "bar",
+                     "buzz",
+                     "xyz"
+                   ]
+                 }
+                 """
+               ],
+               old_contents=[
+                 """
+                 {
+                   "dependencies": [
+                     "foo",
+                     "buzz"
+                   ]
+                 }
+                 """
+               ],
+               action='M')
+    ]
+    added_deps = check_3pp.GetChromiumOwnedAddedDeps(self._input_api)
+    self.assertEqual(len(added_deps), 2, added_deps)
+    self.assertIn('bar', added_deps)
+    self.assertIn('xyz', added_deps)
+
+  def testCheckNoNotOwned3ppDepsFire(self):
+    self._input_api.presubmit_local_path = os.path.join(TEST_DATA_DIR,
+                                                        'not_owned_dep')
+    errors = check_3pp.CheckNoNotOwned3ppDeps(self._input_api, self._output_api,
+                                              ['webrtc'], ['chromium'])
+    self.assertEqual(len(errors), 1)
+    self.assertIn('not_owned', errors[0].message)
+
+  def testCheckNoNotOwned3ppDepsNotFire(self):
+    self._input_api.presubmit_local_path = os.path.join(TEST_DATA_DIR,
+                                                        'not_owned_dep')
+    errors = check_3pp.CheckNoNotOwned3ppDeps(self._input_api, self._output_api,
+                                              ['webrtc', 'not_owned'],
+                                              ['chromium'])
+    self.assertEqual(len(errors), 0, errors)
+
+  def testCheckNoBothOwned3ppDepsFire(self):
+    errors = check_3pp.CheckNoBothOwned3ppDeps(self._output_api, ['foo', 'bar'],
+                                               ['buzz', 'bar'])
+    self.assertEqual(len(errors), 1)
+    self.assertIn('bar', errors[0].message)
+
+  def testCheckNoBothOwned3ppDepsNotFire(self):
+    errors = check_3pp.CheckNoBothOwned3ppDeps(self._output_api, ['foo', 'bar'],
+                                               ['buzz'])
+    self.assertEqual(len(errors), 0, errors)
+
+  def testCheckNoChangesInAutoImportedDepsFire(self):
+    self._input_api.files = [
+      MockFile('third_party/chromium/source.js')
+    ]
+    errors = check_3pp.CheckNoChangesInAutoImportedDeps(self._input_api,
+                                                        self._output_api,
+                                                        ['webrtc'],
+                                                        ['chromium'], [],
+                                                        None)
+    self.assertEqual(len(errors), 1)
+    self.assertIn('chromium', errors[0].message)
+
+  def testCheckNoChangesInAutoImportedDepsNotFire(self):
+    self._input_api.files = [
+      MockFile('third_party/webrtc/source.js')
+    ]
+    errors = check_3pp.CheckNoChangesInAutoImportedDeps(self._input_api,
+                                                        self._output_api,
+                                                        ['webrtc'],
+                                                        ['chromium'], [],
+                                                        None)
+    self.assertEqual(len(errors), 0, errors)
+
+  def testCheckNoChangesInAutoImportedDepsNotFireOnNewlyAdded(self):
+    self._input_api.files = [
+      MockFile('third_party/chromium/source.js')
+    ]
+    errors = check_3pp.CheckNoChangesInAutoImportedDeps(self._input_api,
+                                                        self._output_api,
+                                                        ['webrtc'],
+                                                        ['chromium'],
+                                                        ['chromium'], None)
+    self.assertEqual(len(errors), 0, errors)
+
+
+if __name__ == '__main__':
+  unittest.main()