From 800402b5bc2e19eb0fb9382d7034e97aad45bfae Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 12:12:57 +0000 Subject: [PATCH 01/10] reduce macos deps --- .env | 4 ++-- cpp/Brewfile | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.env b/.env index 761642506f1..b18eb2c3f0e 100644 --- a/.env +++ b/.env @@ -53,7 +53,7 @@ FEDORA=35 UBUNTU=20.04 # Default versions for various dependencies -CLANG_TOOLS=12 +CLANG_TOOLS=14 CUDA=11.0.3 DASK=latest DOTNET=6.0 @@ -64,7 +64,7 @@ HDFS=3.2.1 JDK=8 KARTOTHEK=latest # LLVM 12 and GCC 11 reports -Wmismatched-new-delete. -LLVM=13 +LLVM=14 MAVEN=3.5.4 NODE=16 NUMBA=latest diff --git a/cpp/Brewfile b/cpp/Brewfile index 61fb619dc66..2a2097cf877 100644 --- a/cpp/Brewfile +++ b/cpp/Brewfile @@ -28,8 +28,6 @@ brew "git" brew "glog" brew "googletest" brew "grpc" -brew "llvm" -brew "llvm@12" brew "lz4" brew "ninja" brew "numpy" From e6e90e94354d465de2e48eda545dee0781b7d7b9 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 12:49:18 +0000 Subject: [PATCH 02/10] remove using brew llvm --- .github/workflows/cpp.yml | 3 -- .github/workflows/python.yml | 3 -- .github/workflows/ruby.yml | 3 -- dev/tasks/verify-rc/github.macos.amd64.yml | 4 --- java/gandiva/src/main/cpp/jni_common.cc | 40 ++++++++++++---------- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 602561cba13..b420ec06395 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -182,10 +182,7 @@ jobs: key: cpp-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-macos- - name: Build - # use brew version of clang, to be consistent with LLVM lib, see ARROW-17790. run: | - export CC=$(brew --prefix llvm)/bin/clang - export CXX=$(brew --prefix llvm)/bin/clang++ ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 1c3de785188..884c2fc7367 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -167,10 +167,7 @@ jobs: -r python/requirements-test.txt - name: Build shell: bash - # use brew version of clang, to be consistent with LLVM lib, see ARROW-17790. run: | - export CC=$(brew --prefix llvm)/bin/clang - export CXX=$(brew --prefix llvm)/bin/clang++ export PYTHON=python3 ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ci/scripts/python_build.sh $(pwd) $(pwd)/build diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 57b5e275885..22045700346 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -169,10 +169,7 @@ jobs: key: ruby-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: ruby-ccache-macos- - name: Build C++ - # use brew version of clang, to be consistent with LLVM lib, see ARROW-17790. run: | - export CC=$(brew --prefix llvm)/bin/clang - export CXX=$(brew --prefix llvm)/bin/clang++ ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Build GLib run: | diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index 98043636ed9..cdd61702899 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -71,8 +71,4 @@ jobs: USE_CONDA: 1 {% endif %} run: | - {% if not use_conda %} - export CC=$(brew --prefix llvm)/bin/clang - export CXX=$(brew --prefix llvm)/bin/clang++ - {% endif %} arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index ba0af1106b1..bf9ed7de72a 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -712,9 +713,7 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { Status Resize(const int64_t new_size, bool shrink_to_fit) override; - Status Reserve(const int64_t new_capacity) override { - return Status::NotImplemented("reserve not implemented"); - } + Status Reserve(const int64_t new_capacity) override; private: JNIEnv* env_; @@ -722,20 +721,10 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { int32_t vector_idx_; }; -Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { - if (shrink_to_fit == true) { - return Status::NotImplemented("shrink not implemented"); - } - - if (ARROW_PREDICT_TRUE(new_size < capacity())) { - // no need to expand. - size_ = new_size; - return Status::OK(); - } - +Status JavaResizableBuffer::Reserve(const int64_t new_capacity) { // callback into java to expand the buffer - jobject ret = - env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, new_size); + jobject ret = env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, + new_capacity); if (env_->ExceptionCheck()) { env_->ExceptionDescribe(); env_->ExceptionClear(); @@ -744,14 +733,29 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); - DCHECK_GE(ret_capacity, new_size); data_ = reinterpret_cast(ret_address); - size_ = new_size; capacity_ = ret_capacity; return Status::OK(); } +Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { + if (shrink_to_fit == true) { + return Status::NotImplemented("shrink not implemented"); + } + + if (ARROW_PREDICT_TRUE(new_size < capacity())) { + // no need to expand. + size_ = new_size; + return Status::OK(); + } + + RETURN_NOT_OK(Reserve(new_size)); + DCHECK_GE(capacity_, new_size); + size_ = new_size; + return Status::OK(); +} + #define CHECK_OUT_BUFFER_IDX_AND_BREAK(idx, len) \ if (idx >= len) { \ status = gandiva::Status::Invalid("insufficient number of out_buf_addrs"); \ From 3c324b17366f8a9ed4474b7b308e544891ad8caf Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 13:17:14 +0000 Subject: [PATCH 03/10] pin macos llvm to 14 --- .github/workflows/cpp.yml | 2 ++ .github/workflows/python.yml | 2 ++ .github/workflows/ruby.yml | 2 ++ dev/tasks/java-jars/github.yml | 2 ++ dev/tasks/verify-rc/github.macos.amd64.yml | 2 ++ 5 files changed, 10 insertions(+) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index b420ec06395..511b307c43c 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -182,7 +182,9 @@ jobs: key: cpp-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-macos- - name: Build + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 884c2fc7367..05e08781ee0 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -167,7 +167,9 @@ jobs: -r python/requirements-test.txt - name: Build shell: bash + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) export PYTHON=python3 ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ci/scripts/python_build.sh $(pwd) $(pwd)/build diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 22045700346..63141c1b28d 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -169,7 +169,9 @@ jobs: key: ruby-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: ruby-ccache-macos- - name: Build C++ + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Build GLib run: | diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml index 6f7fdc82d5f..6d2ef9d9465 100644 --- a/dev/tasks/java-jars/github.yml +++ b/dev/tasks/java-jars/github.yml @@ -69,7 +69,9 @@ jobs: run: | arrow/ci/scripts/ccache_setup.sh - name: Build C++ libraries + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) set -e arrow/ci/scripts/java_jni_macos_build.sh \ $GITHUB_WORKSPACE/arrow \ diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index cdd61702899..550a0afa440 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -70,5 +70,7 @@ jobs: {% if use_conda %} USE_CONDA: 1 {% endif %} + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} From e8b607114a345124399c130cdabdc348e6678b7f Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 13:34:59 +0000 Subject: [PATCH 04/10] fix clang-format issues --- cpp/src/arrow/util/bpacking.cc | 9 +++++---- cpp/src/parquet/level_comparison.cc | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/util/bpacking.cc b/cpp/src/arrow/util/bpacking.cc index c1b0d706a5d..b33eb92606b 100644 --- a/cpp/src/arrow/util/bpacking.cc +++ b/cpp/src/arrow/util/bpacking.cc @@ -153,13 +153,14 @@ struct Unpack32DynamicFunction { using FunctionType = decltype(&unpack32_default); static std::vector> implementations() { - return { - { DispatchLevel::NONE, unpack32_default } + return {{DispatchLevel::NONE, unpack32_default} #if defined(ARROW_HAVE_RUNTIME_AVX2) - , { DispatchLevel::AVX2, unpack32_avx2 } + , + {DispatchLevel::AVX2, unpack32_avx2} #endif #if defined(ARROW_HAVE_RUNTIME_AVX512) - , { DispatchLevel::AVX512, unpack32_avx512 } + , + {DispatchLevel::AVX512, unpack32_avx512} #endif }; } diff --git a/cpp/src/parquet/level_comparison.cc b/cpp/src/parquet/level_comparison.cc index 30614ae61fb..c9ad6b76c72 100644 --- a/cpp/src/parquet/level_comparison.cc +++ b/cpp/src/parquet/level_comparison.cc @@ -44,10 +44,10 @@ struct GreaterThanDynamicFunction { using FunctionType = decltype(&GreaterThanBitmap); static std::vector> implementations() { - return { - { DispatchLevel::NONE, standard::GreaterThanBitmapImpl } + return {{DispatchLevel::NONE, standard::GreaterThanBitmapImpl} #if defined(ARROW_HAVE_RUNTIME_AVX2) - , { DispatchLevel::AVX2, GreaterThanBitmapAvx2 } + , + {DispatchLevel::AVX2, GreaterThanBitmapAvx2} #endif }; } @@ -57,10 +57,10 @@ struct MinMaxDynamicFunction { using FunctionType = decltype(&FindMinMax); static std::vector> implementations() { - return { - { DispatchLevel::NONE, standard::FindMinMaxImpl } + return {{DispatchLevel::NONE, standard::FindMinMaxImpl} #if defined(ARROW_HAVE_RUNTIME_AVX2) - , { DispatchLevel::AVX2, FindMinMaxAvx2 } + , + {DispatchLevel::AVX2, FindMinMaxAvx2} #endif }; } From c2005d43fea46f98a47342c6e96ed69f5b42783f Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 13:38:02 +0000 Subject: [PATCH 05/10] revert java gandiva change --- java/gandiva/src/main/cpp/jni_common.cc | 40 +++++++++++-------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index bf9ed7de72a..ba0af1106b1 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -#include #include #include @@ -713,7 +712,9 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { Status Resize(const int64_t new_size, bool shrink_to_fit) override; - Status Reserve(const int64_t new_capacity) override; + Status Reserve(const int64_t new_capacity) override { + return Status::NotImplemented("reserve not implemented"); + } private: JNIEnv* env_; @@ -721,24 +722,6 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { int32_t vector_idx_; }; -Status JavaResizableBuffer::Reserve(const int64_t new_capacity) { - // callback into java to expand the buffer - jobject ret = env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, - new_capacity); - if (env_->ExceptionCheck()) { - env_->ExceptionDescribe(); - env_->ExceptionClear(); - return Status::OutOfMemory("buffer expand failed in java"); - } - - jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); - jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); - - data_ = reinterpret_cast(ret_address); - capacity_ = ret_capacity; - return Status::OK(); -} - Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { if (shrink_to_fit == true) { return Status::NotImplemented("shrink not implemented"); @@ -750,9 +733,22 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { return Status::OK(); } - RETURN_NOT_OK(Reserve(new_size)); - DCHECK_GE(capacity_, new_size); + // callback into java to expand the buffer + jobject ret = + env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, new_size); + if (env_->ExceptionCheck()) { + env_->ExceptionDescribe(); + env_->ExceptionClear(); + return Status::OutOfMemory("buffer expand failed in java"); + } + + jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); + jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); + DCHECK_GE(ret_capacity, new_size); + + data_ = reinterpret_cast(ret_address); size_ = new_size; + capacity_ = ret_capacity; return Status::OK(); } From a36204a67bf7695b936305a3421e2117d63b1d2b Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 14:08:47 +0000 Subject: [PATCH 06/10] fix mac rc conda if --- dev/tasks/verify-rc/github.macos.amd64.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index 550a0afa440..ae1d10d942f 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -72,5 +72,7 @@ jobs: {% endif %} # pin LLVM version on MacOS to 14 ARROW-17902 run: | + {% if not use_conda %} export LLVM_ROOT=$(brew --prefix llvm@14) + {% endif %} arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} From 149336e7c77bb7adacdbf9bc4d4b75d45315c9b4 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 14:55:52 +0000 Subject: [PATCH 07/10] add python cmake policy 0074 --- python/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 1013af4fe85..279ac076ac8 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -43,6 +43,12 @@ include(CMakeParseArguments) # https://www.cmake.org/cmake/help/latest/policy/CMP0054.html cmake_policy(SET CMP0054 NEW) +# find_package() uses _ROOT variables. +# https://cmake.org/cmake/help/latest/policy/CMP0074.html +if(POLICY CMP0074) + cmake_policy(SET CMP0074 NEW) +endif() + # Use the first Python installation on PATH, not the newest one set(Python3_FIND_STRATEGY "LOCATION") # On Windows, use registry last, not first From 193ad51d702d783c2c57a5e4f062b602649ab854 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Thu, 6 Oct 2022 06:49:48 +0000 Subject: [PATCH 08/10] pr feedback --- cpp/Brewfile | 1 + cpp/cmake_modules/FindLLVMAlt.cmake | 56 +++++++++-------------------- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/cpp/Brewfile b/cpp/Brewfile index 2a2097cf877..35941a92895 100644 --- a/cpp/Brewfile +++ b/cpp/Brewfile @@ -28,6 +28,7 @@ brew "git" brew "glog" brew "googletest" brew "grpc" +brew "llvm@14" brew "lz4" brew "ninja" brew "numpy" diff --git a/cpp/cmake_modules/FindLLVMAlt.cmake b/cpp/cmake_modules/FindLLVMAlt.cmake index 56ceead9415..4e64f6a845c 100644 --- a/cpp/cmake_modules/FindLLVMAlt.cmake +++ b/cpp/cmake_modules/FindLLVMAlt.cmake @@ -23,54 +23,32 @@ if(LLVMAlt_FOUND) return() endif() -if(DEFINED LLVM_ROOT) - # if llvm source is set to conda then prefer conda llvm over system llvm even - # if the system one is newer - foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) - find_package(LLVM - ${ARROW_LLVM_VERSION} - CONFIG - NO_DEFAULT_PATH - HINTS - ${LLVM_ROOT}) - if(LLVM_FOUND) - break() - endif() - endforeach() -endif() - -if(NOT LLVM_FOUND) +foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) set(LLVM_HINTS ${LLVM_ROOT} ${LLVM_DIR} /usr/lib /usr/share) + if(APPLE) find_program(BREW brew) if(BREW) - execute_process(COMMAND ${BREW} --prefix "llvm@${ARROW_LLVM_VERSION_PRIMARY_MAJOR}" + string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_MAJOR + "${ARROW_LLVM_VERSION}") + execute_process(COMMAND ${BREW} --prefix "llvm@${ARROW_LLVM_VERSION_MAJOR}" OUTPUT_VARIABLE LLVM_BREW_PREFIX OUTPUT_STRIP_TRAILING_WHITESPACE) - if(NOT LLVM_BREW_PREFIX) - execute_process(COMMAND ${BREW} --prefix llvm - OUTPUT_VARIABLE LLVM_BREW_PREFIX - OUTPUT_STRIP_TRAILING_WHITESPACE) - endif() - if(LLVM_BREW_PREFIX) - list(APPEND LLVM_HINTS ${LLVM_BREW_PREFIX}) - endif() + list(APPEND LLVM_HINTS ${LLVM_BREW_PREFIX}) endif() endif() - foreach(HINT ${LLVM_HINTS}) - foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) - find_package(LLVM - ${ARROW_LLVM_VERSION} - CONFIG - HINTS - ${HINT}) - if(LLVM_FOUND) - break() - endif() - endforeach() - endforeach() -endif() + find_package(LLVM + ${ARROW_LLVM_VERSION} + CONFIG + NO_DEFAULT_PATH + HINTS + ${LLVM_HINTS}) + + if(LLVM_FOUND) + break() + endif() +endforeach() if(LLVM_FOUND) # Find the libraries that correspond to the LLVM components From 961273511b9aa696849ce5ddaa8543652f4f33e8 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Thu, 6 Oct 2022 06:51:56 +0000 Subject: [PATCH 09/10] pr feedback --- cpp/CMakeLists.txt | 3 --- cpp/src/gandiva/GandivaConfig.cmake.in | 1 - 2 files changed, 4 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 01d461d14a2..5116c8a232d 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -138,9 +138,6 @@ set(ARROW_LLVM_VERSIONS "9" "8" "7") -list(GET ARROW_LLVM_VERSIONS 0 ARROW_LLVM_VERSION_PRIMARY) -string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_PRIMARY_MAJOR - "${ARROW_LLVM_VERSION_PRIMARY}") file(READ ${CMAKE_CURRENT_SOURCE_DIR}/../.env ARROW_ENV) string(REGEX MATCH "CLANG_TOOLS=[^\n]+" ARROW_ENV_CLANG_TOOLS_VERSION "${ARROW_ENV}") diff --git a/cpp/src/gandiva/GandivaConfig.cmake.in b/cpp/src/gandiva/GandivaConfig.cmake.in index 20cdc75acb6..18d194f1e4d 100644 --- a/cpp/src/gandiva/GandivaConfig.cmake.in +++ b/cpp/src/gandiva/GandivaConfig.cmake.in @@ -27,7 +27,6 @@ @PACKAGE_INIT@ set(ARROW_LLVM_VERSIONS "@ARROW_LLVM_VERSIONS@") -set(ARROW_LLVM_VERSION_PRIMARY_MAJOR "@ARROW_LLVM_VERSION_PRIMARY_MAJOR@") include(CMakeFindDependencyMacro) find_dependency(Arrow) From feec75635f935c7b004ea8bf993da29a86a857d6 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Thu, 6 Oct 2022 06:52:45 +0000 Subject: [PATCH 10/10] pr feedback --- .github/workflows/cpp.yml | 2 - .github/workflows/python.yml | 2 - .github/workflows/ruby.yml | 2 - cpp/cmake_modules/FindLLVMAlt.cmake | 61 ++++++++++++++-------- dev/tasks/java-jars/github.yml | 2 - dev/tasks/verify-rc/github.macos.amd64.yml | 4 -- 6 files changed, 39 insertions(+), 34 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 511b307c43c..b420ec06395 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -182,9 +182,7 @@ jobs: key: cpp-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-macos- - name: Build - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 05e08781ee0..884c2fc7367 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -167,9 +167,7 @@ jobs: -r python/requirements-test.txt - name: Build shell: bash - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) export PYTHON=python3 ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ci/scripts/python_build.sh $(pwd) $(pwd)/build diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 63141c1b28d..22045700346 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -169,9 +169,7 @@ jobs: key: ruby-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: ruby-ccache-macos- - name: Build C++ - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Build GLib run: | diff --git a/cpp/cmake_modules/FindLLVMAlt.cmake b/cpp/cmake_modules/FindLLVMAlt.cmake index 4e64f6a845c..c44c4802284 100644 --- a/cpp/cmake_modules/FindLLVMAlt.cmake +++ b/cpp/cmake_modules/FindLLVMAlt.cmake @@ -23,32 +23,49 @@ if(LLVMAlt_FOUND) return() endif() -foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) - set(LLVM_HINTS ${LLVM_ROOT} ${LLVM_DIR} /usr/lib /usr/share) +if(DEFINED LLVM_ROOT) + # if llvm source is set to conda then prefer conda llvm over system llvm even + # if the system one is newer + foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) + find_package(LLVM + ${ARROW_LLVM_VERSION} + CONFIG + NO_DEFAULT_PATH + HINTS + ${LLVM_ROOT}) + if(LLVM_FOUND) + break() + endif() + endforeach() +endif() + +if(NOT LLVM_FOUND) + foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS}) + set(LLVM_HINTS ${LLVM_ROOT} ${LLVM_DIR} /usr/lib /usr/share) - if(APPLE) - find_program(BREW brew) - if(BREW) - string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_MAJOR - "${ARROW_LLVM_VERSION}") - execute_process(COMMAND ${BREW} --prefix "llvm@${ARROW_LLVM_VERSION_MAJOR}" - OUTPUT_VARIABLE LLVM_BREW_PREFIX - OUTPUT_STRIP_TRAILING_WHITESPACE) - list(APPEND LLVM_HINTS ${LLVM_BREW_PREFIX}) + if(APPLE) + find_program(BREW brew) + if(BREW) + string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_MAJOR + "${ARROW_LLVM_VERSION}") + execute_process(COMMAND ${BREW} --prefix "llvm@${ARROW_LLVM_VERSION_MAJOR}" + OUTPUT_VARIABLE LLVM_BREW_PREFIX + OUTPUT_STRIP_TRAILING_WHITESPACE) + list(APPEND LLVM_HINTS ${LLVM_BREW_PREFIX}) + endif() endif() - endif() - find_package(LLVM - ${ARROW_LLVM_VERSION} - CONFIG - NO_DEFAULT_PATH - HINTS - ${LLVM_HINTS}) + find_package(LLVM + ${ARROW_LLVM_VERSION} + CONFIG + HINTS + ${LLVM_HINTS}) - if(LLVM_FOUND) - break() - endif() -endforeach() + if(LLVM_FOUND) + break() + endif() + endforeach() +endif() if(LLVM_FOUND) # Find the libraries that correspond to the LLVM components diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml index 6d2ef9d9465..6f7fdc82d5f 100644 --- a/dev/tasks/java-jars/github.yml +++ b/dev/tasks/java-jars/github.yml @@ -69,9 +69,7 @@ jobs: run: | arrow/ci/scripts/ccache_setup.sh - name: Build C++ libraries - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) set -e arrow/ci/scripts/java_jni_macos_build.sh \ $GITHUB_WORKSPACE/arrow \ diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index ae1d10d942f..cdd61702899 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -70,9 +70,5 @@ jobs: {% if use_conda %} USE_CONDA: 1 {% endif %} - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - {% if not use_conda %} - export LLVM_ROOT=$(brew --prefix llvm@14) - {% endif %} arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }}