phrocker commented on a change in pull request #3:
URL: https://github.com/apache/hbase-native-client/pull/3#discussion_r437478926
##########
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:
Copying the version doesn't really make sense if we rely on either
HBASE_HOME or build it within the cmake binary directory. I think the
expectation was that it would exist. I don't feel the need to keep the copy
version script around. What do you think?
Thanks for pointing this out.
##########
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:
yeah that was a good point. I had the files stored in my source tree,
but didn't realize this because they were ignored files. I initially wanted to
avoid dealing with this but as you pointed out it's impossible to build without
generating these source files and without the referenced proto files we have no
way to generate the source. The approach I took was to use the base generated
directory as an include. This means includes will change because Client.pb.h is
no longer in a root folder it is instead within the original relative path. In
the case of Client.pb.h it would have to be included as include
<client/Client.pb.h> as opposed to <hbase/if/Client.pb.h> hence the source
changes.
----------------------------------------------------------------
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]