diff options
-rwxr-xr-x | native_client_sdk/src/build_tools/tests/verify_ppapi_test.py | 194 | ||||
-rwxr-xr-x | native_client_sdk/src/build_tools/verify_ppapi.py | 186 | ||||
-rwxr-xr-x | native_client_sdk/src/test_all.py | 1 | ||||
-rw-r--r-- | ppapi/PRESUBMIT.py | 36 |
4 files changed, 417 insertions, 0 deletions
diff --git a/native_client_sdk/src/build_tools/tests/verify_ppapi_test.py b/native_client_sdk/src/build_tools/tests/verify_ppapi_test.py new file mode 100755 index 0000000..d770182 --- /dev/null +++ b/native_client_sdk/src/build_tools/tests/verify_ppapi_test.py @@ -0,0 +1,194 @@ +#!/usr/bin/env python +# Copyright (c) 2013 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import os +import sys +import unittest + +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) +BUILD_TOOLS_DIR = os.path.dirname(SCRIPT_DIR) +CHROME_SRC = os.path.dirname(os.path.dirname(os.path.dirname(BUILD_TOOLS_DIR))) +MOCK_DIR = os.path.join(CHROME_SRC, "third_party", "pymock") + +# For the mock library +sys.path.append(MOCK_DIR) +import mock + +sys.path.append(BUILD_TOOLS_DIR) +import verify_ppapi + + +class TestPartition(unittest.TestCase): + def testBasic(self): + filenames = [ + os.path.join('ppapi', 'c', 'ppb_foo.h'), + os.path.join('ppapi', 'cpp', 'foo.h'), + os.path.join('ppapi', 'cpp', 'foo.cc'), + ] + result = verify_ppapi.PartitionFiles(filenames) + self.assertTrue(filenames[0] in result['ppapi']) + self.assertTrue(filenames[1] in result['ppapi_cpp']) + self.assertTrue(filenames[2] in result['ppapi_cpp']) + self.assertEqual(0, len(result['ppapi_cpp_private'])) + + def testIgnoreDocumentation(self): + filenames = [ + os.path.join('ppapi', 'c', 'documentation', 'Doxyfile'), + os.path.join('ppapi', 'c', 'documentation', 'index.dox'), + os.path.join('ppapi', 'cpp', 'documentation', 'footer.html'), + ] + result = verify_ppapi.PartitionFiles(filenames) + self.assertEqual(0, len(result['ppapi'])) + self.assertEqual(0, len(result['ppapi_cpp'])) + self.assertEqual(0, len(result['ppapi_cpp_private'])) + + def testIgnoreTrusted(self): + filenames = [ + os.path.join('ppapi', 'c', 'trusted', 'ppb_broker_trusted.h'), + os.path.join('ppapi', 'c', 'trusted', 'ppb_file_io_trusted.h'), + os.path.join('ppapi', 'cpp', 'trusted', 'file_chooser_trusted.cc'), + ] + result = verify_ppapi.PartitionFiles(filenames) + self.assertEqual(0, len(result['ppapi'])) + self.assertEqual(0, len(result['ppapi_cpp'])) + self.assertEqual(0, len(result['ppapi_cpp_private'])) + + def testIgnoreIfNotSourceOrHeader(self): + filenames = [ + os.path.join('ppapi', 'c', 'DEPS'), + os.path.join('ppapi', 'c', 'blah', 'foo.xml'), + os.path.join('ppapi', 'cpp', 'DEPS'), + os.path.join('ppapi', 'cpp', 'foobar.py'), + ] + result = verify_ppapi.PartitionFiles(filenames) + self.assertEqual(0, len(result['ppapi'])) + self.assertEqual(0, len(result['ppapi_cpp'])) + self.assertEqual(0, len(result['ppapi_cpp_private'])) + + def testIgnoreOtherDirectories(self): + ignored_directories = ['api', 'examples', 'generators', 'host', 'lib', + 'native_client', 'proxy', 'shared_impl', 'tests', 'thunk'] + + # Generate some random files in the ignored directories. + filenames = [] + for dirname in ignored_directories: + filenames = os.path.join('ppapi', dirname, 'foo.cc') + filenames = os.path.join('ppapi', dirname, 'subdir', 'foo.h') + filenames = os.path.join('ppapi', dirname, 'DEPS') + + result = verify_ppapi.PartitionFiles(filenames) + self.assertEqual(0, len(result['ppapi'])) + self.assertEqual(0, len(result['ppapi_cpp'])) + self.assertEqual(0, len(result['ppapi_cpp_private'])) + + +class TestGetChangedAndRemoved(unittest.TestCase): + def testBasic(self): + modified_filenames = [ + os.path.join('ppapi', 'cpp', 'audio.cc'), + os.path.join('ppapi', 'cpp', 'graphics_2d.cc'), + os.path.join('ppapi', 'cpp', 'foobar.cc'), + os.path.join('ppapi', 'cpp', 'var.cc'), + ] + directory_list = [ + os.path.join('ppapi', 'cpp', 'audio.cc'), + os.path.join('ppapi', 'cpp', 'graphics_2d.cc'), + ] + changed, removed = verify_ppapi.GetChangedAndRemovedFilenames( + modified_filenames, directory_list) + self.assertTrue(modified_filenames[0] in changed) + self.assertTrue(modified_filenames[1] in changed) + self.assertTrue(modified_filenames[2] in removed) + self.assertTrue(modified_filenames[3] in removed) + + +class TestVerify(unittest.TestCase): + def testBasic(self): + dsc_filename = 'native_client_sdk/src/libraries/ppapi/library.dsc' + # The .dsc files typically uses basenames, not full paths. + dsc_sources_and_headers = [ + 'ppb_audio.h', + 'ppb_console.h', + 'ppb_gamepad.h', + 'ppb.h', + 'ppp_zoom_dev.h', + ] + changed_filenames = [ + os.path.join('ppapi', 'c', 'ppb_audio.h'), + os.path.join('ppapi', 'c', 'ppb_console.h'), + ] + removed_filenames = [] + # Should not raise. + verify_ppapi.Verify(dsc_filename, dsc_sources_and_headers, + changed_filenames, removed_filenames) + + # Raise, because we removed ppp_zoom_dev.h. + removed_filenames = [ + os.path.join('ppapi', 'c', 'ppb_console.h'), + ] + self.assertRaises(verify_ppapi.VerifyException, verify_ppapi.Verify, + dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames) + + # Raise, because we added ppb_foo.h. + removed_filenames = [] + changed_filenames = [ + os.path.join('ppapi', 'c', 'ppb_audio.h'), + os.path.join('ppapi', 'c', 'ppb_console.h'), + os.path.join('ppapi', 'c', 'ppb_foo.h'), + ] + self.assertRaises(verify_ppapi.VerifyException, verify_ppapi.Verify, + dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames) + + def testVerifyPrivate(self): + dsc_filename = \ + 'native_client_sdk/src/libraries/ppapi_cpp_private/library.dsc' + # The .dsc files typically uses basenames, not full paths. + dsc_sources_and_headers = [ + 'ext_crx_file_system_private.cc', + 'file_io_private.cc', + 'ppb_ext_crx_file_system_private.h', + 'ppb_file_io_private.h', + 'host_resolver_private.h', + 'net_address_private.h', + ] + changed_filenames = [ + os.path.join('ppapi', 'c', 'private', 'ppb_foo_private.h'), + ] + removed_filenames = [] + + with mock.patch('sys.stderr') as sys_stderr: + # When a new private file is added, just print to stderr, but don't fail. + result = verify_ppapi.VerifyOrPrintError( + dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames, is_private=True) + self.assertTrue(result) + self.assertTrue(sys_stderr.write.called) + + # If is_private is False, then adding a new interface without updating the + # .dsc is an error. + sys_stderr.reset_mock() + result = verify_ppapi.VerifyOrPrintError( + dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames, is_private=False) + self.assertFalse(result) + self.assertTrue(sys_stderr.write.called) + + # Removing a file without updating the .dsc is always an error. + sys_stderr.reset_mock() + changed_filenames = [] + removed_filenames = [ + os.path.join('ppapi', 'c', 'private', 'net_address_private.h'), + ] + result = verify_ppapi.VerifyOrPrintError( + dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames, is_private=True) + self.assertFalse(result) + self.assertTrue(sys_stderr.write.called) + + +if __name__ == '__main__': + unittest.main() diff --git a/native_client_sdk/src/build_tools/verify_ppapi.py b/native_client_sdk/src/build_tools/verify_ppapi.py new file mode 100755 index 0000000..8a51e3c --- /dev/null +++ b/native_client_sdk/src/build_tools/verify_ppapi.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python +# Copyright (c) 2013 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""Helper script for PPAPI's PRESUBMIT.py to detect if additions or removals of +PPAPI interfaces have been propagated to the Native Client libraries (.dsc +files). + +For example, if a user adds "ppapi/c/foo.h", we check that the interface has +been added to "native_client_sdk/src/libraries/ppapi/library.dsc". +""" + +import optparse +import os +import sys + +from build_paths import PPAPI_DIR, SRC_DIR, SDK_LIBRARY_DIR +import parse_dsc + + +class VerifyException(Exception): + def __init__(self, lib_path, expected, unexpected): + self.expected = expected + self.unexpected = unexpected + + msg = 'In %s:\n' % lib_path + if expected: + msg += ' these files are missing and should be added:\n' + for filename in sorted(expected): + msg += ' %s\n' % filename + if unexpected: + msg += ' these files no longer exist and should be removed:\n' + for filename in sorted(unexpected): + msg += ' %s\n' % filename + + Exception.__init__(self, msg) + + +def PartitionFiles(filenames): + c_filenames = set() + cpp_filenames = set() + private_filenames = set() + + for filename in filenames: + if os.path.splitext(filename)[1] not in ('.cc', '.h'): + continue + + parts = filename.split(os.sep) + if 'private' in filename: + if 'flash' in filename: + continue + private_filenames.add(filename) + elif parts[0:2] == ['ppapi', 'c']: + if len(parts) >= 2 and parts[2] in ('documentation', 'trusted'): + continue + c_filenames.add(filename) + elif (parts[0:2] == ['ppapi', 'cpp'] or + parts[0:2] == ['ppapi', 'utility']): + if len(parts) >= 2 and parts[2] in ('documentation', 'trusted'): + continue + cpp_filenames.add(filename) + else: + continue + + return { + 'ppapi': c_filenames, + 'ppapi_cpp': cpp_filenames, + 'ppapi_cpp_private': private_filenames + } + + +def GetDirectoryList(directory_path, relative_to): + result = [] + for root, _, files in os.walk(directory_path): + rel_root = os.path.relpath(root, relative_to) + if rel_root == '.': + rel_root = '' + for base_name in files: + result.append(os.path.join(rel_root, base_name)) + return result + + +def GetDscSourcesAndHeaders(dsc): + result = [] + for headers_info in dsc.get('HEADERS', []): + result.extend(headers_info['FILES']) + for targets_info in dsc.get('TARGETS', []): + result.extend(targets_info['SOURCES']) + return result + + +def GetChangedAndRemovedFilenames(modified_filenames, directory_list): + changed = set() + removed = set() + directory_list_set = set(directory_list) + for filename in modified_filenames: + if filename in directory_list_set: + # We can't know if a file was added (that would require knowing the + # previous state of the working directory). Instead, we assume that a + # changed file may have been added, and check it accordingly. + changed.add(filename) + else: + removed.add(filename) + return changed, removed + + +def GetDscFilenameFromLibraryName(lib_name): + return os.path.join(SDK_LIBRARY_DIR, lib_name, 'library.dsc') + + +def Verify(dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames): + expected_filenames = set() + unexpected_filenames = set() + + for filename in changed_filenames: + basename = os.path.basename(filename) + if basename not in dsc_sources_and_headers: + expected_filenames.add(filename) + + for filename in removed_filenames: + basename = os.path.basename(filename) + if basename in dsc_sources_and_headers: + unexpected_filenames.add(filename) + + if expected_filenames or unexpected_filenames: + raise VerifyException(dsc_filename, expected_filenames, + unexpected_filenames) + + +def VerifyOrPrintError(dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames, is_private=False): + try: + Verify(dsc_filename, dsc_sources_and_headers, changed_filenames, + removed_filenames) + except VerifyException as e: + should_fail = True + if is_private and e.expected: + # For ppapi_cpp_private, we don't fail if there are expected filenames... + # we may not want to include them. We still want to fail if there are + # unexpected filenames, though. + sys.stderr.write('>>> WARNING: private interface files changed. ' + 'Should they be added to the Native Client SDK? <<<\n') + if not e.unexpected: + should_fail = False + sys.stderr.write(str(e) + '\n') + if should_fail: + return False + return True + + +def main(args): + usage = '%prog <file>...' + description = __doc__ + parser = optparse.OptionParser(usage=usage, description=description) + args = parser.parse_args(args)[1] + if not args: + parser.error('Expected a PPAPI header or source file.') + + retval = 0 + lib_files = PartitionFiles(args) + directory_list = GetDirectoryList(PPAPI_DIR, relative_to=SRC_DIR) + for lib_name, filenames in lib_files.iteritems(): + if not filenames: + continue + + changed_filenames, removed_filenames = \ + GetChangedAndRemovedFilenames(filenames, directory_list) + + dsc_filename = GetDscFilenameFromLibraryName(lib_name) + dsc = parse_dsc.LoadProject(dsc_filename) + dsc_sources_and_headers = GetDscSourcesAndHeaders(dsc) + + # Use the relative path to the .dsc to make the error messages shorter. + rel_dsc_filename = os.path.relpath(dsc_filename, SRC_DIR) + is_private = lib_name == 'ppapi_cpp_private' + if not VerifyOrPrintError(rel_dsc_filename, dsc_sources_and_headers, + changed_filenames, removed_filenames, + is_private=is_private): + retval = 1 + return retval + + +if __name__ == '__main__': + sys.exit(main(sys.argv[1:])) diff --git a/native_client_sdk/src/test_all.py b/native_client_sdk/src/test_all.py index 5e9be95..a13bf80 100755 --- a/native_client_sdk/src/test_all.py +++ b/native_client_sdk/src/test_all.py @@ -28,6 +28,7 @@ TEST_MODULES = [ 'sel_ldr_test', 'update_nacl_manifest_test', 'verify_filelist_test', + 'verify_ppapi_test', ] def main(): diff --git a/ppapi/PRESUBMIT.py b/ppapi/PRESUBMIT.py index 72672e9..0ae9b0e 100644 --- a/ppapi/PRESUBMIT.py +++ b/ppapi/PRESUBMIT.py @@ -110,6 +110,40 @@ def CheckUnversionedPPB(input_api, output_api): long_text='\n'.join(todo))] return [] +# Verify that changes to ppapi headers/sources are also made to NaCl SDK. +def CheckUpdatedNaClSDK(input_api, output_api): + files = input_api.LocalPaths() + + # PPAPI files the Native Client SDK cares about. + nacl_sdk_files = [] + + for filename in files: + name, ext = os.path.splitext(filename) + name_parts = name.split(os.sep) + + if len(name_parts) <= 2: + continue + + if name_parts[0] != 'ppapi': + continue + + if ((name_parts[1] == 'c' and ext == '.h') or + (name_parts[1] in ('cpp', 'utility') and ext in ('.h', '.cc'))): + if name_parts[2] in ('documentation', 'trusted'): + continue + nacl_sdk_files.append(filename) + + if not nacl_sdk_files: + return [] + + verify_ppapi_py = os.path.join(input_api.change.RepositoryRoot(), + 'native_client_sdk', 'src', 'build_tools', + 'verify_ppapi.py') + cmd = [sys.executable, verify_ppapi_py] + nacl_sdk_files + return RunCmdAndCheck(cmd, + 'PPAPI Interface modified without updating NaCl SDK.', + output_api) + def CheckChange(input_api, output_api): results = [] @@ -119,6 +153,8 @@ def CheckChange(input_api, output_api): results.extend(CheckUnversionedPPB(input_api, output_api)) + results.extend(CheckUpdatedNaClSDK(input_api, output_api)) + # Verify all modified *.idl have a matching *.h files = input_api.LocalPaths() h_files = [] |