Commit c2a68d05 authored by Lalit Maganti's avatar Lalit Maganti Committed by Commit Bot

protobuf: include imports when generating descriptor and add depfile

This CL adds the include_imports flag to protoc when generating
descriptors. This flag is necessary to support any sort of imports in
proto files as GN does not have support for transitively listing all
the source files in a target and its deps.

Since now include_imports silently looks at other source files other
than the ones listed in the target, this means we need a depfile to
allow ninja to pick up if changes are made in transitive dep source
files. Unfortunately, protoc's depfile generation is broken out of the
box and does not work with ninja.

Add support in the wrapper script to correct this mirror what we do in
Perfetto to achieve the same thing (see [1])

[1] https://cs.android.com/android/platform/superproject/+/master:external/perfetto/tools/protoc_helper.py

Change-Id: Ib330520232280b3a8a9eb80a397fac2f3cd1e076
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440887Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813161}
parent 2558769b
...@@ -297,8 +297,8 @@ template("proto_library") { ...@@ -297,8 +297,8 @@ template("proto_library") {
# Generate protobuf stubs. # Generate protobuf stubs.
action(action_name) { action(action_name) {
visibility = [ visibility = [
":$source_set_name",
":$javascript_name", ":$javascript_name",
":$source_set_name",
] ]
script = "//tools/protoc_wrapper/protoc_wrapper.py" script = "//tools/protoc_wrapper/protoc_wrapper.py"
sources = proto_sources sources = proto_sources
...@@ -371,9 +371,15 @@ template("proto_library") { ...@@ -371,9 +371,15 @@ template("proto_library") {
} }
if (generate_descriptor != "") { if (generate_descriptor != "") {
depfile =
"$root_gen_dir/" + proto_out_dir + "/" + generate_descriptor + ".d"
rel_depfile = rebase_path(depfile, root_build_dir)
args += [ args += [
"--descriptor-set-out", "--descriptor-set-out",
rel_descriptor_out, rel_descriptor_out,
"--descriptor-set-dependency-file",
rel_depfile,
] ]
} }
......
...@@ -9,6 +9,12 @@ Script for //third_party/protobuf/proto_library.gni . ...@@ -9,6 +9,12 @@ Script for //third_party/protobuf/proto_library.gni .
Features: Features:
- Inserts #include for extra header automatically. - Inserts #include for extra header automatically.
- Prevents bad proto names. - Prevents bad proto names.
- Works around protoc's bad descriptor file generation.
Ninja expects the format:
target: deps
But protoc just outputs:
deps
This script adds the "target:" part.
""" """
from __future__ import print_function from __future__ import print_function
...@@ -99,6 +105,9 @@ def main(argv): ...@@ -99,6 +105,9 @@ def main(argv):
) )
parser.add_argument("--descriptor-set-out", parser.add_argument("--descriptor-set-out",
help="Path to write a descriptor.") help="Path to write a descriptor.")
parser.add_argument(
"--descriptor-set-dependency-file",
help="Path to write the dependency file for descriptor set.")
parser.add_argument("protos", nargs="+", parser.add_argument("protos", nargs="+",
help="Input protobuf definition file(s).") help="Input protobuf definition file(s).")
...@@ -155,6 +164,15 @@ def main(argv): ...@@ -155,6 +164,15 @@ def main(argv):
if options.descriptor_set_out: if options.descriptor_set_out:
protoc_cmd += ["--descriptor_set_out", options.descriptor_set_out] protoc_cmd += ["--descriptor_set_out", options.descriptor_set_out]
protoc_cmd += ["--include_imports"]
dependency_file_data = None
if options.descriptor_set_out and options.descriptor_set_dependency_file:
protoc_cmd += ['--dependency_out', options.descriptor_set_dependency_file]
ret = subprocess.call(protoc_cmd)
with open(options.descriptor_set_dependency_file, 'r') as f:
dependency_file_data = f.read().decode('utf-8')
ret = subprocess.call(protoc_cmd) ret = subprocess.call(protoc_cmd)
if ret != 0: if ret != 0:
...@@ -168,6 +186,11 @@ def main(argv): ...@@ -168,6 +186,11 @@ def main(argv):
raise RuntimeError("Protoc has returned non-zero status: " raise RuntimeError("Protoc has returned non-zero status: "
"{0}".format(error_number)) "{0}".format(error_number))
if dependency_file_data:
with open(options.descriptor_set_dependency_file, 'w') as f:
f.write(options.descriptor_set_out + ":")
f.write(dependency_file_data)
if options.include: if options.include:
WriteIncludes(headers, options.include) WriteIncludes(headers, options.include)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment