Revert "Add presubmit check for changes in 3pp"
This reverts commit 4103b383505575b23222f77fd04116d2f6c10273.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> 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}
TBR=phoglund@webrtc.org,kwiberg@webrtc.org,titovartem@webrtc.org
Change-Id: Ib016ee4ac58729c2c0d302a964dbac71b4ae64af
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8366
Reviewed-on: https://webrtc-review.googlesource.com/77780
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23302}diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 37948db..d4b7796 100755
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -13,6 +13,7 @@
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',
@@ -139,7 +140,6 @@
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,7 +159,6 @@
Related files:
"""
-
def CheckNativeApiHeaderChanges(input_api, output_api):
"""Checks to remind proper changing of native APIs."""
files = []
@@ -289,7 +288,7 @@
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:
@@ -304,7 +303,6 @@
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 \+?= \[(.*?)\]',
@@ -333,13 +331,11 @@
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:
@@ -401,7 +397,6 @@
'\n'.join(errors.keys())))]
return []
-
def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
cwd = input_api.PresubmitLocalPath()
with _AddToPath(input_api.os_path.join(
@@ -461,7 +456,6 @@
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 = []
@@ -483,7 +477,6 @@
line_number)))
return result
-
def CheckCheckIncludesIsNotUsed(gn_files, output_api):
result = []
error_msg = ('check_includes overrides are not allowed since it can cause '
@@ -501,7 +494,6 @@
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)$',),
@@ -522,7 +514,6 @@
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
@@ -539,7 +530,6 @@
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
@@ -599,7 +589,6 @@
warning_descriptions))
return results
-
def CheckCommitMessageBugEntry(input_api, output_api):
"""Check that bug entries are well-formed in commit message."""
bogus_bug_msg = (
@@ -625,7 +614,6 @@
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.
@@ -645,7 +633,6 @@
' * 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."""
@@ -668,8 +655,7 @@
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
@@ -792,14 +778,14 @@
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(
@@ -830,17 +816,9 @@
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))
@@ -896,7 +874,8 @@
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 = []
diff --git a/THIRD_PARTY_WEBRTC_DEPS.json b/THIRD_PARTY_WEBRTC_DEPS.json
index 53d45ee..2f2190f 100644
--- a/THIRD_PARTY_WEBRTC_DEPS.json
+++ b/THIRD_PARTY_WEBRTC_DEPS.json
@@ -5,6 +5,5 @@
"THIRD_PARTY_CHROMIUM_DEPS.json)."
],
"dependencies": [
- "OWNERS"
]
-}
+}
\ No newline at end of file
diff --git a/presubmit_test_mocks.py b/presubmit_test_mocks.py
index 6c0b0c4..4b2b3c7 100644
--- a/presubmit_test_mocks.py
+++ b/presubmit_test_mocks.py
@@ -9,9 +9,6 @@
# 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.
@@ -23,38 +20,10 @@
def __init__(self):
self.change = MockChange([], [])
self.files = []
- self.presubmit_local_path = os.path.dirname(__file__)
def AffectedSourceFiles(self, file_filter=None):
- 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
+ # pylint: disable=unused-argument
+ return self.files
def ReadFile(self, affected_file, mode='rU'):
filename = affected_file.AbsoluteLocalPath()
@@ -110,30 +79,11 @@
MockInputApi for presubmit unittests.
"""
- def __init__(self, local_path, new_contents=None, old_contents=None,
- action='A'):
- if new_contents is None:
- new_contents = ["Data"]
+ def __init__(self, local_path):
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
diff --git a/tools_webrtc/presubmit_checks_lib/check_3pp.py b/tools_webrtc/presubmit_checks_lib/check_3pp.py
deleted file mode 100644
index 5ef70b5..0000000
--- a/tools_webrtc/presubmit_checks_lib/check_3pp.py
+++ /dev/null
@@ -1,169 +0,0 @@
-#!/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
diff --git a/tools_webrtc/presubmit_checks_lib/check_3pp_test.py b/tools_webrtc/presubmit_checks_lib/check_3pp_test.py
deleted file mode 100755
index 1a685ca..0000000
--- a/tools_webrtc/presubmit_checks_lib/check_3pp_test.py
+++ /dev/null
@@ -1,126 +0,0 @@
-#!/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()
diff --git a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/chromium/source.js b/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/chromium/source.js
deleted file mode 100644
index e69de29..0000000
--- a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/chromium/source.js
+++ /dev/null
diff --git a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/not_owned/source.js b/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/not_owned/source.js
deleted file mode 100644
index e69de29..0000000
--- a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/not_owned/source.js
+++ /dev/null
diff --git a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/webrtc/source.js b/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/webrtc/source.js
deleted file mode 100644
index e69de29..0000000
--- a/tools_webrtc/presubmit_checks_lib/testdata/check_third_party_changes/not_owned_dep/third_party/webrtc/source.js
+++ /dev/null