Skip to content

Commit

Permalink
KUDU-46: build exported client library
Browse files Browse the repository at this point in the history
There are three parts to this commit:
1. Sprinkling of KUDU_EXPORT on those classes we want clients to interact
   with directly. KUDU_NO_EXPORT was also used to hide some nested classes.
2. CMake logic for a new build type: the exported client library.
   a. All symbols are hidden by default, so that only the symbols in the
      whitelist (specified by KUDU_EXPORT) are available for dynamic linking.
   b. Some thirdparty libraries are linked as 'local' via linker version
      script. Notable exceptions are glog and gflags, the former of which is
      needed by the client headers directly and the latter of which is
      sufficiently difficult to differentiate from the former.
   c. The build itself is static even though the library is shared: in order
      to merge _all_ dependencies into the library, we need them to be
      available in static archive form.
   d. This kind of client library is quite special. It's got all transitive
      dependencies linked in, and many of its symbols are hidden. As such, a
      bunch of unit tests won't run properly, and some can't even build.
   e. Why can't we just build the client library a second time with special
      flags? Why do we need a new build type? The problem stems from symbol
      visibility: in order to "hide everything by default", we need to
      recompile every source file that's part of the library with
      -fvisibility=hidden. That's enough code that whitelisting it all is
      hard; by contrast, a new build type is easy.
3. CMake logic for "make install", which produces a directory tree with
   the client library and all necessary headers. I chose to explicitly list
   every header instead of globbing, so that we can maintain fine-grained
   control over the list and so we'll know exactly what we're shipping.

Note that the exported library is only buildable on systems with boost<1.42.
On newer systems, there's still a dependency on an -fPIC libboost_system
which nobody has.

Change-Id: I0a99647563bbf58d3e293dfebb5c124d94288eef
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/3682
Tested-by: jenkins
Reviewed-by: Todd Lipcon <[email protected]>
  • Loading branch information
adembo authored and toddlipcon committed Aug 6, 2014
1 parent 37dc989 commit 872aba4
Show file tree
Hide file tree
Showing 19 changed files with 273 additions and 50 deletions.
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ build/
.metadata/
*.iml

# CMake-generated library export headers
*_export.h

# CMake-generated "make install" file
install_manifest.txt

# VIM/emacs stuff
*.swp
*~
Expand Down
32 changes: 31 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ set(CXX_COMMON_FLAGS "-msse4.2 -Wall -Wno-sign-compare -Wno-deprecated -pthread
# Allow linking of static libs into dynamic libs
add_definitions(-fPIC)

# We can safely use gtest.
add_definitions(-DKUDU_HEADERS_USE_GTEST=1)

# compiler flags for different build types (run 'cmake -DCMAKE_BUILD_TYPE=<type> .')
# For all builds:
# For CMAKE_BUILD_TYPE=Debug
Expand Down Expand Up @@ -124,6 +127,29 @@ else()
string(SUBSTRING "${KUDU_LINK}" 0 1 KUDU_LINK)
endif()

# For generate_export_header() and add_compiler_export_flags().
include(GenerateExportHeader)

if(KUDU_EXPORTED_CLIENT)
# Even though the exported client is a shared library, it needs
# intermediate static archives.
if("${KUDU_LINK}" STREQUAL "a")
message("Using static linking for exported client")
set(KUDU_LINK "s")
elseif("${KUDU_LINK}" STREQUAL "d")
message(SEND_ERROR "Cannot build exported client with dynamic linking")
endif()

# Compile all code with visibility flags:
# - default for classes annotated with KUDU_EXPORT.
# - hidden for classes annotated with KUDU_NO_EXPORT.
# - hidden for everything else.
add_compiler_export_flags()
else()
# Make all symbols visible, even ones explicitly annotated with KUDU_NO_EXPORT.
add_definitions("-DKUDU_STATIC_DEFINE")
endif()

# Sanity check sanitizers; ASAN and TSAN imply static and dynamic linking
# respectively, so we can't use both at once.
if ("${KUDU_USE_ASAN}" AND "${KUDU_USE_TSAN}")
Expand Down Expand Up @@ -448,10 +474,14 @@ ADD_THIRDPARTY_LIB(squeasel
##
## Disabled with TSAN/ASAN as well as with gold+dynamic linking (see comment
## near definition of KUDU_USING_GOLD).
##
## Also disabled with the exported client; we don't want to force users to
## use tcmalloc.
find_package(GPerf REQUIRED)
if (NOT "${KUDU_USE_ASAN}" AND
NOT "${KUDU_USE_TSAN}" AND
NOT ("${KUDU_USING_GOLD}" AND "${KUDU_LINK}" STREQUAL "d"))
NOT ("${KUDU_USING_GOLD}" AND "${KUDU_LINK}" STREQUAL "d") AND
NOT "${KUDU_EXPORTED_CLIENT}")
ADD_THIRDPARTY_LIB(tcmalloc
STATIC_LIB "${TCMALLOC_STATIC_LIB}"
SHARED_LIB "${TCMALLOC_LIBS}/libtcmalloc.so")
Expand Down
142 changes: 137 additions & 5 deletions src/kudu/client/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,148 @@ set(CLIENT_SRCS
write_op.cc
)

add_library(client ${CLIENT_SRCS})
target_link_libraries(client
if (KUDU_EXPORTED_CLIENT)
# We're building a shared library and providing all transitive
# dependencies on the link line. As such, the dependencies should
# not be visible to consumers of the 'client' target.
set(CLIENT_LINKAGE "SHARED")
set(CLIENT_LIBS ${KUDU_BASE_LIBS})
set(CLIENT_LIBS_VISIBILITY "LINK_PRIVATE")
endif()

add_library(client ${CLIENT_LINKAGE} ${CLIENT_SRCS})
set(CLIENT_LIBS
kudu_common
master_proto
tserver_proto
tserver_service_proto
kudu_util
gutil
krpc)
krpc
${CLIENT_LIBS})

if (KUDU_EXPORTED_CLIENT)
# Hide symbols using a linker version script.
set_target_properties(client
PROPERTIES LINK_FLAGS -Wl,--version-script=symbols.map)
endif()
target_link_libraries(client ${CLIENT_LIBS_VISIBILITY} ${CLIENT_LIBS})

# Generate kudu_export.h.
generate_export_header(client
BASE_NAME kudu
EXPORT_FILE_NAME ${CMAKE_SOURCE_DIR}/src/kudu/gutil/kudu_export.h)

# "make install" invocations to generate a directory tree containing the
# exported client library and all of its headers.
if (KUDU_EXPORTED_CLIENT)
# For CMAKE_INSTALL_<dir> variables.
include(GNUInstallDirs)

# Shared library.
install(TARGETS client
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR})

