Reland: Split protoc into library and executable.

third_party/protobuf/BUILD.gn

Index: third_party/protobuf/BUILD.gn
diff --git a/third_party/protobuf/BUILD.gn b/third_party/protobuf/BUILD.gn
index e4520ec1473d5eb090d1ac311fa489f7938a2154..1edbbbf699800f2e97396cb38808d4fae1a8b36f 100644
--- a/third_party/protobuf/BUILD.gn
+++ b/third_party/protobuf/BUILD.gn
@@ -306,6 +306,10 @@ source_set("protobuf_full") {
     "src/google/protobuf/wrappers.pb.h",
   ]
 
+  deps = [
+    "//build/config/sanitizers:deps",

[Wez, 2016/04/28 22:46:05]

So there is no GYP equivalent of this required?

Does this definitely have the desired effect?  Previously this deps was at the
executable level, not the source-set, so I'd expect to still see it in the
executable's deps, not here.

[xyzzyz, 2016/04/28 22:56:56]

It's this target that uses sanitizer interface, not the executable, and the
purpose of this CL is to create a separate target so that I can add the gRPC
target that would depend on protoc_lib, but not on protoc itself. 

The way it works is by adding some public_configs, so all targets depending on
this will also depend on the sanitizer stuff. I think it's not necessary in gyp,
because they set up asan in a completely different way.

+  ]
+
   configs -= [ "//build/config/compiler:chromium_code" ]
   configs += [
     "//build/config/compiler:no_chromium_code",
@@ -329,7 +333,7 @@ source_set("protobuf_full") {
 
 # Only compile the compiler for the host architecture.
 if (current_toolchain == host_toolchain) {
-  executable("protoc") {
+  source_set("protoc_lib") {

[Wez, 2016/04/28 22:46:05]

nit: protoc_sources, since it's a source-set?

It might be handy to provide a brief comment explaining the split, even if all
it does is refer to the bug # for that.

[xyzzyz, 2016/04/28 22:56:56]

I'd rather keep it protoc_lib, to match the name in upstream BUILD.

I'll add the comment.

     sources = [
       "src/google/protobuf/compiler/code_generator.cc",
       "src/google/protobuf/compiler/code_generator.h",
@@ -476,7 +480,6 @@ if (current_toolchain == host_toolchain) {
       "src/google/protobuf/compiler/javanano/javanano_primitive_field.h",
       "src/google/protobuf/compiler/js/js_generator.cc",
       "src/google/protobuf/compiler/js/js_generator.h",
-      "src/google/protobuf/compiler/main.cc",
       "src/google/protobuf/compiler/objectivec/objectivec_enum.cc",
       "src/google/protobuf/compiler/objectivec/objectivec_enum.h",
       "src/google/protobuf/compiler/objectivec/objectivec_enum_field.cc",
@@ -528,16 +531,31 @@ if (current_toolchain == host_toolchain) {
       configs -= [ "//build/config/win:lean_and_mean" ]
     }
 
+    public_configs = [ ":protobuf_config" ]
+
     cflags = protobuf_lite_cflags
 
-    deps = [
+    public_deps = [
       ":protobuf_full",
-      "//build/config/sanitizers:deps",
+    ]
+  }
+
+  executable("protoc") {
+    sources = [
+      "src/google/protobuf/compiler/main.cc",
+    ]
+
+    configs -= [ "//build/config/compiler:chromium_code" ]
+    configs += [ "//build/config/compiler:no_chromium_code" ]

[Wez, 2016/04/28 22:46:05]

I assume that these configs affect the protoc_lib as well, since its a
source-set rather than static-library?

[xyzzyz, 2016/04/28 22:56:56]

Sorry, I don't quite understand your question.

[Wez, 2016/04/28 23:00:04]

These are compiler configurations - if you were building a static-library then
you'd need the configs in the library declaration as well as here in the
executable declaration.

I'm just asking you to confirm that source_set works differently from
static_library in that regard, and uses the compiler configuration of the
embedding library or exe?

[xyzzyz, 2016/04/28 23:07:39]

I'm not sure, I think they should work mostly the same except the static_library
also packs the objects to *.a file. 

These compiler configs mostly refer to warnings level in the compiler, and I in
fact have them both here and in the library (source_set) declaration.

[Wez, 2016/04/28 23:15:18]

Gah! codereview caught me out again by eliding unchanged lines *facepalm*

Ahem, I'll just go and hit that LGTM button...

+
+    cflags = protobuf_lite_cflags
+
+    deps = [
+      ":protoc_lib",
 
       # Default manifest on Windows (a no-op elsewhere).
       "//build/win:default_exe_manifest",
     ]
-    deps += [ "//build/config/sanitizers:deps" ]
   }
 }