Merge pull request grpc#15595 from dgquintas/nanopb_build_cleanup

Treat nanopb as a regular external dep
mohits · Jun 15, 2018 · 1c6d6ba · 1c6d6ba
2 parents 5bc31db + 39fbed3
commit 1c6d6ba
Show file tree

Hide file tree

Showing 27 changed files with 471 additions and 68 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
diff --git a/Makefile b/Makefile
@@ -334,12 +334,10 @@ CXXFLAGS += -std=c++11
 ifeq ($(SYSTEM),Darwin)
 CXXFLAGS += -stdlib=libc++
 endif
-CPPFLAGS += -g -Wall -Wextra -Werror -Wno-long-long -Wno-unused-parameter -DOSATOMIC_USE_INLINED=1 -Wno-deprecated-declarations
+CPPFLAGS += -g -Wall -Wextra -Werror -Wno-long-long -Wno-unused-parameter -DOSATOMIC_USE_INLINED=1 -Wno-deprecated-declarations -Ithird_party/nanopb -DPB_FIELD_32BIT
 COREFLAGS += -fno-rtti -fno-exceptions
 LDFLAGS += -g
 
-DEFINES += PB_FIELD_16BIT
-
 CPPFLAGS += $(CPPFLAGS_$(CONFIG))
 CFLAGS += $(CFLAGS_$(CONFIG))
 CXXFLAGS += $(CXXFLAGS_$(CONFIG))

diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl
@@ -36,9 +36,7 @@ def if_not_windows(a):
 def _get_external_deps(external_deps):
   ret = []
   for dep in external_deps:
-    if dep == "nanopb":
-      ret += ["grpc_nanopb"]
-    elif dep == "address_sorting":
+    if dep == "address_sorting":
       ret += ["//third_party/address_sorting"]
     elif dep == "cares":
       ret += select({"//:grpc_no_ares": [],
@@ -161,24 +159,7 @@ def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], da
     linkopts = if_not_windows(["-pthread"]) + linkopts,
   )
 
