Support partial interface for test support idls

Source/bindings/scripts/compute_dependencies.py

 #!/usr/bin/python
 #
 # Copyright (C) 2013 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
@@ skipping 30 matching lines @@
 
 
 class IdlInterfaceFileNotFoundError(Exception):
     """Raised if the IDL file implementing an interface cannot be found."""
     pass
 
 
 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')
     parser.add_option('--dedicatedworkerglobalscope-constructors-file', help='output file')
     parser.add_option('--write-file-only-if-changed', type='int', help='if true, do not write an output file if it would be identical to the existing one, which avoids unnecessary rebuilds in ninja')
     options, args = parser.parse_args()
     if options.event_names_file is None:
         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:
         parser.error('Must specify an output file using --workerglobalscope-constructors-file.')
     if options.workerglobalscope_constructors_file is None:
         parser.error('Must specify an output file using --sharedworkerglobalscope-constructors-file.')
     if options.workerglobalscope_constructors_file is None:
         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)
     if args:
         parser.error('No arguments taken, but "%s" given.' % ' '.join(args))
     return options
 
 
 def get_file_contents(idl_filename):
     with open(idl_filename) as idl_file:
@@ skipping 110 matching lines @@
 
 
 def generate_global_constructors_partial_interface(interface_name, destination_filename, constructor_attributes_list, only_if_changed):
     lines = (['partial interface %s {\n' % interface_name] +
              ['    %s;\n' % constructor_attribute
               for constructor_attribute in sorted(constructor_attributes_list)] +
              ['};\n'])
     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
             generated. This does not include IDL files for interfaces
             implemented by another interface.
         global_constructors:
             dict of global objects -> list of constructors on that object
         event_names:
             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 = {}
     implemented_somewhere = set()
 
     global_constructors = {}
     for global_object in global_constructors_filenames.keys():
         global_constructors[global_object] = []
 
     # Parents and extended attributes (of interfaces with parents) are
     # used in generating event names
     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)
 
         # Handle partial interfaces
         partial_interface_name = get_partial_interface_name_from_idl(idl_file_contents)
         if partial_interface_name:
             partial_interface_files[partial_interface_name].append(full_path)
             continue
 
         interfaces.add(interface_name)
         # Non-partial interfaces default to having bindings generated
         dependencies[full_path] = []
         extended_attributes = get_interface_extended_attributes_from_idl(idl_file_contents)
 
         # Parse 'identifier-A implements identifier-B;' statements
         implemented_interfaces = get_implemented_interfaces_from_idl(idl_file_contents, interface_name)
         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)
+        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
+            # 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():
         if global_object in interfaces:
             partial_interface_files[global_object].append(filename)
 
     # Interfaces that are implemented by another interface do not have
     # their own bindings generated, as this would be redundant with the
     # actual implementation.
     for implemented_interface in implemented_somewhere:
         full_path = interface_name_to_idl_file[implemented_interface]
         del dependencies[full_path]
 
     # 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:
             interface_paths = map(lambda x: interface_name_to_idl_file[x], implemented_interfaces)
         except KeyError as key_name:
             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 = {}
     if 'Event' in interfaces:
         event_names[interface_name_to_idl_file['Event']] = interface_extended_attribute['Event']
     for interface, parent in parent_interface.iteritems():
         while parent in parent_interface:
             parent = parent_interface[parent]
         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):
     """Write the interface dependencies file.
 
     The format is as follows:
 
     Document.idl P.idl
     Event.idl
     Window.idl Q.idl R.idl S.idl
@@ skipping 11 matching lines @@
              for idl_file, dependency_files in sorted(dependencies.iteritems())]
     write_file(lines, filename, only_if_changed)
 
 
 def main():
     options = parse_options()
     idl_files = []
     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,
         'WorkerGlobalScope': options.workerglobalscope_constructors_file,
         'SharedWorkerGlobalScope': options.sharedworkerglobalscope_constructors_file,
         '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)
     generate_event_names_file(options.event_names_file, event_names, only_if_changed)
 
 
 if __name__ == '__main__':
     main()