You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by majetideepak <gi...@git.apache.org> on 2017/07/10 21:50:15 UTC
[GitHub] orc pull request #135: Update and use CMake External Project to build compre...
GitHub user majetideepak opened a pull request:
https://github.com/apache/orc/pull/135
Update and use CMake External Project to build compression libraries
Including the whole source of external libraries adds bloat.
It also is not useful if clients prefer to use their own third-party libraries.
In this PR
1) The libraries are updated to the most recent ones.
2) CMake `ExternalProject_Add` is used to build the libraries
Files to review
```
cmake_modules/ThirdpartyToolchain.cmake
c++/libs/CMakeLists.txt
CMakeLists.txt
```
Rest of the changes are just deletes of the libraries
I will file a JIRA to enable users the ability to provide their own libraries.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/majetideepak/orc ORC-204
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/orc/pull/135.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #135
----
commit 0bc85be0a7ba60dc53ab402a5a278503cf98f97d
Author: Deepak Majeti <de...@hpe.com>
Date: 2017-07-10T16:43:47Z
Update and use CMake External Project to build compression libraries
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/orc/pull/135
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by wgtmac <gi...@git.apache.org>.
Github user wgtmac commented on a diff in the pull request:
https://github.com/apache/orc/pull/135#discussion_r127358830
--- Diff: cmake_modules/ThirdpartyToolchain.cmake ---
@@ -13,17 +13,18 @@ set (SNAPPY_HOME "${SNAPPY_PREFIX}")
set (SNAPPY_INCLUDE_DIRS "${SNAPPY_PREFIX}/include")
set (SNAPPY_STATIC_LIB_NAME snappy)
set (SNAPPY_STATIC_LIB "${SNAPPY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX}")
-set (SNAPPY_SRC_URL "${CMAKE_SOURCE_DIR}/c++/libs/snappy-${SNAPPY_VERSION}.tar.gz")
+set (SNAPPY_SRC_URL "https://github.com/google/snappy/releases/download/1.1.4/snappy-${SNAPPY_VERSION}.tar.gz")
--- End diff --
this line contains a hard-coded version 1.1.4
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by majetideepak <gi...@git.apache.org>.
Github user majetideepak commented on a diff in the pull request:
https://github.com/apache/orc/pull/135#discussion_r127487866
--- Diff: cmake_modules/ThirdpartyToolchain.cmake ---
@@ -13,17 +13,18 @@ set (SNAPPY_HOME "${SNAPPY_PREFIX}")
set (SNAPPY_INCLUDE_DIRS "${SNAPPY_PREFIX}/include")
set (SNAPPY_STATIC_LIB_NAME snappy)
set (SNAPPY_STATIC_LIB "${SNAPPY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX}")
-set (SNAPPY_SRC_URL "${CMAKE_SOURCE_DIR}/c++/libs/snappy-${SNAPPY_VERSION}.tar.gz")
+set (SNAPPY_SRC_URL "https://github.com/google/snappy/releases/download/1.1.4/snappy-${SNAPPY_VERSION}.tar.gz")
--- End diff --
Good catch! Fixing it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc issue #135: ORC-204: Update and use CMake ExternalProject_Add to build c...
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on the issue:
https://github.com/apache/orc/pull/135
Ok, I committed this with only a slight tweak to also filter out the shared libraries on MacOS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc issue #135: ORC-204: Update and use CMake ExternalProject_Add to build c...
Posted by majetideepak <gi...@git.apache.org>.
Github user majetideepak commented on the issue:
https://github.com/apache/orc/pull/135
Fixed packaging static compression libraries. Only the source is available for Snappy 1.1.6 and I could not `install`(build works fine) this release. CMake support was introduced but only shared libraries are being built. Snappy 1.1.5 and 1.1.6 are the same. There is no performance difference between the current 1.1.4 and 1.1.6
https://github.com/google/snappy/releases
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by wgtmac <gi...@git.apache.org>.
Github user wgtmac commented on a diff in the pull request:
https://github.com/apache/orc/pull/135#discussion_r126828652
--- Diff: cmake_modules/ThirdpartyToolchain.cmake ---
@@ -0,0 +1,90 @@
+set (LZ4_VERSION "1.7.5")
+set (SNAPPY_VERSION "1.1.3")
+set (ZLIB_VERSION "1.2.11")
+set (THIRDPARTY_DIR "${CMAKE_BINARY_DIR}/c++/libs/thirdparty")
+
+string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_BUILD_TYPE)
+
+# ----------------------------------------------------------------------
+# Snappy
+
+set (SNAPPY_PREFIX "${THIRDPARTY_DIR}/snappy_ep-install")
+set (SNAPPY_HOME "${SNAPPY_PREFIX}")
+set (SNAPPY_INCLUDE_DIRS "${SNAPPY_PREFIX}/include")
+set (SNAPPY_STATIC_LIB_NAME snappy)
+set (SNAPPY_STATIC_LIB "${SNAPPY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX}")
+set (SNAPPY_SRC_URL "${CMAKE_SOURCE_DIR}/c++/libs/snappy-${SNAPPY_VERSION}.tar.gz")
+if (${UPPERCASE_BUILD_TYPE} EQUAL "RELEASE")
+ set (SNAPPY_CXXFLAGS "CXXFLAGS='-DNDEBUG -O2'")
+endif ()
+
+ExternalProject_Add (snappy_ep
+ CONFIGURE_COMMAND ./configure "--prefix=${SNAPPY_PREFIX}" ${SNAPPY_CXXFLAGS}
+ BUILD_IN_SOURCE 1
+ BUILD_COMMAND ${MAKE}
+ INSTALL_DIR ${SNAPPY_PREFIX}
+ URL ${SNAPPY_SRC_URL}
--- End diff --
Just curious if it is better to put an HTTP URL here to download compression libraries? In this approach, we can reduce the repo size and easy to bump library version in the future.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by majetideepak <gi...@git.apache.org>.
Github user majetideepak commented on a diff in the pull request:
https://github.com/apache/orc/pull/135#discussion_r126829942
--- Diff: cmake_modules/ThirdpartyToolchain.cmake ---
@@ -0,0 +1,90 @@
+set (LZ4_VERSION "1.7.5")
+set (SNAPPY_VERSION "1.1.3")
+set (ZLIB_VERSION "1.2.11")
+set (THIRDPARTY_DIR "${CMAKE_BINARY_DIR}/c++/libs/thirdparty")
+
+string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_BUILD_TYPE)
+
+# ----------------------------------------------------------------------
+# Snappy
+
+set (SNAPPY_PREFIX "${THIRDPARTY_DIR}/snappy_ep-install")
+set (SNAPPY_HOME "${SNAPPY_PREFIX}")
+set (SNAPPY_INCLUDE_DIRS "${SNAPPY_PREFIX}/include")
+set (SNAPPY_STATIC_LIB_NAME snappy)
+set (SNAPPY_STATIC_LIB "${SNAPPY_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX}")
+set (SNAPPY_SRC_URL "${CMAKE_SOURCE_DIR}/c++/libs/snappy-${SNAPPY_VERSION}.tar.gz")
+if (${UPPERCASE_BUILD_TYPE} EQUAL "RELEASE")
+ set (SNAPPY_CXXFLAGS "CXXFLAGS='-DNDEBUG -O2'")
+endif ()
+
+ExternalProject_Add (snappy_ep
+ CONFIGURE_COMMAND ./configure "--prefix=${SNAPPY_PREFIX}" ${SNAPPY_CXXFLAGS}
+ BUILD_IN_SOURCE 1
+ BUILD_COMMAND ${MAKE}
+ INSTALL_DIR ${SNAPPY_PREFIX}
+ URL ${SNAPPY_SRC_URL}
--- End diff --
+1. I mentioned using HTTP URL(download the tar files) as future work because I wasn't sure of its impact on current users. I will update to use URLs in this patch if there are no concerns.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] orc pull request #135: ORC-204: Update and use CMake ExternalProject_Add to ...
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on a diff in the pull request:
https://github.com/apache/orc/pull/135#discussion_r127830348
--- Diff: cmake_modules/ThirdpartyToolchain.cmake ---
@@ -0,0 +1,90 @@
+set (LZ4_VERSION "1.7.5")
+set (SNAPPY_VERSION "1.1.3")
+set (ZLIB_VERSION "1.2.11")
--- End diff --
Let's put these variable sets in the top level CMakeLists.txt file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---