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]


Reply via email to