Repository: kudu
Updated Branches:
  refs/heads/master a72ed600c -> e7e65954b


[thirdparty] Make Boost a regular dependency

For the new TIMESTAMP type we must mimic Impala's
TimestampValue format to make sure that the types have
the same memory format. This will allow to avoid costly
conversions at scan time. Impala's type uses boost's ptime
and nanosecond resolution and so must Kudu.

Previously our use of boost was header-only, but the
date_time library (which includes ptime) needs to be compiled.
This requires that we actually compile libboost_date_time in
thirparty. This patch does that and also changes the way we
handle boost to treat it more like a regular dependency, both
regarding the new static and shared library files and regarding
headers.

Since boost uses its own build system this required tinkering
with jamfiles to make sure that the library is correctly
built under tsan.

Finally this patch also addresses a couple of other issues:
- Adds "thirdparty/installed/common/include" to the include
dirs explicitly (we were doing this indirectly when we added
the boost include dir before).
- Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR
to point to wherever boost was installed, this removes the
logic that makes sure we didn't pick up boost from the
system path.

Change-Id: I277c4fda15575e271c426735552762884cb28c43
Reviewed-on: http://gerrit.cloudera.org:8080/5818
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <[email protected]>
Reviewed-by: Adar Dembo <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/e7e65954
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/e7e65954
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/e7e65954

Branch: refs/heads/master
Commit: e7e65954bdc7992d281151fa36c1218e8417da08
Parents: a72ed60
Author: David Alves <[email protected]>
Authored: Thu Feb 2 21:39:04 2017 -0800
Committer: Adar Dembo <[email protected]>
Committed: Tue Feb 7 21:16:46 2017 +0000

----------------------------------------------------------------------
 CMakeLists.txt                    | 38 ++++++++++-------------
 cmake_modules/FindKuduBoost.cmake | 46 ++++++++++++++++++++++++++++
 thirdparty/build-definitions.sh   | 56 ++++++++++++++++++++++++++++++++--
 thirdparty/build-thirdparty.sh    | 12 +++++---
 4 files changed, 124 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/e7e65954/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f9b191b..c03c86e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -126,17 +126,24 @@ endif()
 
 # compiler flags that are common across debug/release builds
 #  - msse4.2: Enable sse4.2 compiler intrinsics.
+set(CXX_COMMON_FLAGS "-msse4.2")
 #  - Wall: Enable all warnings.
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wall")
 #  - Wno-sign-compare: suppress warnings for comparison between signed and 
unsigned
 #    integers
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-sign-compare")
 #  -Wno-deprecated: some of the gutil code includes old things like 
ext/hash_set, ignore that
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-deprecated")
 #  - pthread: enable multithreaded malloc
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -pthread")
 #  -fno-strict-aliasing
 #     Assume programs do not follow strict aliasing rules.
 #     GCC cannot always verify whether strict aliasing rules are indeed 
followed due to
 #     fundamental limitations in escape analysis, which can result in subtle 
bad code generation.
 #     This has a small perf hit but worth it to avoid hard to debug crashes.
-set(CXX_COMMON_FLAGS "-fno-strict-aliasing -msse4.2 -Wall -Wno-sign-compare 
-Wno-deprecated -pthread")
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -fno-strict-aliasing")
+#  -DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG: enable nanosecond precision for 
boost
+set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} 
-DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG")
 
 # We want access to the PRI* print format macros.
 add_definitions(-D__STDC_FORMAT_MACROS)
@@ -459,6 +466,7 @@ file(MAKE_DIRECTORY "${EXECUTABLE_OUTPUT_PATH}")
 
 include_directories(${CMAKE_CURRENT_BINARY_DIR}/src)
 include_directories(src)
+include_directories(SYSTEM ${THIRDPARTY_INSTALL_COMMON_DIR}/include)
 
 ############################################################
 # Visibility
@@ -956,28 +964,14 @@ endif()
 
 ## Boost
 
