Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove thrift marco in user code. #383

Merged
merged 11 commits into from
Jul 4, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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 = [
Expand All @@ -64,6 +73,12 @@ LINKOPTS = [
"//conditions:default": [
"-lrt",
],
}) + select({
":with_thrift": [
"-lthriftnb",
"-levent",
"-lthrift"],
"//conditions:default": [],
})

genrule(
Expand Down Expand Up @@ -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"
Expand Down
29 changes: 21 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -20,25 +22,24 @@ 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()

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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -366,6 +378,7 @@ set(SOURCES
${JSON2PB_SOURCES}
${MCPACK2PB_SOURCES}
${BRPC_SOURCES}
${THRIFT_SOURCES}
)

add_subdirectory(src)
Expand Down
4 changes: 2 additions & 2 deletions config_brpc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ eval set -- "$TEMP"
# Convert to abspath always so that generated mk is include-able from everywhere
while true; do
case "$1" in
--headers ) HDRS_IN="$(realpath $2)"; shift 2 ;;
--libs ) LIBS_IN="$(realpath $2)"; shift 2 ;;
--headers ) HDRS_IN="$(readlink -e $2)"; shift 2 ;;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mac下没有readlink

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已经改回去了

--libs ) LIBS_IN="$(readlink -e $2)"; shift 2 ;;
--cc ) CC=$2; shift 2 ;;
--cxx ) CXX=$2; shift 2 ;;
--with-glog ) WITH_GLOG=1; shift 1 ;;
Expand Down
2 changes: 1 addition & 1 deletion docs/cn/getting_started.md
Original file line number Diff line number Diff line change
Expand Up @@ -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+

Expand Down
3 changes: 2 additions & 1 deletion docs/cn/thrift.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
基本步骤:
Expand Down
3 changes: 2 additions & 1 deletion docs/en/thrift.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions example/echo_c++/client.cpp
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

DEFINE_string(attachment, "foo", "Carry this along with requests");
DEFINE_string(protocol, "baidu_std", "Protocol type. Defined in src/brpc/options.proto");
DEFINE_string(connection_type, "", "Connection type. Available values: single, pooled, short");
DEFINE_string(connection_type, "pooled", "Connection type. Available values: single, pooled, short");
DEFINE_string(server, "0.0.0.0:8000", "IP Address of server");
DEFINE_string(load_balancer, "", "The algorithm for load balancing");
DEFINE_string(load_balancer, "rr", "The algorithm for load balancing");
DEFINE_int32(timeout_ms, 100, "RPC timeout in milliseconds");
DEFINE_int32(max_retry, 3, "Max retries(not including the first RPC)");
DEFINE_int32(interval_ms, 1000, "Milliseconds between consecutive requests");
Expand All @@ -44,7 +44,7 @@ int main(int argc, char* argv[]) {
options.connection_type = FLAGS_connection_type;
options.timeout_ms = FLAGS_timeout_ms/*milliseconds*/;
options.max_retry = FLAGS_max_retry;
if (channel.Init(FLAGS_server.c_str(), FLAGS_load_balancer.c_str(), &options) != 0) {
if (channel.Init("file://server.list", FLAGS_load_balancer.c_str(), &options) != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里需要改回去

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

个人测试代码, 误操作, 已经回滚

LOG(ERROR) << "Fail to initialize channel";
return -1;
}
Expand Down Expand Up @@ -85,7 +85,7 @@ int main(int argc, char* argv[]) {
} else {
LOG(WARNING) << cntl.ErrorText();
}
usleep(FLAGS_interval_ms * 1000L);
//usleep(FLAGS_interval_ms * 1000L);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要该回去

}

LOG(INFO) << "EchoClient is going to quit";
Expand Down
4 changes: 2 additions & 2 deletions example/thrift_extension_c++/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ 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))
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
Expand Down
3 changes: 0 additions & 3 deletions src/brpc/details/thrift_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -120,4 +118,3 @@ bool serialize_iobuf_to_thrift_message(butil::IOBuf& body,

#endif //BRPC_THRIFT_UTILS_H

#endif //ENABLE_THRIFT_FRAMED_PROTOCOL
13 changes: 8 additions & 5 deletions src/brpc/global.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = {
Expand Down
3 changes: 0 additions & 3 deletions src/brpc/policy/thrift_protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -648,4 +646,3 @@ void RegisterThriftProtocol() {
}
}

#endif
4 changes: 0 additions & 4 deletions src/brpc/policy/thrift_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -51,6 +49,4 @@ bool VerifyThriftRequest(const InputMessageBase *msg);
} // namespace policy
} // namespace brpc


#endif // BRPC_POLICY_THRIFT_PROTOCOL_H
#endif
2 changes: 2 additions & 0 deletions src/brpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 0 additions & 3 deletions src/brpc/thrift_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -246,4 +244,3 @@ ::google::protobuf::Metadata ThriftFramedMessage::GetMetadata() const {

} // namespace brpc

#endif
3 changes: 0 additions & 3 deletions src/brpc/thrift_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

// Authors: wangxuefeng (wangxuefeng@didichuxing.com)

#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL

#ifndef BRPC_THRIFT_MESSAGE_H
#define BRPC_THRIFT_MESSAGE_H

Expand Down Expand Up @@ -167,4 +165,3 @@ class ThriftMessage : public ThriftFramedMessage {

#endif // BRPC_THRIFT_MESSAGE_H

#endif //ENABLE_THRIFT_FRAMED_PROTOCOL
2 changes: 0 additions & 2 deletions src/brpc/thrift_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -58,4 +57,3 @@ void ThriftService::Expose(const butil::StringPiece& prefix) {

} // namespace brpc

#endif
4 changes: 0 additions & 4 deletions src/brpc/thrift_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

// Authors: wangxuefeng (wangxuefeng@didichuxing.com)

#ifdef ENABLE_THRIFT_FRAMED_PROTOCOL

#ifndef BRPC_THRIFT_SERVICE_H
#define BRPC_THRIFT_SERVICE_H

Expand Down Expand Up @@ -130,5 +128,3 @@ friend class Server;

#endif // BRPC_THRIFT_SERVICE_H

#endif