admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Adding PRESUBMIT check to avoid mixing C, C++ and Objc-C/Obj-C++."

Browse Source 
This reverts commit 0c15c5332fea2bbf5fe29dd806f9f4e606eeb9b8.

Reason for revert: This causes problems in this moment. I have to fix a target in rtc_base before landing this presubmit check.

Original change's description:
> Adding PRESUBMIT check to avoid mixing C, C++ and Objc-C/Obj-C++.
> 
> The error message will be something like:
> 
> GN targets cannot mix .c (or .cc) and .m (or .mm) source files.
> Please create a separate target for each collection of sources.
> Mixed sources:
> {
>   BUILD_GN_PATH: [
>     [
>       TARGET_NAME,
>       [
>         SOURCES
>       ]
>     ],
>     ...
>   ],
>   ...
> }
> 
> Bug: webrtc:7743
> Change-Id: I45dd2c621b830e5aeb081fa8d17c9497a49c2554
> Reviewed-on: https://webrtc-review.googlesource.com/1980
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Henrik Kjellander <kjellander@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#19897}

TBR=kjellander@webrtc.org,mbonadei@webrtc.org

Change-Id: I73ff609b0140719473afd36ead1632e5cc3b41f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:7743
Reviewed-on: https://webrtc-review.googlesource.com/2180
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19898}
This commit is contained in:
Mirko Bonadei 2017-09-19 10:54:24 +00:00 committed by Commit Bot
parent 0c15c5332f
commit 034a6b8a4c
3 changed files with 28 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
PRESUBMIT.py
View File

@ -11,7 +11,6 @@ import os
import re import re
import subprocess import subprocess
import sys import sys
from collections import defaultdict
# Files and directories that are *skipped* by cpplint in the presubmit script. # Files and directories that are *skipped* by cpplint in the presubmit script.
@ -68,7 +67,6 @@ NATIVE_API_DIRS = (
'modules/audio_device/include', 'modules/audio_device/include',
'pc', 'pc',
) )
# These directories should not be used but are maintained only to avoid breaking # These directories should not be used but are maintained only to avoid breaking
# some legacy downstream code. # some legacy downstream code.
LEGACY_API_DIRS = ( LEGACY_API_DIRS = (
@ -92,22 +90,8 @@ LEGACY_API_DIRS = (
'system_wrappers/include', 'system_wrappers/include',
'voice_engine/include', 'voice_engine/include',
) )
API_DIRS = NATIVE_API_DIRS[:] + LEGACY_API_DIRS[:] API_DIRS = NATIVE_API_DIRS[:] + LEGACY_API_DIRS[:]
# TARGET_RE matches a GN target, and extracts the target name and the contents.
TARGET_RE = re.compile(r'(?P<indent>\s*)\w+\("(?P<target_name>\w+)"\) {'
r'(?P<target_contents>.*?)'
r'(?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)
# FILE_PATH_RE matchies a file path.
FILE_PATH_RE = re.compile(r'"(?P<file_path>(\w|\/)+)(?P<extension>\.\w+)"')
def _RunCommand(command, cwd): def _RunCommand(command, cwd):
"""Runs a command and returns the output from that command.""" """Runs a command and returns the output from that command."""
@ -313,48 +297,33 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api):
items=violating_gn_files)] items=violating_gn_files)]
return [] return []
def CheckNoMixingSources(input_api, gn_files, output_api): def CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
"""Disallow mixing C, C++ and Obj-C/Obj-C++ in the same target. # Disallow mixing .c and .cc source files in the same target.
source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
See bugs.webrtc.org/7743 for more context. re.MULTILINE | re.DOTALL)
""" file_pattern = input_api.re.compile(r'"(.*)"')
def _MoreThanOneSourceUsed(*sources_lists): violating_gn_files = dict()
sources_used = 0
for source_list in sources_lists:
if len(source_list):
sources_used += 1
return sources_used > 1
errors = defaultdict(lambda: [])
for gn_file in gn_files: for gn_file in gn_files:
gn_file_content = input_api.ReadFile(gn_file) contents = input_api.ReadFile(gn_file)
for target_match in TARGET_RE.finditer(gn_file_content): for source_block_match in source_pattern.finditer(contents):
c_files = [] c_files = []
cc_files = [] cc_files = []
objc_files = [] for file_list_match in file_pattern.finditer(source_block_match.group(1)):
target_name = target_match.group('target_name') source_file = file_list_match.group(1)
target_contents = target_match.group('target_contents') if source_file.endswith('.c'):
for sources_match in SOURCES_RE.finditer(target_contents): c_files.append(source_file)
for file_match in FILE_PATH_RE.finditer(sources_match.group(1)): if source_file.endswith('.cc'):
file_path = file_match.group('file_path') cc_files.append(source_file)
extension = file_match.group('extension') if c_files and cc_files:
if extension == '.c': violating_gn_files[gn_file.LocalPath()] = sorted(c_files + cc_files)
c_files.append(file_path + extension) if violating_gn_files:
if extension == '.cc':
cc_files.append(file_path + extension)
if extension in ['.m', '.mm']:
objc_files.append(file_path + extension)
if _MoreThanOneSourceUsed(c_files, cc_files, objc_files):
all_sources = sorted(c_files + cc_files + objc_files)
errors[gn_file.LocalPath()].append((target_name, all_sources))
if errors:
return [output_api.PresubmitError( return [output_api.PresubmitError(
'GN targets cannot mix .c, .cc and .m (or .mm) source files.\n' 'GN targets cannot mix .cc and .c source files. Please create a '
'Please create a separate target for each collection of sources.\n' 'separate target for each collection of sources.\n'
'Mixed sources: \n' 'Mixed sources: \n'
'%s\n' '%s\n'
'Violating GN files:\n%s\n' % (json.dumps(errors, indent=2), 'Violating GN files:' % json.dumps(violating_gn_files, indent=2),
'\n'.join(errors.keys())))] items=violating_gn_files.keys())]
return [] return []
def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api): def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
@ -381,9 +350,9 @@ def CheckGnChanges(input_api, output_api):
result = [] result = []
if gn_files: if gn_files:
result.extend(CheckNoSourcesAbove(input_api, gn_files, output_api)) result.extend(CheckNoSourcesAbove(input_api, gn_files, output_api))
result.extend(CheckNoMixingSources(input_api, gn_files, output_api)) result.extend(CheckNoMixingCAndCCSources(input_api, gn_files, output_api))
result.extend(CheckNoPackageBoundaryViolations(input_api, gn_files, result.extend(CheckNoPackageBoundaryViolations(
output_api)) input_api, gn_files, output_api))
return result return result
def CheckUnwantedDependencies(input_api, output_api): def CheckUnwantedDependencies(input_api, output_api):

