Support partial interface for test support idls

Source/bindings/scripts/compute_dependencies.py

Index: Source/bindings/scripts/compute_dependencies.py
diff --git a/Source/bindings/scripts/compute_dependencies.py b/Source/bindings/scripts/compute_dependencies.py
index 816b5b237dab754b87f9915967abad33fb62d5bd..31a9a59c776a889be31d6bc710b65efb94bdf8e4 100755
--- a/Source/bindings/scripts/compute_dependencies.py
+++ b/Source/bindings/scripts/compute_dependencies.py
@@ -49,7 +49,9 @@ def parse_options():
     parser = optparse.OptionParser()
     parser.add_option('--event-names-file', help='output file')
     parser.add_option('--idl-files-list', help='file listing all IDLs')

[Nils Barth (inactive), 2013/09/17 02:23:35]

Could you update the comment here and for --testing-idl-files-list?
(They're not all IDL files.)
Also, since --idl-files-list is no longer *all* IDL files, perhaps it should be
renamed to --bindings-idl-files-list or something?

+    parser.add_option('--testing-idl-files-list', help='file listing all IDLs')

[haraken, 2013/09/17 03:36:52]

I'd rename:

  --idl-files-list => --core-idl-files-list
  --testing-idl-files-list => --noncore-idl-files-list

The point is to make it clear that we treat core IDL files specially (i.e. we
have to aggregate generated code using action_derivedsourceallinone.py) because
there are too many IDL files.

[Nils Barth (inactive), 2013/09/17 05:17:46]

Agreed, this naming makes the purposes clear.
However, 'core' is potentially confusing with Source/core ('core' instead of
'modules'??)
--main-idl-files-list?

[kihong, 2013/09/17 06:33:04]

We don't need to think core indicate Source/core but still core-idl/noncore-idl
makes confuse because of noncore-idl.
IMHO, noncore-idl looks like including idls which are in the modules.

core-idl / test-idl
main-idl / assistance-idl

If there is no objection, I would like to use core-idl/test-idl.

[haraken, 2013/09/17 06:42:06]

We're planning to use --testing-idl-files not only for testing/ but for
EventSender bindings and other bindings in Blink. So, what we really want is
core-and-modules-idl-files/other-idl-files.

If you don't like core-idl-files/noncore-idl-files (because of the presence of
modules/), maybe you can use core-idl-files/assistance-idl-files?

[Nils Barth (inactive), 2013/09/17 06:47:15]

Instead of "assistance", maybe "support"?
(More idiomatic.)

[kihong, 2013/10/04 06:11:38]

Done.

     parser.add_option('--interface-dependencies-file', help='output file')
+    parser.add_option('--binding-derived-source-file', help='output file')

[haraken, 2013/09/17 03:36:52]

--binding-derived-source-file => --core-derived-sources-file

Also, I'd be happy if you could rename all "derived sources" in the code base to
"generated code". It's more consistent to the IDL compiler terminology.

[Nils Barth (inactive), 2013/09/17 05:17:46]

Agreed that "derived sources" is pretty confusing.
AFAICT it refers not to the .cpp/.h generated by the IDL compiler, but to the
.cpp files which just *include* the generated code.
Derived sources is a separate step, and the IDL compiler itself doesn't know or
care about it.

So we have:
Node.idl - IDL file
V8Node.h/V8Node.cpp - generated code
V8DerivedSources01.cpp - "derived sources" (has #include "bindings/V8Node.cpp"
and others)

Maybe we can think of a better name to make the difference clearer, but we
shouldn't confuse these.

[kihong, 2013/09/17 06:33:04]

"--binding-derived-source-file" indicates files list for
bindings_derived_sources
Therefore how about --derived-source-list-file ?

     parser.add_option('--window-constructors-file', help='output file')
     parser.add_option('--workerglobalscope-constructors-file', help='output file')
     parser.add_option('--sharedworkerglobalscope-constructors-file', help='output file')
@@ -60,6 +62,8 @@ def parse_options():
         parser.error('Must specify an output file using --event-names-file.')
     if options.interface_dependencies_file is None:
         parser.error('Must specify an output file using --interface-dependencies-file.')
+    if options.binding_derived_source_file is None:
+        parser.error('Must specify an output file using --binding-derived-source-file.')
     if options.window_constructors_file is None:
         parser.error('Must specify an output file using --window-constructors-file.')
     if options.workerglobalscope_constructors_file is None:
