Split protoc into library and executable.

Split protoc into library and executable.

Similarly to the upstream BUILD file, we split the protoc compiler into
a library and executable. The purpose is to allow protoc compiler
plugins (see plugin.h).

BUG=597321
Committed: https://crrev.com/132bbd301e93c7f92feed8f29a79539ea8a24c27
Cr-Commit-Position: refs/heads/master@{#390476}

[xyzzyz, 2016-04-26 23:04:26]

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

[xyzzyz, 2016-04-26 23:04:27]

Hi Peter,

Could you review a small change in protobuf build files? It splits the protoc
compiler into library and the executable, similarly to how it works in upstream
BUILD. The purpose is to enable support of protoc compiler plugins (see
plugin.h), which need to link libprotoc.

Thanks,
Adam

[Peter Kasting, 2016-04-27 22:30:17]

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (left):

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:540: deps += [ "//build/config/sanitizers:deps" ]
This seems to have disappeared entirely?

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (right):

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:550: ":protobuf_warnings",
Do we need this?

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:554: configs -= [
"//build/config/win:lean_and_mean" ]
Likewise, do we need this?

[xyzzyz, 2016-04-27 23:10:27]

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (left):

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:540: deps += [ "//build/config/sanitizers:deps" ]
On 2016/04/27 22:30:16, Peter Kasting wrote:
> This seems to have disappeared entirely?

Yes, I added this during my struggle with the asan buildbot hoping it would
solve the problem, but when I fixed that issue, I forgot to remove it, as it's
not really necessary here.

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (right):

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:550: ":protobuf_warnings",
On 2016/04/27 22:30:16, Peter Kasting wrote:
> Do we need this?

Removed.

https://codereview.chromium.org/1923733002/diff/20001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:554: configs -= [
"//build/config/win:lean_and_mean" ]
On 2016/04/27 22:30:17, Peter Kasting wrote:
> Likewise, do we need this?

Removed.

[Peter Kasting, 2016-04-28 00:29:36]

Besides the comment below, the only real question I have left is whether the use
of source_set vs. static_library is correct, as raised in my previous review
round.  You may want to loop in a GN expert on this if you're not certain.

Otherwise, LGTM.

https://codereview.chromium.org/1923733002/diff/40001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (right):

https://codereview.chromium.org/1923733002/diff/40001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:550: ":protobuf_full",
If this needs to depend on protobuf_full because anything that depends on
protoc_lib needs to depend on that, there's some syntax you should use instead
in the protoc_lib definition that basically says "treat protobuf_full as a
dependency of anything that depends on me".  I don't remember what this syntax
is, though.

[xyzzyz, 2016-04-28 20:00:31]

https://codereview.chromium.org/1923733002/diff/40001/third_party/protobuf/BU...
File third_party/protobuf/BUILD.gn (right):

https://codereview.chromium.org/1923733002/diff/40001/third_party/protobuf/BU...
third_party/protobuf/BUILD.gn:550: ":protobuf_full",
On 2016/04/28 00:29:36, Peter Kasting wrote:
> If this needs to depend on protobuf_full because anything that depends on
> protoc_lib needs to depend on that, there's some syntax you should use instead
> in the protoc_lib definition that basically says "treat protobuf_full as a
> dependency of anything that depends on me".  I don't remember what this syntax
> is, though.

I think it's "public_deps". Setting this for protoc_lib.

[xyzzyz, 2016-04-28 20:01:33]

The CQ bit was checked by xyzzyz@chromium.org

[xyzzyz, 2016-04-28 20:01:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1923733002/#ps60001
(title: "update public deps")

[commit-bot: I haz the power, 2016-04-28 20:02:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1923733002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1923733002/60001

[commit-bot: I haz the power, 2016-04-28 20:57:36]

Committed patchset #4 (id:60001)

[Justin Donnelly, 2016-04-28 21:31:43]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1929963004/ by jdonnelly@chromium.org.

The reason for reverting is: Broke compile on WebKit Linux MSAN (and others?).

[commit-bot: I haz the power, 2016-04-30 17:21:01]

Patchset 4 (id:??) landed as
https://crrev.com/132bbd301e93c7f92feed8f29a79539ea8a24c27
Cr-Commit-Position: refs/heads/master@{#390476}

[commit-bot: I haz the power, 2016-04-30 17:57:27]

Description was changed from

==========
Split protoc into library and executable.

Similarly to the upstream BUILD file, we split the protoc compiler into
a library and executable. The purpose is to allow protoc compiler
plugins (see plugin.h).

BUG=597321
==========

to

==========
Split protoc into library and executable.

Similarly to the upstream BUILD file, we split the protoc compiler into
a library and executable. The purpose is to allow protoc compiler
plugins (see plugin.h).

BUG=597321

Committed: https://crrev.com/132bbd301e93c7f92feed8f29a79539ea8a24c27
Cr-Commit-Position: refs/heads/master@{#390476}
==========