Index: tools/protoc_wrapper/protoc_wrapper.py |
diff --git a/tools/protoc_wrapper/protoc_wrapper.py b/tools/protoc_wrapper/protoc_wrapper.py |
index e6ddf95184adfc089ec8f667da46a99daba35a14..1cffeda2cb9f56e8ba91939ed063a044e56690bd 100755 |
--- a/tools/protoc_wrapper/protoc_wrapper.py |
+++ b/tools/protoc_wrapper/protoc_wrapper.py |
@@ -1,134 +1,132 @@ |
#!/usr/bin/env python |
-# Copyright (c) 2012 The Chromium Authors. All rights reserved. |
+# Copyright (c) 2016 The Chromium Authors. All rights reserved. |
petrcermak
2016/08/15 13:00:06
The date should NOT be changed. See https://chromi
kraynov
2016/08/15 13:37:49
Acknowledged.
|
# Use of this source code is governed by a BSD-style license that can be |
# found in the LICENSE file. |
""" |
A simple wrapper for protoc. |
- |
-- Adds includes in generated headers. |
-- Handles building with system protobuf as an option. |
+Script for //third_party/protobuf/proto_library.gni . |
+Features: |
+- Automatic include insertion. |
petrcermak
2016/08/15 13:00:06
Maybe add a hash ("#include") so that it's easier
kraynov
2016/08/15 13:37:50
Acknowledged.
|
+- Preventing bad proto names. |
petrcermak
2016/08/15 13:00:06
s/Preventing/Prevents/
kraynov
2016/08/15 13:37:49
Acknowledged.
|
""" |
-import fnmatch |
+from __future__ import print_function |
import optparse |
import os.path |
-import shutil |
import subprocess |
import sys |
import tempfile |
-PROTOC_INCLUDE_POINT = '// @@protoc_insertion_point(includes)\n' |
- |
-def ModifyHeader(header_file, extra_header): |
- """Adds |extra_header| to |header_file|. Returns 0 on success. |
- |
- |extra_header| is the name of the header file to include. |
- |header_file| is a generated protobuf cpp header. |
- """ |
- include_point_found = False |
- header_contents = [] |
- with open(header_file) as f: |
- for line in f: |
- header_contents.append(line) |
- if line == PROTOC_INCLUDE_POINT: |
- extra_header_msg = '#include "%s"\n' % extra_header |
- header_contents.append(extra_header_msg) |
- include_point_found = True; |
- if not include_point_found: |
- return 1 |
- |
- with open(header_file, 'wb') as f: |
- f.write(''.join(header_contents)) |
- return 0 |
- |
-def ScanForBadFiles(scan_root): |
- """Scan for bad file names, see http://crbug.com/386125 for details. |
- Returns True if any filenames are bad. Outputs errors to stderr. |
- |
- |scan_root| is the path to the directory to be recursively scanned. |
- """ |
- badname = False |
- real_scan_root = os.path.realpath(scan_root) |
- for dirpath, dirnames, filenames in os.walk(real_scan_root): |
- matches = fnmatch.filter(filenames, '*-*.proto') |
- if len(matches) > 0: |
- if not badname: |
- badname = True |
- sys.stderr.write('proto files must not have hyphens in their names (' |
- 'see http://crbug.com/386125 for more information):\n') |
- for filename in matches: |
- sys.stderr.write(' ' + os.path.join(real_scan_root, |
- dirpath, filename) + '\n') |
- return badname |
- |
- |
-def RewriteProtoFilesForSystemProtobuf(path): |
- wrapper_dir = tempfile.mkdtemp() |
- try: |
- for filename in os.listdir(path): |
- if not filename.endswith('.proto'): |
- continue |
- with open(os.path.join(path, filename), 'r') as src_file: |
- with open(os.path.join(wrapper_dir, filename), 'w') as dst_file: |
- for line in src_file: |
- # Remove lines that break build with system protobuf. |
- # We cannot optimize for lite runtime, because system lite runtime |
- # does not have a Chromium-specific hack to retain unknown fields. |
- # Similarly, it does not understand corresponding option to control |
- # the usage of that hack. |
- if 'LITE_RUNTIME' in line or 'retain_unknown_fields' in line: |
- continue |
- dst_file.write(line) |
- |
- return wrapper_dir |
- except: |
- shutil.rmtree(wrapper_dir) |
- raise |
+PROTOC_INCLUDE_POINT = "// @@protoc_insertion_point(includes)" |
+ |
+ |
+def ProtocGenOptions(options): |
petrcermak
2016/08/15 13:00:06
The name is a noun, so it suggests that it's a cla
petrcermak
2016/08/15 13:00:06
If I understand correctly, all methods in this fil
kraynov
2016/08/15 13:37:49
Acknowledged.
kraynov
2016/08/15 13:37:50
It's standalone utility. I've followed the previou
|
+ if options is not None: |
petrcermak
2016/08/15 13:00:06
Unless '' (empty string) are valid |options|, you
petrcermak
2016/08/15 13:00:06
Don't use nested if statements unless necessary:
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ if options.endswith(":"): |
+ return options |
+ else: |
+ return options + ":" |
+ else: |
+ return "" |
+ |
+ |
+def StripProtos(protos): |
petrcermak
2016/08/15 13:00:06
(Feel free to ignore) I would personally use a com
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ stripped = [] |
+ for filename in protos: |
+ if not filename.endswith(".proto"): |
+ raise RuntimeError("Invalid proto filename extension: " + filename) |
+ if "-" in filename: |
+ raise RuntimeError("Proto files must not have hyphens in their names " |
petrcermak
2016/08/15 13:00:06
suggestion "Proto file names must not contain hyph
kraynov
2016/08/15 13:37:50
Acknowledged.
|
+ "(see http://crbug.com/386125 for more information).") |
petrcermak
2016/08/15 13:00:06
supernit: Your error ends with a period here, but
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ # Remove .proto extension. |
+ stripped.append(filename.rsplit(".", 1)[0]) |
+ return stripped |
+ |
+ |
+def WriteIncludes(headers, include): |
+ for filename in headers: |
+ include_point_found = False |
+ contents = [] |
+ with open(filename) as f: |
+ for line in f: |
+ stripped_line = line.strip() |
+ contents.append(stripped_line) |
+ if stripped_line == PROTOC_INCLUDE_POINT: |
petrcermak
2016/08/15 13:00:06
you should add an if statement asserting that incl
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ extra_statement = '#include "{0}"'.format(include) |
+ contents.append(extra_statement) |
+ include_point_found = True |
+ |
+ if not include_point_found: |
+ raise RuntimeError("Include point not found in header: " + filename) |
+ |
+ with open(filename, "w") as f: |
+ for line in contents: |
petrcermak
2016/08/15 13:00:06
f.writelines(contents)
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ print(line, file=f) |
def main(argv): |
parser = optparse.OptionParser() |
petrcermak
2016/08/15 13:00:06
Could you use the argparse module instead? Optpars
kraynov
2016/08/15 13:37:50
Okay.
|
- parser.add_option('--include', dest='extra_header', |
- help='The extra header to include. This must be specified ' |
- 'along with --protobuf.') |
- parser.add_option('--protobuf', dest='generated_header', |
- help='The c++ protobuf header to add the extra header to. ' |
- 'This must be specified along with --include.') |
- parser.add_option('--proto-in-dir', |
- help='The directory containing .proto files.') |
- parser.add_option('--proto-in-file', help='Input file to compile.') |
- parser.add_option('--use-system-protobuf', type=int, default=0, |
- help='Option to use system-installed protobuf ' |
- 'instead of bundled one.') |
- (options, args) = parser.parse_args(sys.argv) |
- if len(args) < 2: |
- return 1 |
- |
- if ScanForBadFiles(options.proto_in_dir): |
- return 1 |
- |
- proto_path = options.proto_in_dir |
- if options.use_system_protobuf == 1: |
- proto_path = RewriteProtoFilesForSystemProtobuf(proto_path) |
- try: |
- # Run what is hopefully protoc. |
- protoc_args = args[1:] |
- protoc_args += ['--proto_path=%s' % proto_path, |
- os.path.join(proto_path, options.proto_in_file)] |
- ret = subprocess.call(protoc_args) |
- if ret != 0: |
- return ret |
- finally: |
- if options.use_system_protobuf == 1: |
- # Remove temporary directory holding re-written files. |
- shutil.rmtree(proto_path) |
- |
- # protoc succeeded, check to see if the generated cpp header needs editing. |
- if not options.extra_header or not options.generated_header: |
- return 0 |
- return ModifyHeader(options.generated_header, options.extra_header) |
+ parser.add_option("--protoc", |
+ help="Relative path to compiler.") |
+ |
+ parser.add_option("--proto-in-dir", |
+ help="Base directory with source protos.") |
+ parser.add_option("--cc-out-dir", |
+ help="Output directory for standard C++ generator.") |
+ parser.add_option("--py-out-dir", |
+ help="Output directory for standard Python generator.") |
+ parser.add_option("--plugin-out-dir", |
+ help="Output directory for custom generator.") |
petrcermak
2016/08/15 13:00:06
I think that the help string should contain the wo
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ |
+ parser.add_option("--plugin", |
+ help="Relative path to custom generator plugin.") |
+ parser.add_option("--plugin-options", |
+ help="Custom generator options.") |
petrcermak
2016/08/15 13:00:07
I think that the help string should contain the wo
kraynov
2016/08/15 13:37:50
Acknowledged.
|
+ parser.add_option("--cc-options", |
+ help="Standard C++ generator options.") |
+ parser.add_option("--include", |
+ help="Name of include to put into generated headers.") |
petrcermak
2016/08/15 13:00:06
supernit: s/put/insert/
kraynov
2016/08/15 13:37:50
Acknowledged.
|
+ (options, args) = parser.parse_args(argv) |
petrcermak
2016/08/15 13:00:06
no need for parentheses (feel free to ignore)
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ |
+ protoc = os.path.realpath(options.protoc) |
petrcermak
2016/08/15 13:00:06
In my opinion, there's no need to define this here
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ proto_dir = os.path.relpath(options.proto_in_dir) |
+ protoc_args = ["--proto_path", proto_dir] |
petrcermak
2016/08/15 13:00:06
To avoid pre-pending the protoc binary at the end
kraynov
2016/08/15 13:37:50
Acknowledged.
|
+ |
+ protos = args[1:] |
+ stripped_protos = StripProtos(protos) |
+ headers = [] |
+ |
+ if options.cc_out_dir is not None: |
petrcermak
2016/08/15 13:00:06
you could do the following:
if options.cc_out_dir
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ cc_options = ProtocGenOptions(options.cc_options) |
+ protoc_args += ["--cpp_out", cc_options + options.cc_out_dir] |
+ for name in stripped_protos: |
+ headers.append(os.path.join(options.cc_out_dir, name + ".pb.h")) |
+ |
+ if options.py_out_dir is not None: |
+ protoc_args += ["--python_out", options.py_out_dir] |
+ |
+ if options.plugin_out_dir is not None: |
+ plugin_options = ProtocGenOptions(options.plugin_options) |
+ protoc_args += [ |
+ "--plugin", "protoc-gen-plugin=" + os.path.relpath(options.plugin), |
+ "--plugin_out", plugin_options + options.plugin_out_dir, |
+ ] |
+ |
+ for name in protos: |
+ protoc_args.append(os.path.join(proto_dir, name)) |
petrcermak
2016/08/15 13:00:06
I personally prefer comprehensions over explicit l
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ |
+ ret = subprocess.call([protoc] + protoc_args) |
+ if ret != 0: |
+ raise RuntimeError("Protoc has returned non-zero status: " + str(ret)) |
+ |
+ if options.include is not None: |
petrcermak
2016/08/15 13:00:06
I think you can drop "is not None" here
kraynov
2016/08/15 13:37:49
Acknowledged.
|
+ WriteIncludes(headers, options.include) |
if __name__ == '__main__': |
petrcermak
2016/08/15 13:00:06
Before this patch, this file used single quotes ev
kraynov
2016/08/15 13:37:50
Acknowledged.
|
- sys.exit(main(sys.argv)) |
+ try: |
+ main(sys.argv) |
+ except RuntimeError as e: |
+ print(e, file=sys.stderr) |
+ sys.exit(1) |