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}diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 37047d4..aebadb5 100755
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -11,6 +11,7 @@
import re
import subprocess
import sys
+from collections import defaultdict
# Files and directories that are *skipped* by cpplint in the presubmit script.
@@ -67,6 +68,7 @@
'modules/audio_device/include',
'pc',
)
+
# These directories should not be used but are maintained only to avoid breaking
# some legacy downstream code.
LEGACY_API_DIRS = (
@@ -90,8 +92,22 @@
'system_wrappers/include',
'voice_engine/include',
)
+
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):
"""Runs a command and returns the output from that command."""
@@ -297,33 +313,48 @@
items=violating_gn_files)]
return []
-def CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
- # Disallow mixing .c and .cc source files in the same target.
- source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
- re.MULTILINE | re.DOTALL)
- file_pattern = input_api.re.compile(r'"(.*)"')
- violating_gn_files = dict()
+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:
+ if len(source_list):
+ sources_used += 1
+ return sources_used > 1
+
+ errors = defaultdict(lambda: [])
for gn_file in gn_files:
- contents = input_api.ReadFile(gn_file)
- for source_block_match in source_pattern.finditer(contents):
+ gn_file_content = input_api.ReadFile(gn_file)
+ for target_match in TARGET_RE.finditer(gn_file_content):
c_files = []
cc_files = []
- for file_list_match in file_pattern.finditer(source_block_match.group(1)):
- source_file = file_list_match.group(1)
- if source_file.endswith('.c'):
- c_files.append(source_file)
- if source_file.endswith('.cc'):
- cc_files.append(source_file)
- if c_files and cc_files:
- violating_gn_files[gn_file.LocalPath()] = sorted(c_files + cc_files)
- if violating_gn_files:
+ objc_files = []
+ target_name = target_match.group('target_name')
+ target_contents = target_match.group('target_contents')
+ for sources_match in SOURCES_RE.finditer(target_contents):
+ for file_match in FILE_PATH_RE.finditer(sources_match.group(1)):
+ file_path = file_match.group('file_path')
+ extension = file_match.group('extension')
+ if extension == '.c':
+ c_files.append(file_path + extension)
+ 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(
- 'GN targets cannot mix .cc and .c source files. Please create a '
- 'separate target for each collection of sources.\n'
+ 'GN targets cannot mix .c, .cc and .m (or .mm) source files.\n'
+ 'Please create a separate target for each collection of sources.\n'
'Mixed sources: \n'
'%s\n'
- 'Violating GN files:' % json.dumps(violating_gn_files, indent=2),
- items=violating_gn_files.keys())]
+ 'Violating GN files:\n%s\n' % (json.dumps(errors, indent=2),
+ '\n'.join(errors.keys())))]
return []
def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
@@ -350,9 +381,9 @@
result = []
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))
+ result.extend(CheckNoMixingSources(input_api, gn_files, output_api))
+ result.extend(CheckNoPackageBoundaryViolations(input_api, gn_files,
+ output_api))
return result
def CheckUnwantedDependencies(input_api, output_api):
diff --git a/presubmit_test.py b/presubmit_test.py
index f17a20f..5e342ba 100755
--- a/presubmit_test.py
+++ b/presubmit_test.py
@@ -60,7 +60,7 @@
shutil.rmtree(self.tmp_dir, ignore_errors=True)
def testErrorIfProtoFileDoesNotEndWithNewline(self):
- self.__GenerateProtoWithoutNewlineAtTheEnd()
+ self._GenerateProtoWithoutNewlineAtTheEnd()
self.input_api.files = [MockFile(self.proto_file_path)]
errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
self.output_api)
@@ -70,13 +70,13 @@
str(errors[0]))
def testNoErrorIfProtoFileEndsWithNewline(self):
- self.__GenerateProtoWithNewlineAtTheEnd()
+ self._GenerateProtoWithNewlineAtTheEnd()
self.input_api.files = [MockFile(self.proto_file_path)]
errors = PRESUBMIT.CheckNewlineAtTheEndOfProtoFiles(self.input_api,
self.output_api)
self.assertEqual(0, len(errors))
- def __GenerateProtoWithNewlineAtTheEnd(self):
+ def _GenerateProtoWithNewlineAtTheEnd(self):
with open(self.proto_file_path, 'w') as f:
f.write(textwrap.dedent("""
syntax = "proto2";
@@ -84,7 +84,7 @@
package webrtc.audioproc;
"""))
- def __GenerateProtoWithoutNewlineAtTheEnd(self):
+ def _GenerateProtoWithoutNewlineAtTheEnd(self):
with open(self.proto_file_path, 'w') as f:
f.write(textwrap.dedent("""
syntax = "proto2";
@@ -92,5 +92,72 @@
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__':
unittest.main()
diff --git a/presubmit_test_mocks.py b/presubmit_test_mocks.py
index f7ead25..cbfc4f5 100644
--- a/presubmit_test_mocks.py
+++ b/presubmit_test_mocks.py
@@ -25,6 +25,15 @@
# pylint: disable=unused-argument
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):
"""Mock class for the OutputApi class.
@@ -71,3 +80,6 @@
def LocalPath(self):
return self._local_path
+
+ def AbsoluteLocalPath(self):
+ return self._local_path