-def grpc_generate_one_off_targets():
-  native.cc_library(
-    name = "grpc_nanopb",
-    hdrs = [
-      "//third_party/nanopb:pb.h",
-      "//third_party/nanopb:pb_common.h",
-      "//third_party/nanopb:pb_decode.h",
-      "//third_party/nanopb:pb_encode.h",
-    ],
-    srcs = [
-      "//third_party/nanopb:pb_common.c",
-      "//third_party/nanopb:pb_decode.c",
-      "//third_party/nanopb:pb_encode.c",
-    ],
-    defines = [
-      "PB_FIELD_16BIT=1",
-    ],
-  )
+def grpc_generate_one_off_targets(): pass
 
 def grpc_sh_test(name, srcs, args = [], data = []):
   native.sh_test(

diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl
@@ -2,6 +2,12 @@
 
 def grpc_deps():
     """Loads dependencies need to compile and test the grpc library."""
+
+    native.bind(
+        name = "nanopb",
+        actual = "@com_github_nanopb_nanopb//:nanopb",
+    )
+
     native.bind(
         name = "libssl",
         actual = "@boringssl//:ssl",
@@ -59,12 +65,12 @@ def grpc_deps():
 
     native.bind(
         name = "grpc_cpp_plugin",
-        actual = "@com_github_grpc_grpc//:grpc_cpp_plugin"
+        actual = "@com_github_grpc_grpc//:grpc_cpp_plugin",
     )
 
     native.bind(
         name = "grpc++_codegen_proto",
-        actual = "@com_github_grpc_grpc//:grpc++_codegen_proto"
+        actual = "@com_github_grpc_grpc//:grpc++_codegen_proto",
     )
 
     if "boringssl" not in native.existing_rules():
@@ -87,6 +93,14 @@ def grpc_deps():
             name = "com_google_protobuf",
             strip_prefix = "protobuf-b5fbb742af122b565925987e65c08957739976a7",
             url = "https://github.com/google/protobuf/archive/b5fbb742af122b565925987e65c08957739976a7.tar.gz",
+            )
+
+    if "com_github_nanopb_nanopb" not in native.existing_rules():
+        native.new_http_archive(
+            name = "com_github_nanopb_nanopb",
+            build_file = "@com_github_grpc_grpc//third_party:nanopb.BUILD",
+            strip_prefix = "nanopb-f8ac463766281625ad710900479130c7fcb4d63b",
+            url = "https://github.com/nanopb/nanopb/archive/f8ac463766281625ad710900479130c7fcb4d63b.tar.gz",
         )
 
     if "com_github_google_googletest" not in native.existing_rules():

diff --git a/build.yaml b/build.yaml
@@ -5625,7 +5625,7 @@ defaults:
   global:
     COREFLAGS: -fno-rtti -fno-exceptions
     CPPFLAGS: -g -Wall -Wextra -Werror -Wno-long-long -Wno-unused-parameter -DOSATOMIC_USE_INLINED=1
-      -Wno-deprecated-declarations
+      -Wno-deprecated-declarations -Ithird_party/nanopb -DPB_FIELD_32BIT
     LDFLAGS: -g
   zlib:
     CFLAGS: -Wno-sign-conversion -Wno-conversion -Wno-unused-value -Wno-implicit-function-declaration

diff --git a/cmake/nanopb.cmake b/cmake/nanopb.cmake
@@ -0,0 +1,15 @@
+# Copyright 2018 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set(_gRPC_NANOPB_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/third_party/nanopb")
diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec
@@ -663,9 +663,9 @@ Pod::Spec.new do |s|
   end
 
   s.prepare_command = <<-END_OF_COMMAND
-    find src/cpp/ -type f -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
-    find src/cpp/ -name "*.back" -type f -delete
-    find src/core/ -regex ".*\.h" -type f -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
-    find src/core/ -name "*.back" -type f -delete
+    find src/cpp/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+    find src/cpp/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
+    find src/core/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+    find src/core/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
   END_OF_COMMAND
 end
diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec
@@ -1264,6 +1264,7 @@ Pod::Spec.new do |s|
 
   # TODO (mxyan): Instead of this hack, add include path "third_party" to C core's include path?
   s.prepare_command = <<-END_OF_COMMAND
-    find src/core/ -type f ! -path '*.back*' -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
+    find src/core/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+    find src/core/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
   END_OF_COMMAND
 end
diff --git a/grpc.gyp b/grpc.gyp
@@ -59,6 +59,8 @@
       '-Wno-unused-parameter',
       '-DOSATOMIC_USE_INLINED=1',
       '-Wno-deprecated-declarations',
+      '-Ithird_party/nanopb',
+      '-DPB_FIELD_32BIT',
     ],
     'ldflags': [
       '-g',
@@ -137,6 +139,8 @@
             '-Wno-unused-parameter',
             '-DOSATOMIC_USE_INLINED=1',
             '-Wno-deprecated-declarations',
+            '-Ithird_party/nanopb',
+            '-DPB_FIELD_32BIT',
           ],
           'OTHER_CPLUSPLUSFLAGS': [
             '-g',
@@ -147,6 +151,8 @@
             '-Wno-unused-parameter',
             '-DOSATOMIC_USE_INLINED=1',
             '-Wno-deprecated-declarations',
+            '-Ithird_party/nanopb',
+            '-DPB_FIELD_32BIT',
             '-stdlib=libc++',
             '-std=c++11',
             '-Wno-error=deprecated-declarations',

diff --git a/setup.py b/setup.py
@@ -37,6 +37,7 @@
 CORE_INCLUDE = ('include', '.',)
 SSL_INCLUDE = (os.path.join('third_party', 'boringssl', 'include'),)
 ZLIB_INCLUDE = (os.path.join('third_party', 'zlib'),)
+NANOPB_INCLUDE = (os.path.join('third_party', 'nanopb'),)
 CARES_INCLUDE = (
     os.path.join('third_party', 'cares'),
     os.path.join('third_party', 'cares', 'cares'),)
@@ -181,7 +182,7 @@
 
 EXTENSION_INCLUDE_DIRECTORIES = (
     (PYTHON_STEM,) + CORE_INCLUDE + SSL_INCLUDE + ZLIB_INCLUDE +
-    CARES_INCLUDE + ADDRESS_SORTING_INCLUDE)
+    NANOPB_INCLUDE + CARES_INCLUDE + ADDRESS_SORTING_INCLUDE)
 
 EXTENSION_LIBRARIES = ()
 if "linux" in sys.platform:

diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc
@@ -18,9 +18,9 @@
 
 #include <grpc/support/port_platform.h>
 
+#include "pb_decode.h"
+#include "pb_encode.h"
 #include "src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h"
-#include "third_party/nanopb/pb_decode.h"
-#include "third_party/nanopb/pb_encode.h"
 
 #include <grpc/support/alloc.h>
 

diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/proto/grpc/lb/v1/load_balancer.pb.h b/src/core/ext/filters/client_channel/lb_policy/grpclb/proto/grpc/lb/v1/load_balancer.pb.h
@@ -3,7 +3,7 @@
 
 #ifndef PB_GRPC_LB_V1_LOAD_BALANCER_PB_H_INCLUDED
 #define PB_GRPC_LB_V1_LOAD_BALANCER_PB_H_INCLUDED
-#include "third_party/nanopb/pb.h"
+#include "pb.h"
 /* @@protoc_insertion_point(includes) */
 #if PB_PROTO_HEADER_VERSION != 30
 #error Regenerate this file with the current version of nanopb generator.

diff --git a/src/core/tsi/alts/handshaker/alts_handshaker_service_api_util.h b/src/core/tsi/alts/handshaker/alts_handshaker_service_api_util.h
@@ -21,8 +21,8 @@
 
 #include <grpc/support/port_platform.h>
 
-#include "third_party/nanopb/pb_decode.h"
-#include "third_party/nanopb/pb_encode.h"
+#include "pb_decode.h"
+#include "pb_encode.h"
 
 #include <grpc/slice.h>
 #include <grpc/slice_buffer.h>

diff --git a/src/core/tsi/alts/handshaker/altscontext.pb.h b/src/core/tsi/alts/handshaker/altscontext.pb.h
@@ -3,7 +3,7 @@
 
 #ifndef PB_GRPC_GCP_ALTSCONTEXT_PB_H_INCLUDED
 #define PB_GRPC_GCP_ALTSCONTEXT_PB_H_INCLUDED
-#include "third_party/nanopb/pb.h"
+#include "pb.h"
 #include "src/core/tsi/alts/handshaker/transport_security_common.pb.h"
 
 /* @@protoc_insertion_point(includes) */

diff --git a/src/core/tsi/alts/handshaker/handshaker.pb.h b/src/core/tsi/alts/handshaker/handshaker.pb.h
@@ -3,7 +3,7 @@
 
 #ifndef PB_GRPC_GCP_HANDSHAKER_PB_H_INCLUDED
 #define PB_GRPC_GCP_HANDSHAKER_PB_H_INCLUDED
-#include "third_party/nanopb/pb.h"
+#include "pb.h"
 #include "src/core/tsi/alts/handshaker/transport_security_common.pb.h"
 
 /* @@protoc_insertion_point(includes) */

diff --git a/src/core/tsi/alts/handshaker/transport_security_common.pb.h b/src/core/tsi/alts/handshaker/transport_security_common.pb.h
@@ -3,7 +3,7 @@
 
 #ifndef PB_GRPC_GCP_TRANSPORT_SECURITY_COMMON_PB_H_INCLUDED
 #define PB_GRPC_GCP_TRANSPORT_SECURITY_COMMON_PB_H_INCLUDED
