fix ci and comments

facebookincubator · Sep 4, 2024 · c1e5220 · c1e5220
1 parent ce859a2
commit c1e5220
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 105 deletions.
diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml
@@ -54,7 +54,7 @@ jobs:
       CCACHE_DIR: '${{ github.workspace }}/.ccache'
       # The arm runners have only 7GB RAM
       BUILD_TYPE: "${{ matrix.os ==  'macos-14' && 'Release' || 'Debug' }}"
-      INSTALL_PREFIX=/tmp/deps-install
+      INSTALL_PREFIX: "/tmp/deps-install"
     steps:
       - name: Checkout
         uses: actions/checkout@v4

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -46,11 +46,11 @@ if(DEFINED ENV{CONDA_PREFIX})
 endif()
 
 if(DEFINED ENV{INSTALL_PREFIX})
-    message(STATUS "Install directory set to: $ENV{INSTALL_PREFIX}" )
-    list(APPEND CMAKE_PREFIX_PATH "$ENV{INSTALL_PREFIX}")
+  message(STATUS "Dependency install directory set to: $ENV{INSTALL_PREFIX}")
+  list(APPEND CMAKE_PREFIX_PATH "$ENV{INSTALL_PREFIX}")
+  include_directories(BEFORE "$ENV{INSTALL_PREFIX}/include")
 endif()
 
-
 list(PREPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/CMake"
      "${PROJECT_SOURCE_DIR}/CMake/third-party")
 

diff --git a/README.md b/README.md
@@ -87,16 +87,16 @@ dependencies for a given platform.
 The following setup scripts use the `DEPENDENCY_DIR` environment variable to set the
 location to download and build packages. This defaults to `deps-download` in the current
 working directory. Use `INSTALL_PREFIX` to set the install directory of the packages.
-This defaults to `deps-install` in the current working directory on MacOS and to the 
+This defaults to `deps-install` in the current working directory on macOS and to the
 default install location on linux.
 
-For MacOS, you will have to manually add the INSTALL_PREFIX value say
+For macOS, you will have to manually add the INSTALL_PREFIX value say
 `export INSTALL_PREFIX=/Users/$USERNAME/velox/deps-install` to `~/.zshrc` so that
 subsequent builds can pick it up.
 
 ### Setting up on macOS
 
-On a MacOS machine (either Intel or Apple silicon) you can setup and then build like so:
+On a macOS machine (either Intel or Apple silicon) you can setup and then build like so:
 
 ```shell
 $ ./scripts/setup-macos.sh
@@ -137,7 +137,7 @@ $ ./scripts/setup-adapters.sh
 $ make
 ```
 
-Note that `setup-adapters.sh` supports MacOS and Ubuntu 20.04 or later.
+Note that `setup-adapters.sh` supports macOS and Ubuntu 20.04 or later.
 
 ### Using Clang on Linux
 

diff --git a/scripts/adapters.dockerfile b/scripts/adapters.dockerfile
@@ -15,6 +15,7 @@
 ARG image=ghcr.io/facebookincubator/velox-dev:centos9
 FROM $image
 
+COPY scripts/setup-helper-functions.sh /
 COPY scripts/setup-adapters.sh /
 RUN mkdir build && ( cd build &&  source /opt/rh/gcc-toolset-12/enable && \
     bash /setup-adapters.sh ) && rm -rf build && dnf remove -y conda && dnf clean all