@@ -70,6 +74,8 @@ def parse_options():
         parser.error('Must specify an output file using --dedicatedworkerglobalscope-constructors-file.')
     if options.idl_files_list is None:
         parser.error('Must specify the file listing all IDLs using --idl-files-list.')
+    if options.testing_idl_files_list is None:
+        parser.error('Must specify the file listing all IDLs using --testing-idl-files-list.')
     if options.write_file_only_if_changed is None:
         parser.error('Must specify whether file is only written if changed using --write-file-only-if-changed.')
     options.write_file_only_if_changed = bool(options.write_file_only_if_changed)
@@ -200,12 +206,14 @@ def generate_global_constructors_partial_interface(interface_name, destination_f
     write_file(lines, destination_filename, only_if_changed)
 
 
-def parse_idl_files(idl_files, global_constructors_filenames):
+def parse_idl_files(idl_files, testing_idl_files, global_constructors_filenames):

[Nils Barth (inactive), 2013/09/17 02:23:35]

Could you rename 'idl_files' to something like 'bindings_idl_files', since it's
no longer all IDL files?

[kihong, 2013/10/04 06:11:38]

Done.

     """Return dependencies between IDL files, constructors on global objects, and events.
 
     Returns:
         interfaces:
             set of all interfaces
+        source_idls:
+            main IDL filename except test support IDLs
         dependencies:
             dict of main IDL filename (for a given interface) -> list of partial IDL filenames (for that interface)
             The keys (main IDL files) are the files for which bindings are
@@ -217,6 +225,7 @@ def parse_idl_files(idl_files, global_constructors_filenames):
             dict of interfaces that inherit from Event -> list of extended attributes for the interface
     """
     interfaces = set()
+    binding_derived_sources = {}
     dependencies = {}
     partial_interface_files = {}
     implements_interfaces = {}
@@ -231,14 +240,16 @@ def parse_idl_files(idl_files, global_constructors_filenames):
     parent_interface = {}
     interface_extended_attribute = {}
 
+    dependencies_idl_files = idl_files + testing_idl_files
+
     interface_name_to_idl_file = {}
-    for idl_file_name in idl_files:
+    for idl_file_name in dependencies_idl_files:
         full_path = os.path.realpath(idl_file_name)
         interface_name, _ = os.path.splitext(os.path.basename(idl_file_name))
         interface_name_to_idl_file[interface_name] = full_path
         partial_interface_files[interface_name] = []
 
-    for idl_file_name in idl_files:
+    for idl_file_name in dependencies_idl_files:
         interface_name, _ = os.path.splitext(os.path.basename(idl_file_name))
         full_path = interface_name_to_idl_file[interface_name]
         idl_file_contents = get_file_contents(full_path)
@@ -259,20 +270,21 @@ def parse_idl_files(idl_files, global_constructors_filenames):
         implements_interfaces[interface_name] = implemented_interfaces
         implemented_somewhere |= set(implemented_interfaces)
 
-        # Record global constructors
-        if not is_callback_interface_from_idl(idl_file_contents) and 'NoInterfaceObject' not in extended_attributes:
-            global_contexts = extended_attributes.get('GlobalContext', 'Window').split('&')
-            new_constructor_list = generate_constructor_attribute_list(interface_name, extended_attributes)
-            for global_object in global_contexts:
-                global_constructors[global_object].extend(new_constructor_list)
-
-        # Record parents and extended attributes for generating event names
-        if interface_name == 'Event':
-            interface_extended_attribute[interface_name] = extended_attributes
-        parent = get_parent_interface(idl_file_contents)
-        if parent:
-            parent_interface[interface_name] = parent
-            interface_extended_attribute[interface_name] = extended_attributes
+        if idl_file_name not in testing_idl_files:

[Nils Barth (inactive), 2013/09/17 02:23:35]

This is kinda hacky:
it's clearer to iterate over two lists, rather than merging and then skipping.

Firstly, it's cleaner to do:
if idl_file_name in testing_idl_files:
    continue
...so you don't need to nest the second half of this.

Secondly, it would be nicer to do something like:
for idl_file_name in idl_files:
    ...do A
    ...do B

for idl_file_name in testing_idl_files:
    ...do A (only)

[Nils Barth (inactive), 2013/09/17 05:17:46]

BTW, to elaborate:
The code:
interface_name, _ = os.path.splitext(os.path.basename(idl_file_name))
...
implemented_somewhere |= set(implemented_interfaces)

...can be factored out to a nested function (compute_dependencies?) which takes
one argument, idl_file_name, and modifies the other variables in-place (you'll
need to change |= to .update). This avoids duplicating the code.

This function can then be called first for the (usual) idl_files_list, and then
(after doing other things like global constructors), for the testing_idl_files.

Does this make sense?

[kihong, 2013/10/04 06:11:38]

Done.

+            # Record global constructors
+            if not is_callback_interface_from_idl(idl_file_contents) and 'NoInterfaceObject' not in extended_attributes:
+                global_contexts = extended_attributes.get('GlobalContext', 'Window').split('&')
+                new_constructor_list = generate_constructor_attribute_list(interface_name, extended_attributes)
+                for global_object in global_contexts:
+                    global_constructors[global_object].extend(new_constructor_list)
+
+            # Record parents and extended attributes for generating event names
+            if interface_name == 'Event':
+                interface_extended_attribute[interface_name] = extended_attributes
+            parent = get_parent_interface(idl_file_contents)
+            if parent:
+                parent_interface[interface_name] = parent
+                interface_extended_attribute[interface_name] = extended_attributes
 
     # Add constructors on global objects to partial interfaces
     for global_object, filename in global_constructors_filenames.iteritems():
@@ -289,6 +301,7 @@ def parse_idl_files(idl_files, global_constructors_filenames):
     # An IDL file's dependencies are partial interface files that extend it,

[Nils Barth (inactive), 2013/09/17 02:23:35]

If you put this loop (and the global constructors etc.) *before* computing
dependencies for support files, then you don't need to add any logic to
specially skip support files, since they're not included yet.
Cleaner, eh?

     # and files for other interfaces that this interfaces implements.
     for idl_file_path in dependencies.iterkeys():
+        binding_derived_sources[idl_file_path] = []
         interface_name, _ = os.path.splitext(os.path.basename(idl_file_path))
         implemented_interfaces = implements_interfaces[interface_name]
         try:
@@ -297,6 +310,12 @@ def parse_idl_files(idl_files, global_constructors_filenames):
             raise IdlInterfaceFileNotFoundError('Could not find the IDL file where the following implemented interface is defined: %s' % key_name)
         dependencies[idl_file_path] = sorted(partial_interface_files[interface_name] + interface_paths)
 
+    # Remove test suport IDLs for derived_sources_all_in_one

[Nils Barth (inactive), 2013/09/17 02:23:35]

As above, this is hacky.
(Also, "suport" -> "support")
Rather than adding the support files and then removing them, could you not add
them in the first place?

+    for idl_file_name in testing_idl_files:
+        full_path = os.path.realpath(idl_file_name)
+        if full_path in binding_derived_sources.keys():
+            del binding_derived_sources[full_path]
+
     # Generate event names for all interfaces that inherit from Event,
     # including Event itself.
     event_names = {}
@@ -308,7 +327,7 @@ def parse_idl_files(idl_files, global_constructors_filenames):
         if parent == 'Event':
             event_names[interface_name_to_idl_file[interface]] = interface_extended_attribute[interface]
 
-    return interfaces, dependencies, global_constructors, event_names
+    return interfaces, binding_derived_sources, dependencies, global_constructors, event_names
 
 
 def write_dependency_file(filename, dependencies, only_if_changed):
@@ -340,6 +359,10 @@ def main():
     with open(options.idl_files_list) as idl_files_list_file:
         for line in idl_files_list_file:
             idl_files.append(string.rstrip(line, '\n'))
+    testing_idl_files = []
+    with open(options.testing_idl_files_list) as testing_idl_files_list_file:
+        for line in testing_idl_files_list_file:
+            testing_idl_files.append(string.rstrip(line, '\n'))
     only_if_changed = options.write_file_only_if_changed
     global_constructors_filenames = {
         'Window': options.window_constructors_file,
@@ -348,9 +371,10 @@ def main():
         'DedicatedWorkerGlobalScope': options.dedicatedworkerglobalscope_constructors_file,
         }
 
-    interfaces, dependencies, global_constructors, event_names = parse_idl_files(idl_files, global_constructors_filenames)
+    interfaces, binding_derived_sources, dependencies, global_constructors, event_names = parse_idl_files(idl_files, testing_idl_files, global_constructors_filenames)
 
     write_dependency_file(options.interface_dependencies_file, dependencies, only_if_changed)
+    write_dependency_file(options.binding_derived_source_file, binding_derived_sources, only_if_changed)
     for interface_name, filename in global_constructors_filenames.iteritems():
         if interface_name in interfaces:
             generate_global_constructors_partial_interface(interface_name, filename, global_constructors[interface_name], only_if_changed)