subject:"\"\\\\\\\[GitHub\\\\\\\] \\\\\\\[hbase\\\\\\\-native\\\\\\\-client\\\\\\\] bharathv commented on a change in pull request #2\\\\\\\: HBASE\\\\\\\-24400\\\\\\\: Fixup cmake infrastructure to allow dependencies to be built locally\""

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-21 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r429061722



##
File path: .gitignore
##
@@ -3,6 +3,9 @@
 *.lo
 *.o
 
+# Proto files
+src/hbase/if/*.proto

Review comment:
   copying of protos can be rolled into the cmake script instead of 
manually invoking copy-protos script?

##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"

Review comment:
   Any reason you bumped up the versions from the master branch? Probably 
better to use the same versions and upgrade them in a separate commit to limit 
the scope of changes?

##
File path: cmake/DownloadWangle.cmake
##
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's wangle library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_wangle SOURCE_DIR BUILD_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-wangle-proj
+   GIT_REPOSITORY "https://github.com/facebook/wangle.git";
+   GIT_TAG "v2020.05.18.00"

Review comment:
   same as above.

##
File path: cmake/DownloadFizz.cmake
##
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's fizz library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_fizz SOURCE_DIR BUILD_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-fizz-proj
+   GIT_REPOSITORY "https://github.com/facebookincubator/fizz.git";
+   GIT_TAG "v2020.05.18.00"

Review comment:
   nit: Define all the thirdparty version variables in one place in the 
CMakeLists? Easier to make changes in the future if someone wants to try out a 
specific git tag..

##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-22 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r429485955



##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   I had libfmt-dev too but I probably messed up something. I tried again 
and I could get past that, but then I ran into this..
   
   ```
   In file included from 
hbase-native-client/build/dependencies/facebook-folly-proj-src/folly/logging/Logger.h:22:0,
from 
hbase-native-client/build/dependencies/facebook-folly-proj-src/folly/logging/BridgeFromGoogleLogging.cpp:20:
   
hbase-native-client/build/dependencies/facebook-folly-proj-src/folly/logging/LogStreamProcessor.h:19:10:
 fatal error: fmt/core.h: No such file or directory
#include 
 ^~~~
   ```
   Turns out the problem is this https://github.com/facebook/folly/pull/1263. 
libfmt-dev in ubuntu 18.04 doesn't install the right headers. If we end up 
dockerizing this on ubuntu, we may run into this problem in which case we may 
have to install fmt dependency like other dependencies, just FYI.

##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   Next problem is the "version.h"... 
   ```
   cp: cannot stat 
'../bin/../../hbase-common/target/generated-sources/native/utils//*': No such 
file or directory
   CMakeFiles/copy_version_h.dir/build.make:57: recipe for target 
'CMakeFiles/copy_version_h' failed
   make[2]: *** [CMakeFiles/copy_version_h] Error 1
   CMakeFiles/Makefile2:67: recipe for target 
'CMakeFiles/copy_version_h.dir/all' failed
   make[1]: *** [CMakeFiles/copy_version_h.dir/all] Error 2
   Makefile:140: recipe for target 'all' failed
   make: *** [all] Error 2
   ```
   
   I don't see hbase-common in the hbase repo generating a native header file. 
It only generates a Version.java file.. the following code looks sketchy to me. 
How did get the version.h in your case?
   
   ```
   # Copy the version.h generated from hbase-common/src/saveVersion.sh script 
via the mvn build
   BIN_DIR=$(dirname "$0")
   
VERSION_SOURCE_DIR="${BIN_DIR}/../../hbase-common/target/generated-sources/native/utils/"
   VERSION_DEST_DIR="${BIN_DIR}/../include/hbase/utils/"
   cp $VERSION_SOURCE_DIR/* $VERSION_DEST_DIR/ 
   ```

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-22 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r429496828



##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   Its probably worth noting, I ran into similar problems (as libfmt) with 
gmock and gtest. apt install libgtest doesn't install the sharedlibs, I had to 
manually compile and install again (weird).





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:
us...@infra.apache.org

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-26 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r430720072



##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"

Review comment:
   Agreed.

##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   > Yeah version is an issue I'd like to address with cmake getting a 
known tag. I manually created a version.h for the time being. I started working 
on a solution for this, but then the size of the PR would grow even more. 
   
   > Finally, RE the copy_version [1] agree it's reliant on a relative path 
that probably won't exist for most people. A more sustainable solution would be 
to have a variable specifying an hbase release target, and we generate the 
version file from the tag.
   
   Thanks. To keep things simple, IMHO it is also reasonable to pull the 
version.h from hbase source in the parent directory (basically the current 
approach). Just that mvn compilation in hbase project is not generating that 
header file. Let me know if you think this is a reasonable approach and I can 
fix it in a separate patch and you can integrate it with your change (that'll 
keep your changes simple for now).
   
   > My hope is to leverage an internal java project that runs a mini cluster 
and relies on maven to build a jar we can run for integration tests. Right now 
there is a relative path expecting class-path information to be valid.
   
   Agreed.
   
   > libfmt issues are one I think I need to address. In the case of gtest did 
you use libgtest-dev?
   
   Yes, I was using the dev library.  I think this is a known problem, see the 
second answer on this page.
   
https://stackoverflow.com/questions/13513905/how-to-set-up-googletest-as-a-shared-library-on-linux
   
   > but also allow the client to be used across a variety of systems without 
regard to dependencies on those machines.
   
   +1. I think an ideal end state would be a prebuilt toolchain for various 
commonly used platforms we just download them on the fly during compilation. 





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:
us...@infra.apache.org

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-27 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r431501510



##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   https://github.com/apache/hbase/pull/1794 is what I was talking about. I 
couldn't add you to the PR for review but feel free to take a look. Hope it 
helps you in this effort.





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:
us...@infra.apache.org

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

2020-05-27 Thread GitBox



bharathv commented on a change in pull request #2:
URL: https://github.com/apache/hbase-native-client/pull/2#discussion_r431501510



##
File path: cmake/DownloadFolly.cmake
##
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+## Download facebook's folly library. 
+## SOURCE_DIR is typically the cmake source directory
+## BINARY_DIR is the build directory, typically 'build'
+
+function(download_folly SOURCE_DIR BINARY_DIR)
+
+   
+   ExternalProject_Add(
+   facebook-folly-proj
+   GIT_REPOSITORY "https://github.com/facebook/folly.git";
+   GIT_TAG "v2020.05.18.00"
+   SOURCE_DIR "${BINARY_DIR}/dependencies/facebook-folly-proj-src"
+   CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}

Review comment:
   https://github.com/apache/hbase/pull/1794 is what I was talking about. I 
couldn't add you to the PR for review but feel free to talk a look. Hope it 
helps you in this effort.





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:
us...@infra.apache.org

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

[GitHub] [hbase-native-client] bharathv commented on a change in pull request #2: HBASE-24400: Fixup cmake infrastructure to allow dependencies to be built locally

6 matches

Site Navigation

Mail list logo

Footer information