Reland: Split protoc into library and executable.

third_party/protobuf/BUILD.gn

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 config("protobuf_config") {
   include_dirs = [ "src" ]
   defines = [
     "GOOGLE_PROTOBUF_NO_RTTI",
     "GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER",
   ]
@@ skipping 288 matching lines @@
     "src/google/protobuf/util/time_util.h",
     "src/google/protobuf/util/type_resolver.h",
     "src/google/protobuf/util/type_resolver_util.cc",
     "src/google/protobuf/util/type_resolver_util.h",
     "src/google/protobuf/wire_format.cc",
     "src/google/protobuf/wire_format.h",
     "src/google/protobuf/wrappers.pb.cc",
     "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",
 
     # Must be after no_chromium_code for warning flags to be ordered
     # correctly.
     ":protobuf_warnings",
   ]
   if (is_win) {
     configs -= [ "//build/config/win:lean_and_mean" ]
   }
   public_configs = [
     ":protobuf_config",
 
     # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
     "//build/config/compiler:no_size_t_to_int_warning",
   ]
 
   cflags = protobuf_lite_cflags
 }
 
 # 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",
       "src/google/protobuf/compiler/command_line_interface.cc",
       "src/google/protobuf/compiler/command_line_interface.h",
       "src/google/protobuf/compiler/cpp/cpp_enum.cc",
       "src/google/protobuf/compiler/cpp/cpp_enum.h",
       "src/google/protobuf/compiler/cpp/cpp_enum_field.cc",
       "src/google/protobuf/compiler/cpp/cpp_enum_field.h",
       "src/google/protobuf/compiler/cpp/cpp_extension.cc",
@@ skipping 126 matching lines @@
       "src/google/protobuf/compiler/javanano/javanano_map_field.cc",
       "src/google/protobuf/compiler/javanano/javanano_map_field.h",
       "src/google/protobuf/compiler/javanano/javanano_message.cc",
       "src/google/protobuf/compiler/javanano/javanano_message.h",
       "src/google/protobuf/compiler/javanano/javanano_message_field.cc",
       "src/google/protobuf/compiler/javanano/javanano_message_field.h",
       "src/google/protobuf/compiler/javanano/javanano_primitive_field.cc",
       "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",
       "src/google/protobuf/compiler/objectivec/objectivec_enum_field.h",
       "src/google/protobuf/compiler/objectivec/objectivec_extension.cc",
       "src/google/protobuf/compiler/objectivec/objectivec_extension.h",
       "src/google/protobuf/compiler/objectivec/objectivec_field.cc",
       "src/google/protobuf/compiler/objectivec/objectivec_field.h",
       "src/google/protobuf/compiler/objectivec/objectivec_file.cc",
       "src/google/protobuf/compiler/objectivec/objectivec_file.h",
@@ skipping 31 matching lines @@
 
       # Must be after no_chromium_code for warning flags to be ordered
       # correctly.
       ":protobuf_warnings",
     ]
     if (is_win) {
       # This is defined internally, don't warn on duplicate.
       configs -= [ "//build/config/win:lean_and_mean" ]
     }
 
+    public_configs = [ ":protobuf_config" ]
+
+    cflags = protobuf_lite_cflags
+
+    public_deps = [
+      ":protobuf_full",
+    ]
+  }
+
+  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 = [
-      ":protobuf_full",
-      "//build/config/sanitizers:deps",
+      ":protoc_lib",
 
       # Default manifest on Windows (a no-op elsewhere).
       "//build/win:default_exe_manifest",
     ]
-    deps += [ "//build/config/sanitizers:deps" ]
   }
 }
 
 google_python_dir = "$root_out_dir/pyproto/google"
 
 copy("copy_google") {
   sources = [
     "__init__.py",
   ]
   outputs = [
@@ skipping 66 matching lines @@
     ":copy_google_protobuf_internal",
     ":copy_six",
   ]
 
   # Targets that depend on this should depend on the copied data files.
   data = get_target_outputs(":copy_google")
   data += get_target_outputs(":copy_six")
   data += get_target_outputs(":copy_google_protobuf")
   data += get_target_outputs(":copy_google_protobuf_internal")
 }