-#include "third_party/nanopb/pb.h"
+#include "pb.h"
 /* @@protoc_insertion_point(includes) */
 #if PB_PROTO_HEADER_VERSION != 30
 #error Regenerate this file with the current version of nanopb generator.

diff --git a/src/core/tsi/alts/handshaker/transport_security_common_api.h b/src/core/tsi/alts/handshaker/transport_security_common_api.h
@@ -21,8 +21,8 @@
 
 #include <grpc/support/port_platform.h>
 
-#include "third_party/nanopb/pb_decode.h"
-#include "third_party/nanopb/pb_encode.h"
+#include "pb_decode.h"
+#include "pb_encode.h"
 
 #include <grpc/slice.h>
 #include <grpc/slice_buffer.h>

diff --git a/src/cpp/server/health/default_health_check_service.cc b/src/cpp/server/health/default_health_check_service.cc
@@ -24,10 +24,10 @@
 #include <grpc/support/log.h>
 #include <grpcpp/impl/codegen/method_handler_impl.h>
 
+#include "pb_decode.h"
+#include "pb_encode.h"
 #include "src/cpp/server/health/default_health_check_service.h"
 #include "src/cpp/server/health/health.pb.h"
-#include "third_party/nanopb/pb_decode.h"
-#include "third_party/nanopb/pb_encode.h"
 
 namespace grpc {
 namespace {

diff --git a/src/cpp/server/health/health.pb.h b/src/cpp/server/health/health.pb.h
@@ -3,7 +3,7 @@
 
 #ifndef PB_GRPC_HEALTH_V1_HEALTH_PB_H_INCLUDED
 #define PB_GRPC_HEALTH_V1_HEALTH_PB_H_INCLUDED
-#include "third_party/nanopb/pb.h"
+#include "pb.h"
 /* @@protoc_insertion_point(includes) */
 #if PB_PROTO_HEADER_VERSION != 30
 #error Regenerate this file with the current version of nanopb generator.

diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template
@@ -168,6 +168,7 @@
   include(cmake/gflags.cmake)
   include(cmake/benchmark.cmake)
   include(cmake/address_sorting.cmake)
+  include(cmake/nanopb.cmake)
 
   if(NOT MSVC)
     set(CMAKE_C_FLAGS   "<%text>${CMAKE_C_FLAGS}</%text> -std=c99")
@@ -383,6 +384,7 @@
     PRIVATE <%text>${_gRPC_CARES_INCLUDE_DIR}</%text>
     PRIVATE <%text>${_gRPC_GFLAGS_INCLUDE_DIR}</%text>
     PRIVATE <%text>${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}</%text>
+    PRIVATE <%text>${_gRPC_NANOPB_INCLUDE_DIR}</%text>
   % if lib.build in ['test', 'private'] and lib.language == 'c++':
     PRIVATE third_party/googletest/googletest/include
     PRIVATE third_party/googletest/googletest
@@ -464,6 +466,7 @@
     PRIVATE <%text>${_gRPC_CARES_INCLUDE_DIR}</%text>
     PRIVATE <%text>${_gRPC_GFLAGS_INCLUDE_DIR}</%text>
     PRIVATE <%text>${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}</%text>
+    PRIVATE <%text>${_gRPC_NANOPB_INCLUDE_DIR}</%text>
   % if tgt.build in ['test', 'private'] and tgt.language == 'c++':
     PRIVATE third_party/googletest/googletest/include
     PRIVATE third_party/googletest/googletest

diff --git a/templates/Makefile.template b/templates/Makefile.template
@@ -221,8 +221,6 @@
   %  endif
   % endfor
 
-  DEFINES += PB_FIELD_16BIT
-
   CPPFLAGS += $(CPPFLAGS_$(CONFIG))
   CFLAGS += $(CFLAGS_$(CONFIG))
   CXXFLAGS += $(CXXFLAGS_$(CONFIG))

diff --git a/templates/gRPC-C++.podspec.template b/templates/gRPC-C++.podspec.template
@@ -196,9 +196,9 @@
     end
 
     s.prepare_command = <<-END_OF_COMMAND
-      find src/cpp/ -type f -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
-      find src/cpp/ -name "*.back" -type f -delete
-      find src/core/ -regex ".*\.h" -type f -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
-      find src/core/ -name "*.back" -type f -delete
+      find src/cpp/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+      find src/cpp/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
+      find src/core/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+      find src/core/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
     END_OF_COMMAND
   end
diff --git a/templates/gRPC-Core.podspec.template b/templates/gRPC-Core.podspec.template
@@ -220,6 +220,7 @@
 
     # TODO (mxyan): Instead of this hack, add include path "third_party" to C core's include path?
     s.prepare_command = <<-END_OF_COMMAND
-      find src/core/ -type f ! -path '*.back*' -exec sed -E -i'.back' 's;#include "third_party/nanopb/(.*)";#include <nanopb/\\1>;g' {} \\\;
+      find src/core/ -type f ! -path '*.grpc_back' -print0 | xargs -0 -L1 sed -E -i'.grpc_back' 's;#include "(pb(_.*)?\\.h)";#include <nanopb/\\1>;g'
+      find src/core/ -type f -path '*.grpc_back' -print0 | xargs -0 rm
     END_OF_COMMAND
   end
diff --git a/third_party/nanopb.BUILD b/third_party/nanopb.BUILD
@@ -0,0 +1,19 @@
+cc_library(
+    name = "nanopb",
+    srcs = [
+        "pb_common.c",
+        "pb_decode.c",
+        "pb_encode.c",
+    ],
+    hdrs = [
+        "pb.h",
+        "pb_common.h",
+        "pb_decode.h",
+        "pb_encode.h",
+    ],
+    defines = [
+        "PB_FIELD_32BIT=1",
+    ],
+    visibility = ["//visibility:public"],
+)
+
diff --git a/third_party/nanopb/BUILD b/third_party/nanopb/BUILD
@@ -5,17 +5,20 @@ exports_files(["LICENSE.txt"])
 package(default_visibility = ["//visibility:public"])
 
 cc_library(
-  name = "nanopb",
-  visibility = ["//visibility:public"],
-  hdrs = [
-    "pb.h",
-    "pb_common.h",
-    "pb_decode.h",
-    "pb_encode.h",
-  ],
-  srcs = [
-    "pb_common.c",
-    "pb_decode.c",
-    "pb_encode.c",
-  ],
+    name = "nanopb",
+    srcs = [
+        "pb_common.c",
+        "pb_decode.c",
+        "pb_encode.c",
+    ],
+    hdrs = [
+        "pb.h",
+        "pb_common.h",
+        "pb_decode.h",
+        "pb_encode.h",
+    ],
+    defines = [
+        "PB_FIELD_32BIT=1",
+    ],
+    visibility = ["//visibility:public"],
 )
diff --git a/tools/codegen/core/gen_nano_proto.sh b/tools/codegen/core/gen_nano_proto.sh
@@ -74,8 +74,7 @@ pushd "$(dirname $INPUT_PROTO)" > /dev/null
 
 protoc \
 --plugin=protoc-gen-nanopb="$GRPC_ROOT/third_party/nanopb/generator/protoc-gen-nanopb" \
---nanopb_out='-T -L#include\ \"third_party/nanopb/pb.h\"'":$OUTPUT_DIR" \
-"$(basename $INPUT_PROTO)"
+--nanopb_out='-T -L#include\ \"pb.h\"'":$OUTPUT_DIR" "$(basename $INPUT_PROTO)"
 
 readonly PROTO_BASENAME=$(basename $INPUT_PROTO .proto)
 sed -i "s:$PROTO_BASENAME.pb.h:${GRPC_OUTPUT_DIR}/$PROTO_BASENAME.pb.h:g" \