bharathv commented on a change in pull request #3:
URL: https://github.com/apache/hbase-native-client/pull/3#discussion_r436221065
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
Review comment:
nit: extraneous new lines.
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
Review comment:
Also, it looks like this is all or nothing, meaning it either downloads
everything or downloads nothing. I think a nice follow up would be to have
overrides on a package basis (for future :-)).
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
Review comment:
nit: indentation off, we use 2/4 style usually. 2 for subsections and 4
for continuation indents..
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
Review comment:
I think we can get rid of this. If HBASE_HOME is set, I think we can
pick everything up from there, otherwise set HBASE_HOME is set to the
downloaded tag?
##########
File path: CMakeLists.txt
##########
@@ -50,10 +143,23 @@ if (OPENSSL_FOUND)
else ()
message( FATAL_ERROR "OpenSSL was not found. Please install OpenSSL" )
endif (OPENSSL_FOUND)
+## Download Facebook Folly and build locally
+
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ download_folly(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR})
+ download_wangle(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR})
+ download_zookeeper(${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR})
+endif(BUILD_LOCAL_DEPENDENCIES)
+
# ensure we have required dependencies
find_package(Threads)
-find_package(Boost REQUIRED COMPONENTS thread system filesystem)
+find_package(Boost REQUIRED COMPONENTS context thread system filesystem regex)
+find_package(LibEvent REQUIRED)
find_package(Gflags REQUIRED)
+if (BUILD_LOCAL_DEPENDENCIES)
+ find_package(Sodium REQUIRED)
Review comment:
not sure I followed this, we need it irrespective of whether the
dependencies are downloaded right? (for folly)
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
Review comment:
nit: Move versions/git repos to global variables? (just like HBASE stuff)
##########
File path: cmake/patches/zookeeper.3.4.x.cli
##########
@@ -0,0 +1,4 @@
+559c559
Review comment:
Is this used anywhere? Doesn't look like it..
##########
File path: CMakeLists.txt
##########
@@ -109,37 +217,51 @@ generate_protobuf_src(PROTO_SOURCES PROTO_HEADERS
${PROTO_FILES})
add_library(hbaseclient-static STATIC ${PROTO_SOURCES} ${CLIENT_SRC}
${CONNECTION_SRC} ${EXCEPTION_SRC} ${PROTO_SRC} ${SECURITY_SRC} ${SRDE_SRC}
${UTILS_SRC})
set_target_properties(hbaseclient-static PROPERTIES LINKER_LANGUAGE CXX)
SET_TARGET_PROPERTIES(hbaseclient-static PROPERTIES OUTPUT_NAME hbaseclient
CLEAN_DIRECT_OUTPUT 1)
-target_link_libraries(hbaseclient-static ${PROTOBUF_LIBRARY})
-target_link_libraries(hbaseclient-static ${FOLLY_LIBRARIES})
target_link_libraries(hbaseclient-static ${Boost_LIBRARIES})
target_link_libraries(hbaseclient-static ${SASL_LIBS})
-target_link_libraries(hbaseclient-static ${GLOG_SHARED_LIB})
target_link_libraries(hbaseclient-static ${GFLAGS_SHARED_LIB})
target_link_libraries(hbaseclient-static ${KRB5_LIBRARIES})
target_link_libraries(hbaseclient-static ${WANGLE_LIBRARIES})
-target_link_libraries(hbaseclient-static ${Zookeeper_LIBRARIES})
+if (BUILD_LOCAL_DEPENDENCIES)
+ target_link_libraries(hbaseclient-static ${sodium_LIBRARY_RELEASE})
Review comment:
nit: same question as above
##########
File path: cmake/patches/fizz.v2020.05.18.00.cmake
##########
@@ -0,0 +1,18 @@
+36c36
Review comment:
unused?
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
Review comment:
nit: call it DOWNLOAD_DEPENDENCIES? Current naming gives an impression
that it is building already local dependencies somewhere.
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
Review comment:
nit: I think these are wangle stuff? Prefix accordingly to be more
readable? Also, quick comment would be nice..
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
+
+ set(PROTOBUF_DIR "${Protobuf_BINARY_DIR}" CACHE STRING "" FORCE)
+
+ add_library(Protobuf STATIC IMPORTED)
+ set_target_properties(Protobuf PROPERTIES IMPORTED_LOCATION
"${Protobuf_BINARY_DIR}/lib/libprotobuf.a" )
+ set(PROTOBUF_LIBS "${Protobuf_BINARY_DIR}/lib/libprotobuf.a"
"${Protobuf_BINARY_DIR}/lib/libprotoc.a" CACHE STRING "" FORCE)
+ set(PROTOBUF_INCLUDE_DIRS "${Protobuf_BINARY_DIR}/include" CACHE STRING
"" FORCE)
+ add_dependencies(Protobuf Protobuf-download)
+ set(PROTOBUF_FOUND TRUE CACHE STRING "" FORCE)
+
+ set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_BINARY_DIR}/bin/protoc"
CACHE STRING "" FORCE)
+ ## Add CMAKE_MODULE_PATHS
+
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/local")
+
+ ## Build Apache HBase components that are necessary for this project
+
+ if( BUILD_HBASE )
+ ## Download Apache HBase, and build hbase-common so that we can
have a targeted build of version.h
+ download_project(PROJ apachehbase
+ IS_MAVEN
+ MAVEN_DIR "hbase-common"
+ GIT_REPOSITORY "https://github.com/apache/hbase.git";
+ GIT_TAG "${HBASE_TARGET_TAG}")
+
+
include_directories("${CMAKE_CURRENT_BINARY_DIR}/apachehbase-src/hbase-common/target/generated-sources/native/")
+ endif(BUILD_HBASE)
+else()
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/system")
+endif(BUILD_LOCAL_DEPENDENCIES)
+
+############
+## Validate that we have C++ 14 support
+############
+
+set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
+
+
+CHECK_CXX_COMPILER_FLAG("-std=c++14" COMPILER_SUPPORTS_CXX14)
+CHECK_CXX_COMPILER_FLAG("-std=c++0x" COMPILER_SUPPORTS_CXX0X)
if(COMPILER_SUPPORTS_CXX14)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14")
+elseif(COMPILER_SUPPORTS_CXX0X)
Review comment:
Does this mean we are supporting supporting earlier versions?
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
+
+ set(PROTOBUF_DIR "${Protobuf_BINARY_DIR}" CACHE STRING "" FORCE)
+
+ add_library(Protobuf STATIC IMPORTED)
+ set_target_properties(Protobuf PROPERTIES IMPORTED_LOCATION
"${Protobuf_BINARY_DIR}/lib/libprotobuf.a" )
+ set(PROTOBUF_LIBS "${Protobuf_BINARY_DIR}/lib/libprotobuf.a"
"${Protobuf_BINARY_DIR}/lib/libprotoc.a" CACHE STRING "" FORCE)
+ set(PROTOBUF_INCLUDE_DIRS "${Protobuf_BINARY_DIR}/include" CACHE STRING
"" FORCE)
+ add_dependencies(Protobuf Protobuf-download)
+ set(PROTOBUF_FOUND TRUE CACHE STRING "" FORCE)
+
+ set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_BINARY_DIR}/bin/protoc"
CACHE STRING "" FORCE)
+ ## Add CMAKE_MODULE_PATHS
+
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/local")
+
+ ## Build Apache HBase components that are necessary for this project
+
+ if( BUILD_HBASE )
+ ## Download Apache HBase, and build hbase-common so that we can
have a targeted build of version.h
+ download_project(PROJ apachehbase
+ IS_MAVEN
+ MAVEN_DIR "hbase-common"
+ GIT_REPOSITORY "https://github.com/apache/hbase.git";
+ GIT_TAG "${HBASE_TARGET_TAG}")
+
+
include_directories("${CMAKE_CURRENT_BINARY_DIR}/apachehbase-src/hbase-common/target/generated-sources/native/")
+ endif(BUILD_HBASE)
+else()
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/system")
+endif(BUILD_LOCAL_DEPENDENCIES)
+
+############
+## Validate that we have C++ 14 support
+############
+
+set(CMAKE_CXX_STANDARD 14)
Review comment:
Move this before doing anything else?
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
+
+ set(PROTOBUF_DIR "${Protobuf_BINARY_DIR}" CACHE STRING "" FORCE)
+
+ add_library(Protobuf STATIC IMPORTED)
+ set_target_properties(Protobuf PROPERTIES IMPORTED_LOCATION
"${Protobuf_BINARY_DIR}/lib/libprotobuf.a" )
+ set(PROTOBUF_LIBS "${Protobuf_BINARY_DIR}/lib/libprotobuf.a"
"${Protobuf_BINARY_DIR}/lib/libprotoc.a" CACHE STRING "" FORCE)
+ set(PROTOBUF_INCLUDE_DIRS "${Protobuf_BINARY_DIR}/include" CACHE STRING
"" FORCE)
+ add_dependencies(Protobuf Protobuf-download)
+ set(PROTOBUF_FOUND TRUE CACHE STRING "" FORCE)
+
+ set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_BINARY_DIR}/bin/protoc"
CACHE STRING "" FORCE)
+ ## Add CMAKE_MODULE_PATHS
+
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/local")
+
+ ## Build Apache HBase components that are necessary for this project
+
+ if( BUILD_HBASE )
Review comment:
IMO, this shouldn't be nested inside BUILD_LOCAL_DEPENDENCIES. General
usecase is to keep the regular dependencies static (folly..etc) but change
versions of HBase we are building with (for example for newer releases).
Thoughts?
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
Review comment:
nit: use a human readable tag?
##########
File path: CMakeLists.txt
##########
@@ -50,10 +143,23 @@ if (OPENSSL_FOUND)
else ()
message( FATAL_ERROR "OpenSSL was not found. Please install OpenSSL" )
endif (OPENSSL_FOUND)
+## Download Facebook Folly and build locally
+
+
Review comment:
nit: extraneous new lines...
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
+
+ set(PROTOBUF_DIR "${Protobuf_BINARY_DIR}" CACHE STRING "" FORCE)
+
+ add_library(Protobuf STATIC IMPORTED)
+ set_target_properties(Protobuf PROPERTIES IMPORTED_LOCATION
"${Protobuf_BINARY_DIR}/lib/libprotobuf.a" )
+ set(PROTOBUF_LIBS "${Protobuf_BINARY_DIR}/lib/libprotobuf.a"
"${Protobuf_BINARY_DIR}/lib/libprotoc.a" CACHE STRING "" FORCE)
+ set(PROTOBUF_INCLUDE_DIRS "${Protobuf_BINARY_DIR}/include" CACHE STRING
"" FORCE)
+ add_dependencies(Protobuf Protobuf-download)
+ set(PROTOBUF_FOUND TRUE CACHE STRING "" FORCE)
+
+ set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_BINARY_DIR}/bin/protoc"
CACHE STRING "" FORCE)
+ ## Add CMAKE_MODULE_PATHS
+
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/local")
+
+ ## Build Apache HBase components that are necessary for this project
+
+ if( BUILD_HBASE )
+ ## Download Apache HBase, and build hbase-common so that we can
have a targeted build of version.h
+ download_project(PROJ apachehbase
+ IS_MAVEN
+ MAVEN_DIR "hbase-common"
+ GIT_REPOSITORY "https://github.com/apache/hbase.git";
+ GIT_TAG "${HBASE_TARGET_TAG}")
+
+
include_directories("${CMAKE_CURRENT_BINARY_DIR}/apachehbase-src/hbase-common/target/generated-sources/native/")
+ endif(BUILD_HBASE)
+else()
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/system")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/system")
+endif(BUILD_LOCAL_DEPENDENCIES)
+
+############
+## Validate that we have C++ 14 support
+############
+
+set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
+
+
+CHECK_CXX_COMPILER_FLAG("-std=c++14" COMPILER_SUPPORTS_CXX14)
+CHECK_CXX_COMPILER_FLAG("-std=c++0x" COMPILER_SUPPORTS_CXX0X)
if(COMPILER_SUPPORTS_CXX14)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14")
+elseif(COMPILER_SUPPORTS_CXX0X)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x")
else()
- message(STATUS "The compiler ${CMAKE_CXX_COMPILER} has no C++14 support.
Please use a different C++ compiler.")
+ message(FATAL "The compiler ${CMAKE_CXX_COMPILER} has no c++14 support.
Please use a different C++ compiler.")
endif()
-set(CMAKE_CXX_STANDARD 14)
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
+
+if (BUILD_HBASE)
+ find_package(Java REQUIRED)
+ find_package(Maven REQUIRED)
+endif(BUILD_HBASE)
+
+
set(HBASE_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/src/hbase")
Review comment:
This should be inferred from the downloaded hbase git tag right? (unless
it is overriden).. I'm running into this..
```
CMake Error at cmake/ProtobufGen.cmake:22 (message):
Error: generate_protobuf_src() called without any proto files
Call Stack (most recent call first):
CMakeLists.txt:215 (generate_protobuf_src)
```
Essentially this should be from the downloaded path IIUC...
##########
File path: CMakeLists.txt
##########
@@ -162,12 +284,29 @@ add_custom_target(
linter
COMMAND ${CMAKE_SOURCE_DIR}/bin/cpplint.sh)
# Copy the version.h file in before doing anything
-add_custom_target (
- copy_version_h
- COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/bin/copy-version.sh"
-)
-add_dependencies(hbaseclient-static copy_version_h)
-add_dependencies(hbaseclient-shared copy_version_h)
+if (NOT BUILD_HBASE)
+ add_custom_target (
+ copy_version_h
+ COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/bin/copy-version.sh"
+ )
+
+ add_dependencies(hbaseclient-static copy_version_h)
+ add_dependencies(hbaseclient-shared copy_version_h)
+
+endif(NOT BUILD_HBASE)
Review comment:
This fails if BUILD_HBASE is false right? (we don't add version.h
includes)... I think the code should be structured like this..
set HBASE_SRC_DIR if it is already set by user or set it to the downloaded
path in the beginning of the script
Rest of the script shouldn't care about BUILD_HBASE ... they should add
dependencies from the set HBASE_SRC_DIR path...
##########
File path: CMakeLists.txt
##########
@@ -162,12 +284,29 @@ add_custom_target(
linter
COMMAND ${CMAKE_SOURCE_DIR}/bin/cpplint.sh)
# Copy the version.h file in before doing anything
-add_custom_target (
- copy_version_h
- COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/bin/copy-version.sh"
-)
-add_dependencies(hbaseclient-static copy_version_h)
-add_dependencies(hbaseclient-shared copy_version_h)
+if (NOT BUILD_HBASE)
+ add_custom_target (
Review comment:
nit: indentation...
##########
File path: CMakeLists.txt
##########
@@ -25,15 +25,108 @@ set(PROJECT_VERSION_PATCH 0)
set(BUILD_SHARED_LIBS ON)
## set our cmake module path
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+include(CMakeDependentOption)
+include(CheckIncludeFile)
+include(ExternalProject)
+include(DownloadProject)
+include(CheckCXXCompilerFlag)
+
+
+option(BUILD_LOCAL_DEPENDENCIES "Downloads and builds all dependencies locally
" OFF)
+option(HBASE_TARGET_TAG "Downloads and builds all dependencies locally"
"e5345b3a7c32c6a80394319c17540b84c8fe66ba")
+option(BUILD_HBASE "Builds Hbase " OFF)
+
+
+
+######### Includes
+
+
+if (WIN32)
+ set(BYPRODUCT_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_PREFIX "" CACHE STRING "" FORCE)
+ set(BUILD_ARGS " -GVisual Studio 15 2017")
+else()
+ set(BYPRODUCT_PREFIX "lib" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SHARED_SUFFIX ".so" CACHE STRING "" FORCE)
+ set(BYPRODUCT_SUFFIX ".a" CACHE STRING "" FORCE)
+endif()
+
+
+
## include the Protobuf generation code
include(ProtobufGen)
+include(DownloadFolly)
+include(DownloadWangle)
+include(DownloadZookeeper)
+
+if (BUILD_LOCAL_DEPENDENCIES)
+ ## we want to find the system protoc
+ download_project(PROJ Protobuf
+ IS_AUTOGEN
+ GIT_REPOSITORY
"https://github.com/protocolbuffers/protobuf.git";
+ GIT_TAG "3.5.1.1")
+
+ set(PROTOBUF_DIR "${Protobuf_BINARY_DIR}" CACHE STRING "" FORCE)
+
+ add_library(Protobuf STATIC IMPORTED)
+ set_target_properties(Protobuf PROPERTIES IMPORTED_LOCATION
"${Protobuf_BINARY_DIR}/lib/libprotobuf.a" )
+ set(PROTOBUF_LIBS "${Protobuf_BINARY_DIR}/lib/libprotobuf.a"
"${Protobuf_BINARY_DIR}/lib/libprotoc.a" CACHE STRING "" FORCE)
+ set(PROTOBUF_INCLUDE_DIRS "${Protobuf_BINARY_DIR}/include" CACHE STRING
"" FORCE)
+ add_dependencies(Protobuf Protobuf-download)
+ set(PROTOBUF_FOUND TRUE CACHE STRING "" FORCE)
+
+ set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_BINARY_DIR}/bin/protoc"
CACHE STRING "" FORCE)
+ ## Add CMAKE_MODULE_PATHS
+
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/zookeeper/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/protobuf/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/folly/local")
+ list(APPEND CMAKE_MODULE_PATH
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/wangle/local")
+
+ ## Build Apache HBase components that are necessary for this project
+
+ if( BUILD_HBASE )
+ ## Download Apache HBase, and build hbase-common so that we can
have a targeted build of version.h
+ download_project(PROJ apachehbase
+ IS_MAVEN
+ MAVEN_DIR "hbase-common"
Review comment:
nit: Same comment as above, move versions, repos, tags etc to global
variables?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
