[mojo] Allow bindings generation with explicit relative include paths

mojo/public/tools/bindings/mojom_bindings_generator.py

Index: mojo/public/tools/bindings/mojom_bindings_generator.py
diff --git a/mojo/public/tools/bindings/mojom_bindings_generator.py b/mojo/public/tools/bindings/mojom_bindings_generator.py
index 5240dd7f4d2b0660e98794b2658fa8ce624114e8..5132b481327db66bf4c3b4ad3a316bb9ca35b632 100755
--- a/mojo/public/tools/bindings/mojom_bindings_generator.py
+++ b/mojo/public/tools/bindings/mojom_bindings_generator.py
@@ -49,6 +49,7 @@ _BUILTIN_GENERATORS = {
   "java": "mojom_java_generator.py",
 }
 
+
 def LoadGenerators(generators_string):
   if not generators_string:
     return []  # No generators.
@@ -77,12 +78,28 @@ def MakeImportStackMessage(imported_filename_stack):
                     zip(imported_filename_stack[1:], imported_filename_stack)]))
 
 
-def FindImportFile(dir_name, file_name, search_dirs):
-  for search_dir in [dir_name] + search_dirs:
-    path = os.path.join(search_dir, file_name)
+class RelativePath(object):
+  """Represents a path relative to the source tree."""
+  def __init__(self, path, source_root):
+    self.path = path
+    self.source_root = source_root
+
+  def relative_path(self):
+    return os.path.relpath(os.path.abspath(self.path),
+                           os.path.abspath(self.source_root))
+
+
+def FindImportFile(rel_dir, file_name, search_rel_dirs):
+  """Finds |file_name| in either |rel_dir| or |search_rel_dirs|. Returns a
+  RelativePath with first file found, or an arbitrary non-existent file
+  otherwise."""
+  for rel_search_dir in [rel_dir] + search_rel_dirs:
+    path = os.path.join(rel_search_dir.path, file_name)
     if os.path.isfile(path):
-      return path
-  return os.path.join(dir_name, file_name)
+      return RelativePath(path, rel_search_dir.source_root)
+  return RelativePath(os.path.join(rel_dir.path, file_name),
+                      rel_dir.source_root)
+
 
 class MojomProcessor(object):
   def __init__(self, should_generate):
@@ -105,18 +122,20 @@ class MojomProcessor(object):
           self._typemap[language] = language_map
 
   def ProcessFile(self, args, remaining_args, generator_modules, filename):
-    self._ParseFileAndImports(filename, args.import_directories, [])
+    self._ParseFileAndImports(RelativePath(filename, args.depth),
+                              args.import_directories, [])
 
     return self._GenerateModule(args, remaining_args, generator_modules,
-        filename)
+        RelativePath(filename, args.depth))
 
-  def _GenerateModule(self, args, remaining_args, generator_modules, filename):
+  def _GenerateModule(self, args, remaining_args, generator_modules,
+                      rel_filename):
     # Return the already-generated module.
-    if filename in self._processed_files:
-      return self._processed_files[filename]
-    tree = self._parsed_files[filename]
+    if rel_filename.path in self._processed_files:
+      return self._processed_files[rel_filename.path]
+    tree = self._parsed_files[rel_filename.path]
 
-    dirname, name = os.path.split(filename)
+    dirname, name = os.path.split(rel_filename.path)
     mojom = Translate(tree, name)
     if args.debug_print_intermediate:
       pprint.PrettyPrinter().pprint(mojom)
@@ -124,22 +143,21 @@ class MojomProcessor(object):
     # Process all our imports first and collect the module object for each.
     # We use these to generate proper type info.
     for import_data in mojom['imports']:
-      import_filename = FindImportFile(dirname,
-                                       import_data['filename'],
-                                       args.import_directories)
+      rel_import_file = FindImportFile(
+          RelativePath(dirname, args.depth), import_data['filename'],

[yzshen1, 2016/07/26 18:35:26]

Is it intended to use args.depth instead of rel_filename.source_root here?

[Luis Héctor Chávez, 2016/07/26 19:50:02]

It's not. Using rel_filename.source_root instead.

+          args.import_directories)
       import_data['module'] = self._GenerateModule(
-          args, remaining_args, generator_modules, import_filename)
+          args, remaining_args, generator_modules, rel_import_file)
 
     module = OrderedModuleFromData(mojom)
 
     # Set the path as relative to the source root.
