You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/05 21:25:27 UTC
[GitHub] [arrow] kou commented on a diff in pull request #14310: ARROW-17872: [C++][CI] Reduce MacOS CI Dependencies
kou commented on code in PR #14310:
URL: https://github.com/apache/arrow/pull/14310#discussion_r988359328
##########
cpp/Brewfile:
##########
@@ -28,8 +28,6 @@ brew "git"
brew "glog"
brew "googletest"
brew "grpc"
-brew "llvm"
-brew "llvm@12"
Review Comment:
Can we add `llvm@14` instead?
```ruby
brew "llvm@14"
```
I assume that adding `llvm@14` doesn't cause `llvm@14` (re)install on GitHub Actions because `llvm@14` is already installed.
##########
.github/workflows/cpp.yml:
##########
@@ -182,10 +182,9 @@ 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.
+ # pin LLVM version on MacOS to 14 ARROW-17902
run: |
- export CC=$(brew --prefix llvm)/bin/clang
- export CXX=$(brew --prefix llvm)/bin/clang++
+ export LLVM_ROOT=$(brew --prefix llvm@14)
Review Comment:
Can we remove this with the following patch?
```diff
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index 01d461d14a..5116c8a232 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/cmake_modules/FindLLVMAlt.cmake b/cpp/cmake_modules/FindLLVMAlt.cmake
index 56ceead941..d371c8d3cc 100644
--- a/cpp/cmake_modules/FindLLVMAlt.cmake
+++ b/cpp/cmake_modules/FindLLVMAlt.cmake
@@ -40,26 +40,8 @@ if(DEFINED LLVM_ROOT)
endif()
if(NOT LLVM_FOUND)
- 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}"
- 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()
- endif()
- endif()
-
- foreach(HINT ${LLVM_HINTS})
- foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS})
+ foreach(ARROW_LLVM_VERSION ${ARROW_LLVM_VERSIONS})
+ foreach(HINT ${LLVM_ROOT} ${LLVM_DIR} /usr/lib /usr/share)
find_package(LLVM
${ARROW_LLVM_VERSION}
CONFIG
@@ -69,6 +51,29 @@ if(NOT LLVM_FOUND)
break()
endif()
endforeach()
+ if(APPLE)
+ find_program(BREW brew)
+ if(BREW)
+ execute_process(COMMAND ${BREW} --prefix "llvm@${ARROW_LLVM_VERSION}"
+ 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)
+ find_package(LLVM
+ ${ARROW_LLVM_VERSION}
+ CONFIG
+ HINTS
+ ${LLVM_BREW_PREFIX})
+ if(LLVM_FOUND)
+ break()
+ endif()
+ endif()
+ endif()
+ endif()
endforeach()
endif()
diff --git a/cpp/src/gandiva/GandivaConfig.cmake.in b/cpp/src/gandiva/GandivaConfig.cmake.in
index 20cdc75acb..18d194f1e4 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)
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org