-# The boost-cmake project hasn't been maintained for years. Let's make sure we
-# don't accidentally use it if it can be found.
-set(Boost_NO_BOOST_CMAKE ON)
+# We use a custom cmake module and not cmake's FindBoost.
+# see: cmake_modules/FindKuduBoost.cmake
+find_package(KuduBoost REQUIRED)
+include_directories(SYSTEM ${BOOST_INCLUDE_DIR})
 
-# Look for Boost in thirdparty only.
-set(BOOST_ROOT ${THIRDPARTY_INSTALL_COMMON_DIR})
-
-# As of cmake 3.5, there's still no way to force FindBoost.cmake to omit system
-# paths from its search. No combination of Boost_NO_SYSTEM_PATHS, BOOST_ROOT, 
or
-# BOOST_INCLUDEDIR does the trick; the best they can do is ensure the system
-# paths are searched last.
-#
-# To prevent system boost installations from being picked up, we'll work around
-# this ourselves by checking the prefix of Boost_INCLUDE_DIR.
-find_package(Boost REQUIRED)
-string(FIND ${Boost_INCLUDE_DIR} ${BOOST_ROOT} BOOST_ROOT_IDX_FOUND)
-if(${BOOST_ROOT_IDX_FOUND} EQUAL 0)
-  message(STATUS "Boost location: ${Boost_INCLUDE_DIR}")
-else()
-  message(FATAL_ERROR "Error: chose Boost outside ${BOOST_ROOT}: 
${Boost_INCLUDE_DIR}")
-endif()
-include_directories(SYSTEM ${Boost_INCLUDE_DIR})
+ADD_THIRDPARTY_LIB(boost_date_time
+    STATIC_LIB "${BOOST_DATE_TIME_STATIC_LIB}"
+    SHARED_LIB "${BOOST_DATE_TIME_SHARED_LIB}")
 
 ############################################################
 # Linker setup

http://git-wip-us.apache.org/repos/asf/kudu/blob/e7e65954/cmake_modules/FindKuduBoost.cmake
----------------------------------------------------------------------
diff --git a/cmake_modules/FindKuduBoost.cmake 
b/cmake_modules/FindKuduBoost.cmake
new file mode 100644
index 0000000..3b1cea4
--- /dev/null
+++ b/cmake_modules/FindKuduBoost.cmake
@@ -0,0 +1,46 @@
+# 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.
+#
+# Find Boost's libs (currently date_time) and makes sure boost includes are 
available.
+#
+# We use a custom cmake module instead of cmake's FindBoost as otherwise we'd 
have to:
+# - Run cmake's FindBoost twice to make sure it picks up the shared and static 
version of the lib.
+# - Have a check to make sure we're not picking up the system's boost.
+#
+# Less (and simpler) cmake lines are required than if we were to work around 
cmake's FindBoost
+# behavior.
+#
+# This module defines
+#  BOOST_DATE_TIME_SHARED_LIB, path to boost's date_time shared library
+#  BOOST_DATE_TIME_STATIC_LIB, path to boost's date_time static library
+#  BOOST_INCLUDE_DIR, patch to boost's headers
+#  BOOST_DATE_TIME_FOUND, whether boost has been found
+
+find_path(BOOST_INCLUDE_DIR boost/bind.hpp
+    NO_CMAKE_SYSTEM_PATH
+    NO_SYSTEM_ENVIRONMENT_PATH)
+
+find_library(BOOST_DATE_TIME_SHARED_LIB boost_date_time
+    NO_CMAKE_SYSTEM_PATH
+    NO_SYSTEM_ENVIRONMENT_PATH)
+find_library(BOOST_DATE_TIME_STATIC_LIB libboost_date_time.a
+    NO_CMAKE_SYSTEM_PATH
+    NO_SYSTEM_ENVIRONMENT_PATH)
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(BOOST_DATE_TIME REQUIRED_VARS
+    BOOST_DATE_TIME_SHARED_LIB BOOST_DATE_TIME_STATIC_LIB BOOST_INCLUDE_DIR)
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/kudu/blob/e7e65954/thirdparty/build-definitions.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-definitions.sh b/thirdparty/build-definitions.sh
index 7860e4d..66c062e 100644
--- a/thirdparty/build-definitions.sh
+++ b/thirdparty/build-definitions.sh
@@ -616,6 +616,58 @@ build_nvml() {
 }
 
 build_boost() {
-  # This is a header-only installation of Boost.
-  rsync -a --delete $BOOST_SOURCE/boost $PREFIX/include
+  local BUILD_TYPE=$1
+  pushd $BOOST_SOURCE
+  local TOOLSET="" # By default, use default toolset.
+  # Boost only looks for this file here and in the user's $HOME directory.
+  # To avoid adding this file to the source tree we would have to patch the 
auto-generated
+  # project-config.jam that is generated by ./bootstrap which is a worse 
option.
+  # This file is cleaned up before and after the build as to reduce the risk 
of polluting
+  # the source tree.
+  local USER_JAMFILE=./tools/build/src/user-config.jam
+
+  if [ -e "$USER_JAMFILE" ]; then
+    rm $USER_JAMFILE
+  fi
+
+  BOOST_BDIR=$TP_BUILD_DIR/boost-$BOOST_VERSION$MODE_SUFFIX
+
+  # Compile with PIC and set nanosecond resolution for impala compatibility.
+  BOOST_CFLAGS="$EXTRA_CFLAGS -fPIC -DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG"
+  BOOST_CXXFLAGS="$EXTRA_CXXFLAGS -fPIC 
-DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG"
+  BOOST_LDFLAGS="$EXTRA_LDFLAGS"
+
+
+  case $BUILD_TYPE in
+    "normal")
+      # Default: use the default toolset.
+      ;;
+    "tsan")
+      # We're using a custom clang, so we specify its path in the jamfile.
+      echo "using clang : : $CXX ;" > $USER_JAMFILE
+      echo "User jamfile location: $USER_JAMFILE"
+      echo "User jamfile contents:"
+      cat $USER_JAMFILE
+      TOOLSET="toolset=clang"
+      BOOST_LDFLAGS="-stdlib=libc++ $BOOST_LDFLAGS"
+      ;;
+    *)
+      echo "Unknown Boost build type: $BUILD_TYPE"
+      exit 1
+      ;;
+  esac
+
+  # Build the date_time boost lib.
+  ./bootstrap.sh --prefix=$PREFIX threading=multi --with-libraries=date_time
+  ./b2 clean $TOOLSET --build-dir="$BOOST_BDIR"
+  ./b2 install variant=release link=static,shared --build-dir="$BOOST_BDIR" 
$TOOLSET -q -d0 \
+    --debug-configuration \
+    cflags="$BOOST_CFLAGS" \
+    cxxflags="$BOOST_CXXFLAGS" \
+    linkflags="$BOOST_LDFLAGS"
+
+  if [ -e "$USER_JAMFILE" ]; then
+    rm $USER_JAMFILE
+  fi
+  popd
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/e7e65954/thirdparty/build-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-thirdparty.sh b/thirdparty/build-thirdparty.sh
index e6340a7..4ce4271 100755
--- a/thirdparty/build-thirdparty.sh
+++ b/thirdparty/build-thirdparty.sh
@@ -196,10 +196,6 @@ if [ -n "$F_COMMON" -o -n "$F_TRACE_VIEWER" ]; then
   build_trace_viewer
 fi
 
-if [ -n "$F_COMMON" -o -n "$F_BOOST" ]; then
-  build_boost
-fi
-
 ### Build C dependencies without instrumentation
 
 PREFIX=$PREFIX_DEPS
@@ -289,6 +285,10 @@ if [ -n "$F_UNINSTRUMENTED" -o -n "$F_CRCUTIL" ]; then
   build_crcutil
 fi
 
+if [ -n "$F_UNINSTRUMENTED" -o -n "$F_BOOST" ]; then
+  build_boost normal
+fi
+
 restore_env
 
 # If we're on MacOs best to exit here, otherwise single dependency builds will 
try to
@@ -448,6 +448,10 @@ if [ -n "$F_TSAN" -o -n "$F_CRCUTIL" ]; then
   build_crcutil
 fi
 
+if [ -n "$F_TSAN" -o -n "$F_BOOST" ]; then
+  build_boost tsan
+fi
+
 restore_env
 
 finish

Reply via email to