Uh oh!
There was an error while loading. Please reload this page.
ARROW-17872: [C++][CI] Reduce macOS CI dependencies - #14310
Conversation
2753e8a to
40f9653Comparejs8544
commented
Oct 5, 2022
@github-actions crossbow submit -g nightly-release |
This comment was marked as outdated.
This comment was marked as outdated.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Can we remove this with the following patch?
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)There was a problem hiding this comment.
The current logic is to check brew paths if the default ones fail. I think a simpler approach is to append brew paths to LLVM_HINTS. Could you have a look at the latest changes?
There was a problem hiding this comment.
Could you keep the "brew --prefix llvm is used when brew --prefix llvm@${LLVM_VERSION} is failed" logic to support the llvm formula (that is for the latest LLVM) in Homebrew?
There was a problem hiding this comment.
Could you keep the "
brew --prefix llvmis used whenbrew --prefix llvm@${LLVM_VERSION}is failed" logic to support thellvmformula (that is for the latest LLVM) in Homebrew?
It won't be necessary because llvm@15 is an alias for llvm (https://formulae.brew.sh/formula/llvm) . For example on my machine brew --prefix llvm@15 gives /usr/local/opt/llvm.
There was a problem hiding this comment.
Wow! I didn't know the Homebrew feature!
js8544
commented
Oct 7, 2022
@github-actions crossbow submit -g nightly-release |
082fe8f to
d7a3c35Compare
This comment was marked as outdated.
This comment was marked as outdated.
d7a3c35 to
7b3e768Comparejs8544
commented
Oct 7, 2022
@github-actions crossbow submit -g nightly-release |
This comment was marked as outdated.
This comment was marked as outdated.
7b3e768 to
167f165Comparejs8544
commented
Oct 7, 2022
@github-actions crossbow submit -g nightly-release |
Revision: 167f165b50903d06bfc1098d31a00577bcc97e00 Submitted crossbow builds: ursacomputing/crossbow @ actions-e7309d9ae6 |
kou
commented
Oct 7, 2022
Could you rebase on master for the verify-rc-source-python-linux-ubuntu-18.04-amd64 failure? https://github.com/ursacomputing/crossbow/actions/runs/3205223717/jobs/5237466278#step:5:10529 |
167f165 to
feec756Comparejs8544
commented
Oct 7, 2022
@github-actions crossbow submit -g nightly-release |
1 similar comment
js8544
commented
Oct 7, 2022
@github-actions crossbow submit -g nightly-release |
Revision: feec756 Submitted crossbow builds: ursacomputing/crossbow @ actions-2b41928aa0 |
ursabot
commented
Oct 9, 2022
Benchmark runs are scheduled for baseline = 12667cd and contender = 48c6738. 48c6738 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Pin LLVM and Clang-format version to 14 because it's pre-installed on the runner image.