75
presubmit_test.py
View File

@ -60,7 +60,7 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
shutil.rmtree(self.tmp_dir, ignore_errors=True) shutil.rmtree(self.tmp_dir, ignore_errors=True)
def testErrorIfProtoFileDoesNotEndWithNewline(self): def testErrorIfProtoFileDoesNotEndWithNewline(self):
self._GenerateProtoWithoutNewlineAtTheEnd() self.__GenerateProtoWithoutNewlineAtTheEnd()
self.input_api.files = [MockFile(self.proto_file_path)] self.input_api.files = [MockFile(self.proto_file_path)]
errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api, errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
self.output_api) self.output_api)
@ -70,13 +70,13 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
str(errors[0])) str(errors[0]))
def testNoErrorIfProtoFileEndsWithNewline(self): def testNoErrorIfProtoFileEndsWithNewline(self):
self._GenerateProtoWithNewlineAtTheEnd() self.__GenerateProtoWithNewlineAtTheEnd()
self.input_api.files = [MockFile(self.proto_file_path)] self.input_api.files = [MockFile(self.proto_file_path)]
errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api, errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
self.output_api) self.output_api)
self.assertEqual(0, len(errors)) self.assertEqual(0, len(errors))
def _GenerateProtoWithNewlineAtTheEnd(self): def __GenerateProtoWithNewlineAtTheEnd(self):
with open(self.proto_file_path, 'w') as f: with open(self.proto_file_path, 'w') as f:
f.write(textwrap.dedent(""" f.write(textwrap.dedent("""
syntax = "proto2"; syntax = "proto2";
@ -84,7 +84,7 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
package webrtc.audioproc; package webrtc.audioproc;
""")) """))
def _GenerateProtoWithoutNewlineAtTheEnd(self): def __GenerateProtoWithoutNewlineAtTheEnd(self):
with open(self.proto_file_path, 'w') as f: with open(self.proto_file_path, 'w') as f:
f.write(textwrap.dedent(""" f.write(textwrap.dedent("""
syntax = "proto2"; syntax = "proto2";
@ -92,72 +92,5 @@ class CheckNewlineAtTheEndOfProtoFilesTest(unittest.TestCase):
package webrtc.audioproc;""")) package webrtc.audioproc;"""))
class CheckNoMixingSourcesTest(unittest.TestCase):
def setUp(self):
self.tmp_dir = tempfile.mkdtemp()
self.file_path = os.path.join(self.tmp_dir, 'BUILD.gn')
self.input_api = MockInputApi()
self.output_api = MockOutputApi()
def tearDown(self):
shutil.rmtree(self.tmp_dir, ignore_errors=True)
def testErrorIfCAndCppAreMixed(self):
self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.cc', 'bar.h'])
def testErrorIfCAndObjCAreMixed(self):
self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.m', 'bar.h'])
def testErrorIfCAndObjCppAreMixed(self):
self._AssertNumberOfErrorsWithSources(1, ['foo.c', 'bar.mm', 'bar.h'])
def testErrorIfCppAndObjCAreMixed(self):
self._AssertNumberOfErrorsWithSources(1, ['foo.cc', 'bar.m', 'bar.h'])
def testErrorIfCppAndObjCppAreMixed(self):
self._AssertNumberOfErrorsWithSources(1, ['foo.cc', 'bar.mm', 'bar.h'])
def testNoErrorIfOnlyC(self):
self._AssertNumberOfErrorsWithSources(0, ['foo.c', 'bar.c', 'bar.h'])
def testNoErrorIfOnlyCpp(self):
self._AssertNumberOfErrorsWithSources(0, ['foo.cc', 'bar.cc', 'bar.h'])
def testNoErrorIfOnlyObjC(self):
self._AssertNumberOfErrorsWithSources(0, ['foo.m', 'bar.m', 'bar.h'])
def testNoErrorIfOnlyObjCpp(self):
self._AssertNumberOfErrorsWithSources(0, ['foo.mm', 'bar.mm', 'bar.h'])
def testNoErrorIfObjCAndObjCppAreMixed(self):
self._AssertNumberOfErrorsWithSources(0, ['foo.m', 'bar.mm', 'bar.h'])
def _AssertNumberOfErrorsWithSources(self, number_of_errors, sources):
assert 3 == len(sources), 'This function accepts a list of 3 source files'
self._GenerateBuildFile(textwrap.dedent("""
rtc_source_set("foo_bar") {
sources = [
"%s",
"%s",
"%s",
],
}
""" % tuple(sources)))
self.input_api.files = [MockFile(self.file_path)]
errors = PRESUBMIT.CheckNoMixingSources(self.input_api,
[MockFile(self.file_path)],
self.output_api)
self.assertEqual(number_of_errors, len(errors))
if number_of_errors == 1:
for source in sources:
if not source.endswith('.h'):
self.assertTrue(source in str(errors[0]))
def _GenerateBuildFile(self, content):
with open(self.file_path, 'w') as f:
f.write(content)
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()

12
presubmit_test_mocks.py
View File

@ -25,15 +25,6 @@ class MockInputApi(object):
# pylint: disable=unused-argument # pylint: disable=unused-argument
return self.files return self.files
def ReadFile(self, affected_file, mode='rU'):
filename = affected_file.AbsoluteLocalPath()
for f in self.files:
if f.LocalPath() == filename:
with open(filename, mode) as f:
return f.read()
# Otherwise, file is not in our mock API.
raise IOError, "No such file or directory: '%s'" % filename
class MockOutputApi(object): class MockOutputApi(object):
"""Mock class for the OutputApi class. """Mock class for the OutputApi class.
@ -80,6 +71,3 @@ class MockFile(object):
def LocalPath(self): def LocalPath(self):
return self._local_path return self._local_path
def AbsoluteLocalPath(self):
return self._local_path