GN proto_library refactoring |
|||
Issue description[Tracker "bug" for CL] I took some effort to refactor //third_party/protobuf/proto_library.gni and //tools/protoc_wrapper.py and there is motivation to do so. 1. It didn't work with sub-directories. For example: //foo/bar/baz.proto # Import "baz_internals/epic.proto" //foo/bar/baz_internals/epic.proto If these files are compiled using action_foreach with base (--proto_path) directory where each file located it will cause build errors because official C++ generator mangles some internal names relying on relative path as well. Sub-directories would be useful for Tracing-V2 because it's expected to define lots of event types. 2. Some proto libraries in Chromium are multi-file but it's faster to read all these files in a single pass than enjoying benefits of better action_foreach's parallelism. Simply because any imports should be parsed and running protoc file-by-file causes parsing some common proto files repeatedly. 3. Week dependency. GN can't see the dependencies between proto files. Such scenario: a) Create //foo/bar.proto and //foo/baz.proto b) Import baz in bar (so now bar depends on baz) c) Get gen/foo/bar.pb.h gen/foo/bar.pb.cc gen/foo/baz.pb.h gen/foo/baz.pb.cc built b) Edit //foo/baz.proto e) Run ninja f) Realise that //foo/bar.proto was not rebuilt. Passing all the files to protoc is more reliable.
,
Aug 19 2016
I've refactored proto_library https://codereview.chromium.org/2239383002/ and will remove |json_converter| option very soon replacing with native support for python plugins. The only dependency is //third_party/dom_distiller_js/BUILD.gn TODO(kraynov): Author a CL with the following changes: 1. Introduce native Python plugin support in proto_library.gni (on Windows as well) 2. Adjust //third_party/dom_distiller_js/BUILD.gn 3. Remove |json_converter| and |inputs| options.
,
Sep 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec3645ef783be944a98beafd6c8b1858b4c39aeb commit ec3645ef783be944a98beafd6c8b1858b4c39aeb Author: kraynov <kraynov@chromium.org> Date: Fri Sep 16 10:29:11 2016 Remove workaround from GN proto_library. Added support for script protobuf compiler plugins. Removed |json_converter| option workaround. BUG= 637292 Review-Url: https://codereview.chromium.org/2343833002 Cr-Commit-Position: refs/heads/master@{#419139} [modify] https://crrev.com/ec3645ef783be944a98beafd6c8b1858b4c39aeb/third_party/dom_distiller_js/BUILD.gn [modify] https://crrev.com/ec3645ef783be944a98beafd6c8b1858b4c39aeb/third_party/protobuf/proto_library.gni
,
Sep 16 2016
,
Sep 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f26f4a6b513281fb98c976339e681b40a504d40 commit 7f26f4a6b513281fb98c976339e681b40a504d40 Author: Nico Weber <thakis@chromium.org> Date: Tue Sep 05 01:24:12 2017 Make dom_distiller_js build in linux->win cross builds. Bug: 495204, 637292 Change-Id: I72c5117537be0fdffd20dd46faca3124a458ab16 Reviewed-on: https://chromium-review.googlesource.com/649966 Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#499536} [modify] https://crrev.com/7f26f4a6b513281fb98c976339e681b40a504d40/third_party/dom_distiller_js/BUILD.gn |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Aug 18 2016