Option added to protobuf build for import directories

third_party/protobuf/proto_library.gni

 # Copyright 2014 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.
 
 # Compile a protocol buffer.
 #
 # Protobuf parameters:
 #
 #   proto_in_dir (optional)
 #       Specifies the path relative to the current BUILD.gn file where
@@ skipping 45 matching lines @@
 #   generator_plugin_options (optional)
 #       Extra flags passed to the plugin. See cc_generator_options.
 #
 #   deps (optional)
 #       Additional dependencies.
 #
 #   use_protobuf_full (optional)
 #       If adding protobuf library would be required, adds protobuf_full to deps
 #       instead of protobuf_lite.
 #
+#   import_dirs (optional)
+#       A list of extra import directories to be passed to protoc compiler. The
+#       default case is just proto_in_dir.
+#
 # Parameters for compiling the generated code:
 #
 #   component_build_force_source_set (Default=false)
 #       When set true the generated code will be compiled as a source set in
 #       the component build. This does not affect static builds.  If you are
 #       exporting symbols from a component, this is required to prevent those
 #       symbols from being stripped. If you're not using dllexports in
 #       cc_generator_options, it's usually best to leave this false.
 #
 #   defines (optional)
@@ skipping 194 matching lines @@
         rel_cc_out_dir,
       ]
       if (defined(invoker.generator_plugin_options)) {
         args += [
           "--plugin-options",
           invoker.generator_plugin_options,
         ]
       }
     }
 
+    if (defined(invoker.import_dirs)) {
+      args += ["--import-dir"]

[xyzzyz, 2017/01/23 17:53:56]

I was under impression before that the way this flag is supposed to work is that
we pass multiple occurences of it to the script, as in "wrapper.py --import-dir
dir1/ --import-dir dir2/". Seems like Python's argparse actually expects that we
do "--import-dir dir1/ dir2/", and your code is written this way too. It means
that this flag should rather be named "--import-dirs", to reflect that we want
to pass a list of directories to it, not a single directory.

[Dirk Pranke, 2017/01/24 02:31:20]

argparse will actually accept either, but the latter usage is weird and can lead
to strange bugs,
particularly when you're also expecting positional arguments.

In particular, in this case, if you have `protoc_wrapper.py --import-dir foo
bar.proto`, this will get parsed as import_dir=["foo", "bar.proto"] and then
error out because it didn't find a proto file.

To avoid this you need to specify '--' before any positional args that might
follow --import-dir (which doesn't actually happen in this usage).

For this reason I really don't like using --foo f1 f2; I'd rather you did
--foo=f1 --foo=f2, even if the positional arguments come first.

[Ramin Halavati, 2017/01/24 08:38:33]

Done, changed it to one --import-dir switch for each directory.

+      args += rebase_path(invoker.import_dirs, root_build_dir)
+    }
+
     # System protoc is not used so it's necessary to build a chromium one.
     inputs = [
       protoc_path,
     ]
     deps = [
       protoc_label,
     ]
 
     if (generate_with_plugin) {
       inputs += [ plugin_path ]
@@ skipping 78 matching lines @@
       ":$action_name",
     ]
 
     # This will link any libraries in the deps (the use of invoker.deps in the
     # action won't link it).
     if (defined(invoker.deps)) {
       deps += invoker.deps
     }
   }
 }