diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 37948dbdf8..d4b7796288 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -13,6 +13,7 @@ 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', @@ -139,7 +140,6 @@ 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,7 +159,6 @@ 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 = [] @@ -289,7 +288,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: @@ -304,7 +303,6 @@ 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 \+?= \[(.*?)\]', @@ -333,13 +331,11 @@ 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: @@ -401,7 +397,6 @@ 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( @@ -461,7 +456,6 @@ 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 = [] @@ -483,7 +477,6 @@ 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 ' @@ -501,7 +494,6 @@ 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)$',), @@ -522,7 +514,6 @@ 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 @@ -539,7 +530,6 @@ 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 @@ -599,7 +589,6 @@ 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 = ( @@ -625,7 +614,6 @@ 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. @@ -645,7 +633,6 @@ 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.""" @@ -668,8 +655,7 @@ 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 @@ -792,14 +778,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( @@ -830,17 +816,9 @@ 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)) @@ -896,7 +874,8 @@ 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 = [] diff --git a/THIRD_PARTY_WEBRTC_DEPS.json b/THIRD_PARTY_WEBRTC_DEPS.json index 53d45ee913..2f2190f910 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 6c0b0c48a1..4b2b3c7522 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 @@ 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): - 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 @@ class MockFile(object): 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 5ef70b5eff..0000000000 --- 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 1a685ca5a9..0000000000 --- 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 e69de29bb2..0000000000 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 e69de29bb2..0000000000 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 e69de29bb2..0000000000