Path evaluation to protobuf plugin.

Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
Committed: https://crrev.com/b01034d7a9d4377f8d2e244a3e14d334435de67d
Cr-Commit-Position: refs/heads/master@{#409481}

[kraynov, 2016-08-02 11:29:39]

The CQ bit was checked by kraynov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 11:29:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kraynov, 2016-08-02 11:29:56]

The CQ bit was unchecked by kraynov@chromium.org

[kraynov, 2016-08-02 11:30:15]

kraynov@chromium.org changed reviewers:
+ primiano@chromium.org

[kraynov, 2016-08-02 11:30:29]

The CQ bit was checked by kraynov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 11:30:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kraynov, 2016-08-02 11:30:42]

The CQ bit was unchecked by kraynov@chromium.org

[kraynov, 2016-08-02 12:04:56]

The CQ bit was checked by kraynov@chromium.org to run a CQ dry run

[kraynov, 2016-08-02 12:05:07]

The CQ bit was unchecked by kraynov@chromium.org

[kraynov, 2016-08-02 12:06:27]

The CQ bit was checked by kraynov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 12:06:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kraynov, 2016-08-02 12:07:11]

The CQ bit was unchecked by kraynov@chromium.org

[kraynov, 2016-08-02 12:39:14]

Description was changed from

==========
Path evaluation for protobuf plugin.

Protobuf compiler (protoc) plugin is basically executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

to

==========
Path evaluation for protobuf plugin.

Protobuf compiler (protoc) plugin is basically executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

[kraynov, 2016-08-02 12:39:14]

kraynov@chromium.org changed reviewers:
+ brettw@chromium.org

[kraynov, 2016-08-02 12:43:35]

Please take a look.
//components/tracing/BUILD.gn is only usage at the moment. I just removed some
lines otherwise GN complaining for unused variable.
This approach to get the path of executable seems to be universal and inspired
by https://cs.chromium.org/chromium/src/build/compiled_action.gni?l=110

[kraynov, 2016-08-02 12:44:29]

Description was changed from

==========
Path evaluation for protobuf plugin.

Protobuf compiler (protoc) plugin is basically executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

to

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

[kraynov, 2016-08-02 13:11:59]

kraynov@chromium.org changed reviewers:
+ xyzzyz@chromium.org

[kraynov, 2016-08-02 13:22:43]

kraynov@chromium.org changed reviewers:
+ thakis@chromium.org

[kraynov, 2016-08-02 13:26:07]

+xyzzyz
+Nico

Please take a look.
//components/tracing/BUILD.gn is only usage at the moment. I just removed some
lines otherwise GN complaining for unused variable.
This approach to get the path of executable seems to be universal and inspired
by https://cs.chromium.org/chromium/src/build/compiled_action.gni?l=110

[kraynov, 2016-08-02 13:28:21]

Description was changed from

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

to

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

[xyzzyz, 2016-08-02 17:20:15]

LGTM, but I feel like this is something that GN should provide. If it doesn't,
then we should have a issue filed for that.

[Nico, 2016-08-02 17:38:27]

Looks generally good.

https://codereview.chromium.org/2202233002/diff/20001/third_party/protobuf/pr...
File third_party/protobuf/proto_library.gni (right):

https://codereview.chromium.org/2202233002/diff/20001/third_party/protobuf/pr...
third_party/protobuf/proto_library.gni:190: if (is_win) {
This is likely wrong (also in the old place). I'm guessing that
$generator_plugin is a host executable, so you need to check the host os. is_win
checks the target os. If cross-compiling chrome/win on linux, this here
currently does the wrong thing. (cross-compiling chrome/win on windows isn't
fully working yet, but it's also not super far away, see
https://codereview.chromium.org/1183633003/)


See
https://cs.chromium.org/chromium/src/chrome/test/base/js2gtest.gni?rcl=0&l=42
for an example. The get_labelinfo() call there also looks a bit simpler than
what you have.

[kraynov, 2016-08-02 18:02:25]

The CQ bit was checked by kraynov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 18:03:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kraynov, 2016-08-02 18:06:24]

The CQ bit was unchecked by kraynov@chromium.org

[kraynov, 2016-08-02 19:28:50]

https://codereview.chromium.org/2202233002/diff/20001/third_party/protobuf/pr...
File third_party/protobuf/proto_library.gni (right):

https://codereview.chromium.org/2202233002/diff/20001/third_party/protobuf/pr...
third_party/protobuf/proto_library.gni:190: if (is_win) {
On 2016/08/02 17:38:27, Nico wrote:
> This is likely wrong (also in the old place). I'm guessing that
> $generator_plugin is a host executable, so you need to check the host os.
is_win
> checks the target os. If cross-compiling chrome/win on linux, this here
> currently does the wrong thing. (cross-compiling chrome/win on windows isn't
> fully working yet, but it's also not super far away, see
> https://codereview.chromium.org/1183633003/)
> 
> 
> See
> https://cs.chromium.org/chromium/src/chrome/test/base/js2gtest.gni?rcl=0&l=42
> for an example. The get_labelinfo() call there also looks a bit simpler than
> what you have.

Done.

[Nico, 2016-08-02 19:29:24]

lgtm

[Primiano Tucci (use gerrit), 2016-08-03 09:16:10]

LGTM

[kraynov, 2016-08-03 09:16:32]

The CQ bit was checked by kraynov@chromium.org

[kraynov, 2016-08-03 09:16:32]

The patchset sent to the CQ was uploaded after l-g-t-m from xyzzyz@chromium.org
Link to the patchset: https://codereview.chromium.org/2202233002/#ps40001
(title: "Windows cross-compile in future")

[commit-bot: I haz the power, 2016-08-03 09:16:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 10:48:20]

Description was changed from

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

to

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

[commit-bot: I haz the power, 2016-08-03 10:48:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-03 10:49:54]

Description was changed from

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680
==========

to

==========
Path evaluation to protobuf plugin.

Protobuf compiler (protoc) plugin is basically an executable.
Path for executables can vary if building for diffent platforms.
This change allows to skip writing complex boilerplate each time.

BUG=622680

Committed: https://crrev.com/b01034d7a9d4377f8d2e244a3e14d334435de67d
Cr-Commit-Position: refs/heads/master@{#409481}
==========

[commit-bot: I haz the power, 2016-08-03 10:49:55]

Patchset 3 (id:??) landed as
https://crrev.com/b01034d7a9d4377f8d2e244a3e14d334435de67d
Cr-Commit-Position: refs/heads/master@{#409481}