[bindings] partial interfaces should not violate componentization

Source/bindings/scripts/aggregate_generated_bindings.py

Index: Source/bindings/scripts/aggregate_generated_bindings.py
diff --git a/Source/bindings/scripts/aggregate_generated_bindings.py b/Source/bindings/scripts/aggregate_generated_bindings.py
index 5efb42c9ea0edd46e2750bf8e03aa6655672e3ae..9623be297f5d627f422084c8b050bf69d1bddf43 100755
--- a/Source/bindings/scripts/aggregate_generated_bindings.py
+++ b/Source/bindings/scripts/aggregate_generated_bindings.py
@@ -37,23 +37,18 @@
 This can be a single output file, to preserve symbol space; or multiple output
 files, to reduce maximum compilation unit size and allow parallel compilation.
 
-Usage:
-aggregate_generated_bindings.py COMPONENT_DIR IDL_FILES_LIST -- OUTPUT_FILE1 OUTPUT_FILE2 ...
-
-COMPONENT_DIR is the relative directory of a component, e.g., 'core', 'modules'.
-IDL_FILES_LIST is a text file containing the IDL file paths, so the command
-line doesn't exceed OS length limits.
-OUTPUT_FILE1 etc. are filenames of output files.
-
 Design doc: http://www.chromium.org/developers/design-documents/idl-build
 """
 
+import cPickle as pickle
 import errno
+from optparse import OptionParser
 import os
 import re
 import sys
 
-from utilities import idl_filename_to_interface_name, read_idl_files_list_from_file
+from idl_reader import IdlReader
+from utilities import idl_filename_to_component, idl_filename_to_interface_name, read_idl_files_list_from_file
 
 # A regexp for finding Conditional attributes in interface definitions.
 CONDITIONAL_PATTERN = re.compile(
@@ -100,6 +95,46 @@ COPYRIGHT_TEMPLATE = """/*
 """
 
 
+def parse_options():
+    usage = 'Usage: %prog [options] OUTPUT_FILE1 OUTPUT_FILE2 ...'
+    parser = OptionParser(usage=usage)
+    parser.add_option('--cache-directory',
+                      help='cache directory, defaults to output directory')
+    parser.add_option('--idl-files-list', help="a text file containing the IDL file paths, so the command line doesn't exceed OS length limits.")
+    parser.add_option('--component-dir', help="the relative directory of a component, e.g., 'core', 'modules'")
+    parser.add_option('--partial', type='int', help='if true, aggregates partial interface code, e.g. V8XXXPartial.cpp')

[bashi, 2014/10/10 09:48:11]

--partial -> --aggregate-partial-interfaces

How about using a boolean flag option instead int?

parser.add_option('--partial', help='...', action='store_true', default=False)

[tasak, 2014/10/10 11:43:40]

Removed this flag.

+    parser.add_option('--interfaces-info')
+    parser.add_option('--write-file-only-if-changed', type='int')
+    # ensure output comes last, so command line easy to parse via regexes
+    parser.disable_interspersed_args()
+
+    options, args = parser.parse_args()
+    if options.idl_files_list is None:
+        parser.error('Must specify idl files list file using --idl-files-list.')
+    if options.component_dir is None:
+        parser.error('Must specify component directory using --component-dir.')
+    if options.partial:
+        if options.interfaces_info is None:
+            parser.error('Must specify interfaces info using --interfaces-info-file because of --partial.')
+        options.partial = True
+    else:
+        options.partial = False
+    options.write_file_only_if_changed = bool(options.write_file_only_if_changed)
+    if len(args) == 0:
+        parser.error('Must specify exactly more than 1 output file as argument, but %d given.' % len(args))

[bashi, 2014/10/10 09:48:11]

'Must specify one or more output files...' ?

[tasak, 2014/10/10 11:43:40]

Done.

+    return options, args[0:]
+
+
+def format_conditional(conditional):
+    """Wraps conditional with ENABLE() and replace '&','|' with '&&','||' if
+    more than one conditional is specified."""
+    def wrap_with_enable(s):
+        if s in ['|', '&']:
+            return s * 2
+        return 'ENABLE(' + s + ')'
+    return ' '.join(map(wrap_with_enable, conditional))
+
+
 def extract_conditional(idl_file_path):
     """Find [Conditional] interface extended attribute."""
     with open(idl_file_path) as idl_file:
@@ -135,7 +170,7 @@ def extract_meta_data(file_paths):
     return meta_data_list
 
 
-def generate_content(component_dir, files_meta_data_this_partition):
+def generate_content(component_dir, partial, files_meta_data_this_partition):
     # Add fixed content.
     output = [COPYRIGHT_TEMPLATE,
               '#define NO_IMPLICIT_ATOMICSTRING\n\n']
@@ -152,8 +187,8 @@ def generate_content(component_dir, files_meta_data_this_partition):
                 output.append('\n#if ENABLE(%s)\n' % conditional)
         prev_conditional = conditional
 
-        output.append('#include "bindings/%s/v8/V8%s.cpp"\n' %
-                      (component_dir, meta_data['name']))
+        output.append('#include "bindings/%s/v8/V8%s%s.cpp"\n' %
+                      (component_dir, meta_data['name'], 'Partial' if partial else ''))
 
     if prev_conditional:
         output.append('#endif\n')
@@ -170,22 +205,39 @@ def write_content(content, output_file_name):
         f.write(content)
 
 
+def filter_partial_interface(idl_filenames, interfaces_info, cache_directory):
+    reader = IdlReader(interfaces_info, cache_directory)
+
+    patched_idl_filenames = []
+    for idl_filename in idl_filenames:
+        interface_name = idl_filename_to_interface_name(idl_filename)
+        component = idl_filename_to_component(idl_filename)

[bashi, 2014/10/10 09:48:11]

Can |component| be different from options.component_dir?

[tasak, 2014/10/10 11:43:40]

Yes.
To aggregate partial interface code, idl_filename is defined in
core_idl_with_modules_dependency_files, but component_dir = modules.

+        definitions = reader.read_idl_definitions(idl_filename)
+        if len(definitions.keys()) == 1 and definitions[component]:

[bashi, 2014/10/10 09:48:11]

Could you elaborate on this condition?

[tasak, 2014/10/10 11:43:40]

Removed.
Instead, added "raise exception" when a given interface has no partial
interfaces.

+            continue
+
+        patched_idl_filenames.append(idl_filename)
+
+    return patched_idl_filenames
+
+
 def main(args):
-    if len(args) <= 4:
-        raise Exception('Expected at least 5 arguments.')
-    component_dir = args[1]
-    input_file_name = args[2]
-    in_out_break_index = args.index('--')
-    output_file_names = args[in_out_break_index + 1:]
-
-    idl_file_names = read_idl_files_list_from_file(input_file_name)
+    options, output_file_names = parse_options()
+
+    idl_file_names = read_idl_files_list_from_file(options.idl_files_list)
+    if options.partial:
+        with open(options.interfaces_info) as interfaces_info_file:
+            interfaces_info = pickle.load(interfaces_info_file)
+        idl_file_names = filter_partial_interface(idl_file_names, interfaces_info, options.cache_directory)
+
     files_meta_data = extract_meta_data(idl_file_names)
     total_partitions = len(output_file_names)
     for partition, file_name in enumerate(output_file_names):
         files_meta_data_this_partition = [
                 meta_data for meta_data in files_meta_data
                 if hash(meta_data['name']) % total_partitions == partition]
-        file_contents = generate_content(component_dir,
+        file_contents = generate_content(options.component_dir,
+                                         options.partial,
                                          files_meta_data_this_partition)
         write_content(file_contents, file_name)