Merge pull request #383 from kenshinxf/kenshinxf

Remove thrift marco in user code.

BUILD

@@ -10,6 +10,12 @@ config_setting(
     visibility = ["//visibility:public"],
 )
 
+config_setting(
+    name = "with_thrift",
+    define_values = {"with_thrift": "true"},
+    visibility = ["//visibility:public"],
+)
+
 config_setting(
     name = "unittest",
     define_values = {"unittest": "true"},
@@ -40,6 +46,9 @@ COPTS = [
 ] + select({
     ":with_glog": ["-DBRPC_WITH_GLOG=1"],
     "//conditions:default": ["-DBRPC_WITH_GLOG=0"],
+}) + select({
+    ":with_thrift": ["-DENABLE_THRIFT_FRAMED_PROTOCOL=1"],
+    "//conditions:default": [""],
 })
 
 LINKOPTS = [
@@ -64,6 +73,12 @@ LINKOPTS = [
     "//conditions:default": [
       "-lrt",
     ],
+}) + select({
+    ":with_thrift": [
+        "-lthriftnb",
+        "-levent",
+        "-lthrift"],
+    "//conditions:default": [],
 })
 
 genrule(
@@ -443,7 +458,17 @@ cc_library(
     srcs = glob([
         "src/brpc/*.cpp",
         "src/brpc/**/*.cpp",
-    ]),
+    ],
+    exclude = [
+        "src/brpc/thrift_service.cpp",
+        "src/brpc/thrift_message.cpp",
+        "src/brpc/policy/thrift_protocol.cpp",
+    ]) + select({
+        ":with_thrift" : glob([
+            "src/brpc/thrift*.cpp",
+            "src/brpc/**/thrift*.cpp"]),
+        "//conditions:default" : [],
+    }),
     hdrs = glob([
         "src/brpc/*.h",
         "src/brpc/**/*.h"

CMakeLists.txt

@@ -2,7 +2,9 @@ cmake_minimum_required(VERSION 2.8.10)
 project(brpc C CXX)
 
 # Enable MACOSX_RPATH. Run "cmake --help-policy CMP0042" for policy details.
-cmake_policy(SET CMP0042 NEW)
+if(POLICY CMP0042)
+    cmake_policy(SET CMP0042 NEW)
+endif()
 
 set(BRPC_VERSION 0.9.0)
 
@@ -20,14 +22,14 @@ else()
     message(WARNING "You are using an unsupported compiler! Compilation has only been tested with Clang and GCC.")
 endif()
 
-option(BRPC_WITH_GLOG "With glog" OFF)
+option(WITH_GLOG "With glog" OFF)
 option(DEBUG "Print debug logs" OFF)
 option(WITH_DEBUG_SYMBOLS "With debug symbols" ON)
-option(BRPC_WITH_THRIFT "With thrift framed protocol supported" OFF)
+option(WITH_THRIFT "With thrift framed protocol supported" OFF)
 option(BUILD_UNIT_TESTS "Whether to build unit tests" OFF)
 
 set(WITH_GLOG_VAL "0")
-if(BRPC_WITH_GLOG)
+if(WITH_GLOG)
     set(WITH_GLOG_VAL "1")
 endif()
 
@@ -35,10 +37,9 @@ if(WITH_DEBUG_SYMBOLS)
     set(DEBUG_SYMBOL "-g")
 endif()
 
-if(BRPC_WITH_THRIFT)
+if(WITH_THRIFT)
     set(THRIFT_CPP_FLAG "-DENABLE_THRIFT_FRAMED_PROTOCOL")
     set(THRIFT_LIB "thriftnb")
-    message("Enable thrift framed procotol")
 endif()
 
 include(GNUInstallDirs)
@@ -119,7 +120,7 @@ if ((NOT LEVELDB_INCLUDE_PATH) OR (NOT LEVELDB_LIB))
     message(FATAL_ERROR "Fail to find leveldb")
 endif()
 
-if(BRPC_WITH_GLOG)
+if(WITH_GLOG)
     find_path(GLOG_INCLUDE_PATH NAMES glog/logging.h)
     find_library(GLOG_LIB NAMES glog)
     if((NOT GLOG_INCLUDE_PATH) OR (NOT GLOG_LIB))
@@ -162,7 +163,7 @@ set(DYNAMIC_LIB
     )
 set(BRPC_PRIVATE_LIBS "-lgflags -lprotobuf -lleveldb -lprotoc -lssl -lcrypto -ldl -lz")
 
-if(BRPC_WITH_GLOG)
+if(WITH_GLOG)
     set(DYNAMIC_LIB ${DYNAMIC_LIB} ${GLOG_LIB})
     set(BRPC_PRIVATE_LIBS "${BRPC_PRIVATE_LIBS} -lglog")
 endif()
@@ -326,6 +327,17 @@ file(GLOB_RECURSE BVAR_SOURCES "${CMAKE_SOURCE_DIR}/src/bvar/*.cpp")
 file(GLOB_RECURSE BTHREAD_SOURCES "${CMAKE_SOURCE_DIR}/src/bthread/*.cpp")
 file(GLOB_RECURSE JSON2PB_SOURCES "${CMAKE_SOURCE_DIR}/src/json2pb/*.cpp")
 file(GLOB_RECURSE BRPC_SOURCES "${CMAKE_SOURCE_DIR}/src/brpc/*.cpp")
+file(GLOB_RECURSE THRIFT_SOURCES "thrift*.cpp")
+
+if(WITH_THRIFT)
+    message("brpc compile with thrift proctol")
+else()
+    # Remove thrift sources
+    foreach(v ${THRIFT_SOURCES})
+        list(REMOVE_ITEM BRPC_SOURCES ${v})
+    endforeach()
+    set(THRIFT_SOURCES "")
+endif()
 
 set(MCPACK2PB_SOURCES
     ${CMAKE_SOURCE_DIR}/src/mcpack2pb/field_type.cpp
@@ -366,6 +378,7 @@ set(SOURCES
     ${JSON2PB_SOURCES}
     ${MCPACK2PB_SOURCES}
     ${BRPC_SOURCES}
+    ${THRIFT_SOURCES}
     )
 
 add_subdirectory(src)

docs/cn/getting_started.md

@@ -345,7 +345,7 @@ When you remove tcmalloc, not only remove the linkage with tcmalloc but also the
 
 ## glog: 3.3+
 
-brpc implements a default [logging utility](../../src/butil/logging.h) which conflicts with glog. To replace this with glog, add *--with-glog* to config_brpc.sh or add `-DBRPC_WITH_GLOG=ON` to cmake.
+brpc implements a default [logging utility](../../src/butil/logging.h) which conflicts with glog. To replace this with glog, add *--with-glog* to config_brpc.sh or add `-DWITH_GLOG=ON` to cmake.
 
 ## valgrind: 3.8+
 

docs/cn/thrift.md

@@ -28,8 +28,9 @@ sudo make install
 配置brpc支持thrift协议后make。编译完成后会生成libbrpc.a, 其中包含了支持thrift协议的扩展代码, 像正常使用brpc的代码一样链接即可。
 ```bash
 sh config_brpc.sh --headers=/usr/include --libs=/usr/lib --with-thrift
+#或者使用cmake
+mkdir build && cd build && cmake ../ -DWITH_THRIFT=1
 ```
-注意: 在编译用户代码的时候, 请定义ENABLE_THRIFT_FRAMED_PROTOCOL宏, 否则在include brpc中thrift协议相关头文件后, 实际并不生效
 
 # Client端访问thrift server
 基本步骤：

docs/en/thrift.md

@@ -28,8 +28,9 @@ sudo make install
 Config brpc with thrift support, then make. The compiled libbrpc.a includes extended code for thrift support and can be linked normally as in other brpc projects.
 ```bash
 sh config_brpc.sh --headers=/usr/include --libs=/usr/lib --with-thrift
+#or use cmake
+mkdir build && cd build && cmake ../ -DWITH_THRIFT=1
 ```
-PS: Please Define ENABLE_THRIFT_FRAMED_PROTOCOL Marco in user code in order to make it work when including thrift utils headers in brpc.
 
 # Client accesses thrift server
 Steps：

example/thrift_extension_c++/Makefile

@@ -3,7 +3,7 @@ include $(BRPC_PATH)/config.mk
 # Notes on the flags:
 # 1. Added -fno-omit-frame-pointer: perf/tcmalloc-profiler use frame pointers by default
 # 2. Added -D__const__= : Avoid over-optimizations of TLS variables by GCC>=4.8
-CXXFLAGS = -std=c++0x -g -DENABLE_THRIFT_FRAMED_PROTOCOL -DDEBUG -D__const__= -pipe -W -Wall -Werror -Wno-unused-parameter -fPIC -fno-omit-frame-pointer
+CXXFLAGS = -std=c++0x -g -DDEBUG -D__const__= -pipe -W -Wall -Werror -Wno-unused-parameter -fPIC -fno-omit-frame-pointer
 HDRS+=$(BRPC_PATH)/output/include
 LIBS+=$(BRPC_PATH)/output/lib
 HDRPATHS = $(addprefix -I, $(HDRS))
@@ -11,7 +11,7 @@ LIBPATHS = $(addprefix -L, $(LIBS))
 COMMA=,
 SOPATHS=$(addprefix -Wl$(COMMA)-rpath=, $(LIBS))
 
-STATIC_LINKINGS += -lbrpc -lthrift -lgflags -Wl,--whole-archive -Wl,--no-whole-archive -levent
+STATIC_LINKINGS += -lthrift -lgflags -lbrpc -levent
 
 CLIENT_SOURCES = client.cpp
 SERVER_SOURCES = server.cpp

src/brpc/details/thrift_utils.h

@@ -14,8 +14,6 @@
 
 // utils for serialize/parse thrift binary message to brpc protobuf obj.
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #ifndef BRPC_THRIFT_UTILS_H
 #define BRPC_THRIFT_UTILS_H
 
@@ -120,4 +118,3 @@ bool serialize_iobuf_to_thrift_message(butil::IOBuf& body,
 
 #endif //BRPC_THRIFT_UTILS_H
 
-#endif //ENABLE_THRIFT_FRAMED_PROTOCOL

src/brpc/global.cpp

@@ -77,7 +77,10 @@
 extern "C" {
 // defined in gperftools/malloc_extension_c.h
 void BAIDU_WEAK MallocExtension_ReleaseFreeMemory(void);
-void BAIDU_WEAK RegisterThriftProtocol();
+// Register Thrift Protocol if thrift was enabled
+#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
+    void RegisterThriftProtocol();
+#endif
 }
 
 namespace brpc {
@@ -466,10 +469,10 @@ static void GlobalInitializeOrDieImpl() {
         exit(1);
     }
 
-    // Register Thrift framed protocol if linked
-    if (RegisterThriftProtocol) {
-        RegisterThriftProtocol();
-    }
+// Use Macro is more straight forward than weak link technology(becasue of static link issue)
+#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
+    RegisterThriftProtocol();
+#endif
 
     // Only valid at client side
     Protocol ubrpc_compack_protocol = {

src/brpc/policy/thrift_protocol.cpp

@@ -14,8 +14,6 @@
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #include <google/protobuf/descriptor.h>         // MethodDescriptor
 #include <google/protobuf/message.h>            // Message
 #include <gflags/gflags.h>
@@ -648,4 +646,3 @@ void RegisterThriftProtocol() {
 }
 }
 
-#endif

src/brpc/policy/thrift_protocol.h

@@ -14,8 +14,6 @@
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #ifndef BRPC_POLICY_THRIFT_PROTOCOL_H
 #define BRPC_POLICY_THRIFT_PROTOCOL_H
 
@@ -51,6 +49,4 @@ bool VerifyThriftRequest(const InputMessageBase *msg);
 } // namespace policy
 } // namespace brpc
 
-
 #endif // BRPC_POLICY_THRIFT_PROTOCOL_H
-#endif

src/brpc/server.cpp

@@ -40,7 +40,9 @@
 #include "brpc/details/ssl_helper.h"           // CreateServerSSLContext
 #include "brpc/protocol.h"                     // ListProtocols
 #include "brpc/nshead_service.h"               // NsheadService
+#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
 #include "brpc/thrift_service.h"               // ThriftService
+#endif
 #include "brpc/builtin/bad_method_service.h"   // BadMethodService
 #include "brpc/builtin/get_favicon_service.h"
 #include "brpc/builtin/get_js_service.h"

src/brpc/thrift_message.cpp

@@ -14,8 +14,6 @@
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #define INTERNAL_SUPPRESS_PROTOBUF_FIELD_DEPRECATION
 #include "brpc/thrift_message.h"
 
@@ -246,4 +244,3 @@ void ThriftFramedMessage::Swap(ThriftFramedMessage* other) {
 
 } // namespace brpc
 
-#endif

src/brpc/thrift_message.h

@@ -14,8 +14,6 @@
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #ifndef BRPC_THRIFT_MESSAGE_H
 #define BRPC_THRIFT_MESSAGE_H
 
@@ -167,4 +165,3 @@ private:
 
 #endif // BRPC_THRIFT_MESSAGE_H
 
-#endif //ENABLE_THRIFT_FRAMED_PROTOCOL

src/brpc/thrift_service.cpp

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
 
 #include "butil/class_name.h"
 #include "brpc/thrift_service.h"
@@ -58,4 +57,3 @@ void ThriftService::Expose(const butil::StringPiece& prefix) {
 
 } // namespace brpc
 
-#endif

src/brpc/thrift_service.h

@@ -14,8 +14,6 @@
 
 // Authors: wangxuefeng (wangxuefeng@didichuxing.com)
 
-#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL
-
 #ifndef BRPC_THRIFT_SERVICE_H
 #define BRPC_THRIFT_SERVICE_H
 
@@ -130,5 +128,3 @@ private:
 
 #endif // BRPC_THRIFT_SERVICE_H
 
-#endif
-