diff --git a/scripts/setup-adapters.sh b/scripts/setup-adapters.sh
@@ -117,16 +117,16 @@ function install_azure-storage-sdk-cpp {
     sed -i "s/\"version-string\"/\"builtin-baseline\": \"$vcpkg_commit_id\",\"version-string\"/" $azure_core_dir/vcpkg.json
     sed -i "s/\"version-string\"/\"overrides\": [{ \"name\": \"openssl\", \"version-string\": \"$openssl_version\" }],\"version-string\"/" $azure_core_dir/vcpkg.json
   fi
-  cmake_install $azure_core_dir -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir $azure_core_dir -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
 
   # install azure-storage-common
-  cmake_install sdk/storage/azure-storage-common -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir sdk/storage/azure-storage-common -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
 
   # install azure-storage-blobs
-  cmake_install sdk/storage/azure-storage-blobs -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir sdk/storage/azure-storage-blobs -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
 
   # install azure-storage-files-datalake
-  cmake_install sdk/storage/azure-storage-files-datalake -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir sdk/storage/azure-storage-files-datalake -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=OFF
 }
 
 function install_hdfs_deps {
@@ -144,7 +144,8 @@ function install_hdfs_deps {
     wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop
     cp -a hadoop /usr/local/
   fi
-  cmake_install $libhdfs3_dir
+  cd $libhdfs3_dir
+  cmake_install
 }
 
 cd "${DEPENDENCY_DIR}" || exit

diff --git a/scripts/setup-centos9.sh b/scripts/setup-centos9.sh
@@ -36,6 +36,7 @@ export CFLAGS=${CXXFLAGS//"-std=c++17"/} # Used by LZO.
 CMAKE_BUILD_TYPE="${BUILD_TYPE:-Release}"
 BUILD_DUCKDB="${BUILD_DUCKDB:-true}"
 USE_CLANG="${USE_CLANG:-false}"
+DEPENDENCY_DIR=${DEPENDENCY_DIR:-$(pwd)/deps-download}
 
 FB_OS_VERSION="v2024.05.20.00"
 FMT_VERSION="10.1.1"
@@ -85,18 +86,18 @@ function install_gflags {
   # Remove an older version if present.
   dnf remove -y gflags
   wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags
-  cmake_install gflags -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DBUILD_gflags_LIB=ON -DLIB_SUFFIX=64
+  cmake_install_dir gflags -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DBUILD_gflags_LIB=ON -DLIB_SUFFIX=64
 }
 
 function install_glog {
   wget_and_untar https://github.com/google/glog/archive/v0.6.0.tar.gz glog
-  cmake_install glog -DBUILD_SHARED_LIBS=ON
+  cmake_install_dir glog -DBUILD_SHARED_LIBS=ON
 }
 
 function install_lzo {
   wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo
   (
-    cd lzo
+    cd ${DEPENDENCY_DIR}/lzo
     ./configure --prefix=/usr --enable-shared --disable-static --docdir=/usr/share/doc/lzo-2.10
     make "-j$(nproc)"
     make install
@@ -106,7 +107,7 @@ function install_lzo {
 function install_boost {
   wget_and_untar https://github.com/boostorg/boost/releases/download/${BOOST_VERSION}/${BOOST_VERSION}.tar.gz boost
   (
-    cd boost
+    cd ${DEPENDENCY_DIR}/boost
     if [[ ${USE_CLANG} != "false" ]]; then
       ./bootstrap.sh --prefix=/usr/local --with-toolset="clang-15"
       # Switch the compiler from the clang-15 toolset which doesn't exist (clang-15.jam) to
@@ -123,18 +124,18 @@ function install_boost {
 
 function install_snappy {
   wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy
-  cmake_install snappy -DSNAPPY_BUILD_TESTS=OFF
+  cmake_install_dir snappy -DSNAPPY_BUILD_TESTS=OFF
 }
 
 function install_fmt {
   wget_and_untar https://github.com/fmtlib/fmt/archive/${FMT_VERSION}.tar.gz fmt
-  cmake_install fmt -DFMT_TEST=OFF
+  cmake_install_dir fmt -DFMT_TEST=OFF
 }
 
 function install_protobuf {
   wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.8/protobuf-all-21.8.tar.gz protobuf
   (
-    cd protobuf
+    cd ${DEPENDENCY_DIR}/protobuf
     ./configure --prefix=/usr
     make "-j${NPROC}"
     make install
@@ -144,60 +145,59 @@ function install_protobuf {
 
 function install_fizz {
   wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz
-  cmake_install fizz/fizz -DBUILD_TESTS=OFF
+  cmake_install_dir fizz/fizz -DBUILD_TESTS=OFF
 }
 
 function install_folly {
   wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly
-  cmake_install folly -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
+  cmake_install_dir folly -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
 }
 
 function install_wangle {
   wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle
-  cmake_install wangle/wangle -DBUILD_TESTS=OFF
+  cmake_install_dir wangle/wangle -DBUILD_TESTS=OFF
 }
 
 function install_fbthrift {
   wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift
-  cmake_install fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF
 }
 
 function install_mvfst {
   wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst
-  cmake_install mvfst -DBUILD_TESTS=OFF
+  cmake_install_dir mvfst -DBUILD_TESTS=OFF
 }
 
 function install_duckdb {
   if $BUILD_DUCKDB ; then
     echo 'Building DuckDB'
     wget_and_untar https://github.com/duckdb/duckdb/archive/refs/tags/v0.8.1.tar.gz duckdb
-    cmake_install duckdb -DBUILD_UNITTESTS=OFF -DENABLE_SANITIZER=OFF -DENABLE_UBSAN=OFF -DBUILD_SHELL=OFF -DEXPORT_DLL_SYMBOLS=OFF -DCMAKE_BUILD_TYPE=Release
+    cmake_install_dir duckdb -DBUILD_UNITTESTS=OFF -DENABLE_SANITIZER=OFF -DENABLE_UBSAN=OFF -DBUILD_SHELL=OFF -DEXPORT_DLL_SYMBOLS=OFF -DCMAKE_BUILD_TYPE=Release
   fi
 }
 
 function install_arrow {
   wget_and_untar https://archive.apache.org/dist/arrow/arrow-${ARROW_VERSION}/apache-arrow-${ARROW_VERSION}.tar.gz arrow
-  (
-    cd arrow/cpp
-    cmake_install \
-      -DARROW_PARQUET=OFF \
-      -DARROW_WITH_THRIFT=ON \
-      -DARROW_WITH_LZ4=ON \
-      -DARROW_WITH_SNAPPY=ON \
-      -DARROW_WITH_ZLIB=ON \
-      -DARROW_WITH_ZSTD=ON \
-      -DARROW_JEMALLOC=OFF \
-      -DARROW_SIMD_LEVEL=NONE \
-      -DARROW_RUNTIME_SIMD_LEVEL=NONE \
-      -DARROW_WITH_UTF8PROC=OFF \
-      -DARROW_TESTING=ON \
-      -DCMAKE_INSTALL_PREFIX=/usr/local \
-      -DCMAKE_BUILD_TYPE=Release \
-      -DARROW_BUILD_STATIC=ON \
-      -DThrift_SOURCE=BUNDLED
+  cmake_install_dir arrow/cpp \
+    -DARROW_PARQUET=OFF \
+    -DARROW_WITH_THRIFT=ON \
+    -DARROW_WITH_LZ4=ON \
+    -DARROW_WITH_SNAPPY=ON \
+    -DARROW_WITH_ZLIB=ON \
+    -DARROW_WITH_ZSTD=ON \
+    -DARROW_JEMALLOC=OFF \
+    -DARROW_SIMD_LEVEL=NONE \
+    -DARROW_RUNTIME_SIMD_LEVEL=NONE \
+    -DARROW_WITH_UTF8PROC=OFF \
+    -DARROW_TESTING=ON \
+    -DCMAKE_INSTALL_PREFIX=/usr/local \
+    -DCMAKE_BUILD_TYPE=Release \
+    -DARROW_BUILD_STATIC=ON \
+    -DThrift_SOURCE=BUNDLED
 
+  (
     # Install thrift.
-    cd _build/thrift_ep-prefix/src/thrift_ep-build
+    cd ${DEPENDENCY_DIR}/arrow/cpp/_build/thrift_ep-prefix/src/thrift_ep-build
     cmake --install ./ --prefix /usr/local/
   )
 }

diff --git a/scripts/setup-helper-functions.sh b/scripts/setup-helper-functions.sh
@@ -162,11 +162,13 @@ function wget_and_untar {
   mkdir -p "${DEPENDENCY_DIR}"
   pushd "${DEPENDENCY_DIR}"
   SUDO="${SUDO:-""}"
-  if [ -d "${DIR}" ] && prompt "${DIR} already exists. Delete?"; then
-    ${SUDO} rm -rf "${DIR}"
-  else
-    popd
-    return
+  if [ -d "${DIR}" ]; then
+    if prompt "${DIR} already exists. Delete?"; then
+      ${SUDO} rm -rf "${DIR}"
+    else
+      popd
+      return
+    fi
   fi
   mkdir -p "${DIR}"
   pushd "${DIR}"
@@ -176,26 +178,26 @@ function wget_and_untar {
   popd
 }
 
-function cmake_install {
-  mkdir -p "${DEPENDENCY_DIR}"
-  pushd "${DEPENDENCY_DIR}"
-  if [ -d "$1" ]; then
-    DIR="$1"
-    shift
-  else
-    DIR=$(pwd)
-  fi
-  pushd "${DIR}"
+function cmake_install_dir {
+  pushd "${DEPENDENCY_DIR}/$1"
   local NAME=$(basename "$(pwd)")
   local BINARY_DIR=_build
   SUDO="${SUDO:-""}"
-  if [ -d "${BINARY_DIR}" ] && prompt "Do you want to rebuild ${NAME}?"; then
-    ${SUDO} rm -rf "${BINARY_DIR}"
-  else
-    popd
-    popd
-    return
+  if [ -d "${BINARY_DIR}" ]; then
+    if prompt "Do you want to rebuild ${NAME}?"; then
+      ${SUDO} rm -rf "${BINARY_DIR}"
+    else
+      popd
+      return
+    fi
   fi
+  # remove the directory argument
+  shift
+  cmake_install $@
+  popd
+}
+
+function cmake_install {
   mkdir -p "${BINARY_DIR}"
   COMPILER_FLAGS=$(get_cxx_flags)
   # Add platform specific CXX flags if any
@@ -214,7 +216,5 @@ function cmake_install {
   # Exit if the build fails.
   cmake --build "${BINARY_DIR}" || { echo 'build failed' ; exit 1; }
   ${SUDO} cmake --install "${BINARY_DIR}"
-  popd
-  popd
 }
 
diff --git a/scripts/setup-macos.sh b/scripts/setup-macos.sh
@@ -32,7 +32,8 @@ SCRIPTDIR=$(dirname "${BASH_SOURCE[0]}")
 export INSTALL_PREFIX=${INSTALL_PREFIX:-"$(pwd)/deps-install"}
 source $SCRIPTDIR/setup-helper-functions.sh
 PYTHON_VENV=${PYHTON_VENV:-"${SCRIPTDIR}/../.venv"}
-export OS_CXXFLAGS=" -isystem $(brew --prefix)/include"
+# Allow installed package headers to be picked up before brew package headers
+export OS_CXXFLAGS=" -I /usr/local/include -isystem $(brew --prefix)/include "
 NPROC=$(getconf _NPROCESSORS_ONLN)
 
 MACOS_VELOX_DEPS="bison flex gflags glog googletest icu4c libevent libsodium lz4 lzo openssl protobuf@21 snappy xz zstd"
@@ -88,47 +89,47 @@ function install_velox_deps_from_brew {
 
 function install_fmt {
   wget_and_untar https://github.com/fmtlib/fmt/archive/${FMT_VERSION}.tar.gz fmt
-  cmake_install fmt -DFMT_TEST=OFF
+  cmake_install_dir fmt -DFMT_TEST=OFF
 }
 
 function install_folly {
   wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly
-  cmake_install folly -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
+  cmake_install_dir folly -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
 }
 
 function install_fizz {
   wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz
-  cmake_install fizz/fizz -DBUILD_TESTS=OFF
+  cmake_install_dir fizz/fizz -DBUILD_TESTS=OFF
 }
 
 function install_wangle {
   wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle
-  cmake_install wangle/wangle -DBUILD_TESTS=OFF
+  cmake_install_dir wangle/wangle -DBUILD_TESTS=OFF
 }
 
 function install_mvfst {
   wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst
-  cmake_install mvfst -DBUILD_TESTS=OFF
+  cmake_install_dir mvfst -DBUILD_TESTS=OFF
 }
 
 function install_fbthrift {
   wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift
-  cmake_install fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF
+  cmake_install_dir fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF
 }
 
 function install_double_conversion {
   wget_and_untar https://github.com/google/double-conversion/archive/refs/tags/v3.1.5.tar.gz double-conversion
-  cmake_install double-conversion -DBUILD_TESTING=OFF
+  cmake_install_dir double-conversion -DBUILD_TESTING=OFF
 }
 
 function install_ranges_v3 {
   wget_and_untar https://github.com/ericniebler/range-v3/archive/refs/tags/0.12.0.tar.gz ranges_v3
-  cmake_install ranges_v3 -DRANGES_ENABLE_WERROR=OFF -DRANGE_V3_TESTS=OFF -DRANGE_V3_EXAMPLES=OFF
+  cmake_install_dir ranges_v3 -DRANGES_ENABLE_WERROR=OFF -DRANGE_V3_TESTS=OFF -DRANGE_V3_EXAMPLES=OFF
 }
 
 function install_re2 {
   wget_and_untar https://github.com/google/re2/archive/refs/tags/2022-02-01.tar.gz re2
-  cmake_install re2 -DRE2_BUILD_TESTING=OFF
+  cmake_install_dir re2 -DRE2_BUILD_TESTING=OFF
 }
 
 function install_velox_deps {