# Headers: gutil
install(FILES
../gutil/arm_instruction_set_select.h
../gutil/atomic_refcount.h
../gutil/atomicops.h
../gutil/atomicops-internals-x86.h
../gutil/basictypes.h
../gutil/bind.h
../gutil/bind_helpers.h
../gutil/bind_internal.h
../gutil/callback.h
../gutil/callback_forward.h
../gutil/callback_internal.h
../gutil/casts.h
../gutil/dynamic_annotations.h
../gutil/gscoped_ptr.h
../gutil/gtest.h
../gutil/int128.h
../gutil/integral_types.h
../gutil/kudu_export.h
../gutil/logging-inl.h
../gutil/macros.h
../gutil/move.h
../gutil/port.h
../gutil/raw_scoped_refptr_mismatch_checker.h
../gutil/ref_counted.h
../gutil/template_util.h
../gutil/tuple.h
../gutil/type_traits.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/gutil)

# Headers: gutil/hash
install(FILES
../gutil/hash/builtin_type_hash.h
../gutil/hash/city.h
../gutil/hash/hash.h
../gutil/hash/hash128to64.h
../gutil/hash/jenkins.h
../gutil/hash/jenkins_lookup2.h
../gutil/hash/legacy_hash.h
../gutil/hash/string_hash.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/gutil/hash)

# Headers: gutil/strings
install(FILES
../gutil/strings/fastmem.h
../gutil/strings/stringpiece.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/gutil/strings)

# Headers: gutil/threading
install(FILES
../gutil/threading/thread_collision_warner.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/gutil/threading)

# Headers: client
install(FILES
client.h
encoded_key.h
row_result.h
scan_predicate.h
schema.h
write_op.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/client)

# Headers: common
install(FILES
../common/partial_row.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/common)

# Headers: util
install(FILES
../util/faststring.h
../util/monotime.h
../util/slice.h
../util/status.h
../util/status_callback.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/kudu/util)

# Headers: gtest (thirdparty)
install(FILES
${GTEST_INCLUDE_DIR}/gtest/gtest_prod.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/gtest)

# Headers: glog (thirdparty)
install(FILES
${GLOG_INCLUDE_DIR}/glog/logging.h
${GLOG_INCLUDE_DIR}/glog/log_severity.h
${GLOG_INCLUDE_DIR}/glog/vlog_is_on.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/glog)

# Headers: gflags (thirdparty)
install(FILES
${GFLAGS_INCLUDE_DIR}/gflags/gflags.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/gflags)
endif()

# Tests
set(KUDU_TEST_LINK_LIBS client integration-tests ${KUDU_MIN_TEST_LIBS})
ADD_KUDU_TEST(client-test)

# Can't be used with the exported client; it references hidden symbols.
if (NOT KUDU_EXPORTED_CLIENT)
set(KUDU_TEST_LINK_LIBS client integration-tests ${KUDU_MIN_TEST_LIBS})
ADD_KUDU_TEST(client-test)
endif()
38 changes: 19 additions & 19 deletions src/kudu/client/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
#include <tr1/unordered_set>
#include <vector>

#include <gtest/gtest_prod.h>