-    module.path = os.path.relpath(os.path.abspath(filename),
-                                  os.path.abspath(args.depth))
+    module.path = rel_filename.relative_path()
 
     # Normalize to unix-style path here to keep the generators simpler.
     module.path = module.path.replace('\\', '/')
 
-    if self._should_generate(filename):
+    if self._should_generate(rel_filename.path):
       for language, generator_module in generator_modules.iteritems():
         generator = generator_module.Generator(
             module, args.output_dir, typemap=self._typemap.get(language, {}),
@@ -154,49 +172,56 @@ class MojomProcessor(object):
         generator.GenerateFiles(filtered_args)
 
     # Save result.
-    self._processed_files[filename] = module
+    self._processed_files[rel_filename.path] = module
     return module
 
-  def _ParseFileAndImports(self, filename, import_directories,
+  def _ParseFileAndImports(self, rel_filename, import_directories,
       imported_filename_stack):
     # Ignore already-parsed files.
-    if filename in self._parsed_files:
+    if rel_filename.path in self._parsed_files:
       return
 
-    if filename in imported_filename_stack:
-      print "%s: Error: Circular dependency" % filename + \
-          MakeImportStackMessage(imported_filename_stack + [filename])
+    if rel_filename.path in imported_filename_stack:
+      print "%s: Error: Circular dependency" % rel_filename.path + \
+          MakeImportStackMessage(imported_filename_stack + [rel_filename.path])
       sys.exit(1)
 
     try:
-      with open(filename) as f:
+      with open(rel_filename.path) as f:
         source = f.read()
     except IOError as e:
-      print "%s: Error: %s" % (e.filename, e.strerror) + \
-          MakeImportStackMessage(imported_filename_stack + [filename])
+      print "%s: Error: %s" % (e.rel_filename.path, e.strerror) + \
+          MakeImportStackMessage(imported_filename_stack + [rel_filename.path])
       sys.exit(1)
 
     try:
-      tree = Parse(source, filename)
+      tree = Parse(source, rel_filename.path)
     except Error as e:
-      full_stack = imported_filename_stack + [filename]
+      full_stack = imported_filename_stack + [rel_filename.path]
       print str(e) + MakeImportStackMessage(full_stack)
       sys.exit(1)
 
-    dirname = os.path.split(filename)[0]
+    dirname = os.path.split(rel_filename.path)[0]
     for imp_entry in tree.import_list:
-      import_filename = FindImportFile(dirname,
+      import_file_entry = FindImportFile(
+          RelativePath(dirname, rel_filename.source_root),
           imp_entry.import_filename, import_directories)
-      self._ParseFileAndImports(import_filename, import_directories,
-          imported_filename_stack + [filename])
+      self._ParseFileAndImports(import_file_entry, import_directories,
+          imported_filename_stack + [rel_filename.path])
 
-    self._parsed_files[filename] = tree
+    self._parsed_files[rel_filename.path] = tree
 
 
 def _Generate(args, remaining_args):
   if args.variant == "none":
     args.variant = None
 
+  for idx, import_dir in enumerate(args.import_directories):
+    tokens = import_dir.split(":")
+    if len(tokens) >= 2:
+      args.import_directories[idx] = RelativePath(tokens[0], tokens[1])
+    else:
+      args.import_directories[idx] = RelativePath(tokens[0], args.depth)

[yzshen1, 2016/07/22 23:38:10]

I am confused, args.depth is described as "depth from source root". but this is
passed into RelativePath as "source_root".

[Luis Héctor Chávez, 2016/07/25 21:00:22]

args.depth is used as "relative path from current directory to source root".
Should I change the documentation? (It confused me as well).

   generator_modules = LoadGenerators(args.generators_string)
 
   fileutil.EnsureDirectoryExists(args.output_dir)
@@ -243,7 +268,10 @@ def main():
                                help="comma-separated list of generators")
   generate_parser.add_argument(
       "-I", dest="import_directories", action="append", metavar="directory",
-      default=[], help="add a directory to be searched for import files")
+      default=[],
+      help="add a directory to be searched for import files. The depth from "
+           "source root can be specified for each import by appending it after "
+           "a colon")

[yzshen1, 2016/07/22 23:38:10]

Could you please give an example why this is necessary? Maybe in the CL
description.

[Luis Héctor Chávez, 2016/07/25 21:00:22]

Done.

   generate_parser.add_argument("--typemap", action="append", metavar="TYPEMAP",
                                default=[], dest="typemaps",
                                help="apply TYPEMAP to generated output")