#include "kudu/client/scan_predicate.h"
#include "kudu/client/schema.h"
#include "kudu/client/write_op.h"
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/gutil/gtest.h"
#include "kudu/gutil/kudu_export.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/util/monotime.h"
#include "kudu/util/status.h"
Expand All @@ -35,7 +35,7 @@ class RemoteTabletServer;
//
// Note that KuduClients are shared amongst multiple threads and, as such,
// are stored in shared pointers.
class KuduClientBuilder {
class KUDU_EXPORT KuduClientBuilder {
public:
KuduClientBuilder();
~KuduClientBuilder();
Expand All @@ -58,7 +58,7 @@ class KuduClientBuilder {
// returned.
Status Build(std::tr1::shared_ptr<KuduClient>* client);
private:
class Data;
class KUDU_NO_EXPORT Data;

gscoped_ptr<Data> data_;

Expand Down Expand Up @@ -89,7 +89,7 @@ class KuduClientBuilder {
// as well.
//
// This class is thread-safe.
class KuduClient : public std::tr1::enable_shared_from_this<KuduClient> {
class KUDU_EXPORT KuduClient : public std::tr1::enable_shared_from_this<KuduClient> {
public:
gscoped_ptr<KuduTableCreator> NewTableCreator();

Expand Down Expand Up @@ -140,7 +140,7 @@ class KuduClient : public std::tr1::enable_shared_from_this<KuduClient> {
const MonoDelta& default_admin_operation_timeout() const;

private:
class Data;
class KUDU_NO_EXPORT Data;

friend class KuduClientBuilder;
friend class KuduScanner;
Expand All @@ -163,7 +163,7 @@ class KuduClient : public std::tr1::enable_shared_from_this<KuduClient> {
};

// Creates a new table with the desired options.
class KuduTableCreator {
class KUDU_EXPORT KuduTableCreator {
public:
~KuduTableCreator();

Expand Down Expand Up @@ -199,7 +199,7 @@ class KuduTableCreator {
// returned.
Status Create();
private:
class Data;
class KUDU_NO_EXPORT Data;

friend class KuduClient;

Expand All @@ -218,7 +218,7 @@ class KuduTableCreator {
// and the schema fetched for introspection.
//
// This class is thread-safe.
class KuduTable : public base::RefCountedThreadSafe<KuduTable> {
class KUDU_EXPORT KuduTable : public base::RefCountedThreadSafe<KuduTable> {
public:
~KuduTable();

Expand All @@ -234,7 +234,7 @@ class KuduTable : public base::RefCountedThreadSafe<KuduTable> {
KuduClient* client() const;

private:
class Data;
class KUDU_NO_EXPORT Data;

friend class KuduClient;

Expand All @@ -254,7 +254,7 @@ class KuduTable : public base::RefCountedThreadSafe<KuduTable> {
// alterer->table_name("table-name");
// alterer->add_nullable_column("col1", UINT32);
// alterer->Alter();
class KuduTableAlterer {
class KUDU_EXPORT KuduTableAlterer {
public:
~KuduTableAlterer();

Expand Down Expand Up @@ -295,7 +295,7 @@ class KuduTableAlterer {
// TODO: Add Edit column

private:
class Data;
class KUDU_NO_EXPORT Data;

friend class KuduClient;

Expand All @@ -308,7 +308,7 @@ class KuduTableAlterer {

// An error which occurred in a given operation. This tracks the operation
// which caused the error, along with whatever the actual error was.
class KuduError {
class KUDU_EXPORT KuduError {
public:
~KuduError();

Expand All @@ -333,7 +333,7 @@ class KuduError {
bool was_possibly_successful() const;

private:
class Data;
class KUDU_NO_EXPORT Data;

friend class internal::Batcher;
friend class KuduSession;
Expand Down Expand Up @@ -402,7 +402,7 @@ class KuduError {
// concept of a Session familiar.
//
// This class is not thread-safe except where otherwise specified.
class KuduSession : public std::tr1::enable_shared_from_this<KuduSession> {
class KUDU_EXPORT KuduSession : public std::tr1::enable_shared_from_this<KuduSession> {
public:
~KuduSession();

Expand Down Expand Up @@ -578,7 +578,7 @@ class KuduSession : public std::tr1::enable_shared_from_this<KuduSession> {
KuduClient* client() const;

private:
class Data;
class KUDU_NO_EXPORT Data;

friend class KuduClient;
friend class internal::Batcher;
Expand All @@ -592,10 +592,8 @@ class KuduSession : public std::tr1::enable_shared_from_this<KuduSession> {

// A single scanner. This class is not thread-safe, though different
// scanners on different threads may share a single KuduTable object.
class KuduScanner {
class KUDU_EXPORT KuduScanner {
public:
class Data;

// The possible read modes for clients.
enum ReadMode {
// When READ_LATEST is specified the server will execute the read independently
Expand Down Expand Up @@ -692,6 +690,8 @@ class KuduScanner {
std::string ToString() const;

private:
class KUDU_NO_EXPORT Data;

gscoped_ptr<Data> data_;

DISALLOW_COPY_AND_ASSIGN(KuduScanner);
Expand Down
Loading

0 comments on commit 872aba4

Please sign in to comment.