Skip to content

Commit 40cfbca

Browse files
wesmxhochy
wesm
authored and
xhochy
committed
ARROW-3972: [C++] Migrate to LLVM 7. Add option to disable using ld.gold
All in all this wasn't too painful, and Gandiva seems to run fine (I wouldn't have expected otherwise). I tested this locally on Ubuntu 18.10, and found that the LLVM libraries from apt fail to link with ld.gold (binutils 1.16), so I added an option to toggle ld.gold ON and OFF (it's now OFF by default; using `ld` by default should yield strictly fewer bugs / build failures). Not sure why that is I will need some help testing and debugging the Crossbow and packaging tasks to make sure this doesn't break anything else before we merge it Author: Wes McKinney <wesm+git@apache.org> Author: Pindikura Ravindra <ravindra@dremio.com> Author: Korn, Uwe <Uwe.Korn@blue-yonder.com> Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#3499 from wesm/llvm-7 and squashes the following commits: 5c48eed <Korn, Uwe> Switch to docker image on branch 93aadd7 <Uwe L. Korn> Update clang e155fad <Pindikura Ravindra> Use the llvm from apt-get (instead of travis) 987fc5f <Wes McKinney> Enable all LLVM libraries ea1f013 <Wes McKinney> Link libLLVMSupport.a later 034b17d <Wes McKinney> Add missing libLLVMSupport.a dependency 4448243 <Wes McKinney> Install clang before calling travis_install_linux.sh 59d170a <Wes McKinney> Fix more usages of LLVM version ce831e2 <Wes McKinney> Fix clang executable name in .travis.yml 69fd486 <Pindikura Ravindra> ARROW-3972: misc fixes for LLVM7 c858441 <Wes McKinney> Some CI fixes 87434da <Wes McKinney> Code comments d198eb8 <Wes McKinney> Decruft. Turn off ld.gold by default 2fc3a26 <Wes McKinney> Build project with LLVM 7. Add option to disable using ld.gold
1 parent 1cf4cdd commit 40cfbca

20 files changed

Lines changed: 87 additions & 69 deletions

‎.travis.yml‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ matrix:
5858
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
5959
script:
6060
- $TRAVIS_BUILD_DIR/ci/travis_lint.sh
61-
- name: "C++ unit tests w/ Valgrind, clang 6.0"
61+
- name: "C++ unit tests w/ Valgrind, clang 7.0"
6262
language: cpp
6363
os: linux
6464
env:
@@ -70,12 +70,12 @@ matrix:
7070
- ARROW_TRAVIS_GANDIVA=1
7171
- ARROW_TRAVIS_USE_SYSTEM_JAVA=1
7272
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
73+
- CC="clang-7"
74+
- CXX="clang++-7"
7375
before_script:
7476
- if [ $ARROW_CI_CPP_AFFECTED != "1" ]; then exit; fi
75-
- export CC="clang-6.0"
76-
- export CXX="clang++-6.0"
77-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
7877
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
78+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
7979
# If either C++ or Python changed, we must install the C++ libraries
8080
- git submodule update --init
8181
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
@@ -99,8 +99,8 @@ matrix:
9999
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
100100
before_script:
101101
- if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
102-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
103102
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
103+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
104104
# If either C++ or Python changed, we must install the C++ libraries
105105
- git submodule update --init
106106
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
@@ -133,8 +133,8 @@ matrix:
133133
- eval `python $TRAVIS_BUILD_DIR/ci/detect-changes.py`
134134
before_script:
135135
- if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
136-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
137136
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
137+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
138138
# If either C++ or Python changed, we must install the C++ libraries
139139
- git submodule update --init
140140
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
@@ -160,8 +160,8 @@ matrix:
160160
# - ARROW_TRAVIS_PYTHON_BENCHMARKS=1
161161
before_script:
162162
- if [ $ARROW_CI_PYTHON_AFFECTED != "1" ] && [ $ARROW_CI_DOCS_AFFECTED != "1" ]; then exit; fi
163-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
164163
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
164+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
165165
- $TRAVIS_BUILD_DIR/ci/travis_install_toolchain.sh
166166
script:
167167
- $TRAVIS_BUILD_DIR/ci/travis_script_java.sh || travis_terminate 1
@@ -225,7 +225,7 @@ matrix:
225225
- name: "[manylinux1] Python"
226226
language: cpp
227227
before_script:
228-
- if [ $ARROW_CI_PYTHON_AFFECTED == "1" ]; then docker pull quay.io/xhochy/arrow_manylinux1_x86_64_base:latest; fi
228+
- if [ $ARROW_CI_PYTHON_AFFECTED == "1" ]; then docker pull quay.io/xhochy/arrow_manylinux1_x86_64_base:llvm-7-manylinux1; fi
229229
script:
230230
- if [ $ARROW_CI_PYTHON_AFFECTED == "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_manylinux.sh; fi
231231
- name: "Java w/ OpenJDK 8"
@@ -254,20 +254,20 @@ matrix:
254254
- if [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
255255
script:
256256
- $TRAVIS_BUILD_DIR/ci/travis_script_java.sh
257-
- name: "Integration w/ OpenJDK 8"
257+
- name: "Integration w/ OpenJDK 8, clang 7"
258258
language: java
259259
os: linux
260260
env: ARROW_TEST_GROUP=integration
261261
jdk: openjdk8
262262
env:
263263
- ARROW_TRAVIS_PLASMA=1
264264
- ARROW_TRAVIS_PLASMA_JAVA_CLIENT=1
265+
- CC="clang-7"
266+
- CXX="clang++-7"
265267
before_script:
266268
- if [ $ARROW_CI_INTEGRATION_AFFECTED != "1" ]; then exit; fi
267-
- export CC="clang-6.0"
268-
- export CXX="clang++-6.0"
269-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
270269
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
270+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
271271
- nvm install 11.6
272272
- $TRAVIS_BUILD_DIR/ci/travis_before_script_js.sh
273273
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
@@ -297,8 +297,8 @@ matrix:
297297
- ARROW_TRAVIS_PLASMA=1
298298
before_script:
299299
- if [ $ARROW_CI_RUBY_AFFECTED != "1" ]; then exit; fi
300-
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
301300
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
301+
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
302302
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
303303
- $TRAVIS_BUILD_DIR/ci/travis_before_script_c_glib.sh
304304
- $TRAVIS_BUILD_DIR/ci/travis_before_script_ruby.sh

‎c_glib/Brewfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ brew "git"
2424
brew "gobject-introspection"
2525
brew "gtk-doc"
2626
brew "libtool"
27-
brew "llvm@6"
27+
brew "llvm@7"
2828
brew "lua"
2929
brew "luarocks"
3030
brew "ninja"

‎ci/travis_env_common.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ export MINICONDA=$HOME/miniconda
2323
export CONDA_PKGS_DIRS=$HOME/.conda_packages
2424
export CONDA_BINUTILS_VERSION=2.31
2525

26+
export ARROW_LLVM_VERSION=7.0
27+
export CONDA_LLVM_VERSION="7.0.*"
28+
29+
# extract the major version
30+
export ARROW_LLVM_MAJOR_VERSION=$(echo $ARROW_LLVM_VERSION | cut -d. -f1)
31+
2632
export ARROW_CPP_DIR=$TRAVIS_BUILD_DIR/cpp
2733
export ARROW_PYTHON_DIR=$TRAVIS_BUILD_DIR/python
2834
export ARROW_C_GLIB_DIR=$TRAVIS_BUILD_DIR/c_glib

‎ci/travis_install_clang_tools.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
2424

2525
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key|sudo apt-key add -
2626
sudo apt-add-repository -y \
27-
"deb https://apt.llvm.org/$DISTRO_CODENAME/ llvm-toolchain-$DISTRO_CODENAME-6.0 main"
27+
"deb https://apt.llvm.org/$DISTRO_CODENAME/ llvm-toolchain-$DISTRO_CODENAME-$ARROW_LLVM_MAJOR_VERSION main"
2828
sudo apt-get update -qq
29-
sudo apt-get install -q clang-6.0 clang-format-6.0 clang-tidy-6.0
29+
sudo apt-get install -q clang-$ARROW_LLVM_MAJOR_VERSION clang-format-$ARROW_LLVM_MAJOR_VERSION clang-tidy-$ARROW_LLVM_MAJOR_VERSION

‎ci/travis_install_linux.sh‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
3838
fi
3939

4040
set -x
41-
if [ "$DISTRO_CODENAME" != "trusty" ]; then
42-
if [ "$ARROW_TRAVIS_GANDIVA" == "1" ]; then
43-
sudo apt-get install -y -qq llvm-6.0-dev
44-
fi
41+
if [ "$ARROW_TRAVIS_GANDIVA" == "1" ]; then
42+
sudo apt-get install -y -qq llvm-$ARROW_LLVM_MAJOR_VERSION-dev
43+
fi
4544

45+
set -x
46+
if [ "$DISTRO_CODENAME" != "trusty" ]; then
4647
sudo apt-get install -y -qq maven
4748

4849
# Remove Travis-specific versions of Java
@@ -56,4 +57,3 @@ if [ "$DISTRO_CODENAME" != "trusty" ]; then
5657
java -version
5758
mvn -v
5859
fi
59-

‎ci/travis_install_toolchain.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ if [ ! -e $CPP_TOOLCHAIN ]; then
2626
CONDA_LABEL=""
2727

2828
if [ $ARROW_TRAVIS_GANDIVA == "1" ] && [ $TRAVIS_OS_NAME == "osx" ]; then
29-
CONDA_PACKAGES="$CONDA_PACKAGES llvmdev=6.0.1"
29+
CONDA_PACKAGES="$CONDA_PACKAGES llvmdev=$CONDA_LLVM_VERSION"
3030
fi
3131

3232
if [ $TRAVIS_OS_NAME == "linux" ]; then

‎ci/travis_script_manylinux.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
set -ex
2222

2323
pushd python/manylinux1
24-
docker run --shm-size=2g --rm -e PYARROW_PARALLEL=3 -v $PWD:/io -v $PWD/../../:/arrow quay.io/xhochy/arrow_manylinux1_x86_64_base:latest /io/build_arrow.sh
24+
docker run --shm-size=2g --rm -e PYARROW_PARALLEL=3 -v $PWD:/io -v $PWD/../../:/arrow quay.io/xhochy/arrow_manylinux1_x86_64_base:llvm-7-manylinux1 /io/build_arrow.sh
2525

2626
# Testing for https://issues.apache.org/jira/browse/ARROW-2657
2727
# These tests cannot be run inside of the docker container, since TensorFlow

‎cpp/CMakeLists.txt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ endif()
7474

7575
set(BUILD_SUPPORT_DIR "${CMAKE_SOURCE_DIR}/build-support")
7676

77-
set(CLANG_FORMAT_VERSION "6.0")
77+
set(ARROW_LLVM_VERSION "7.0")
78+
STRING(REGEX REPLACE "^([0-9]+)\\.[0-9]+" "\\1" ARROW_LLVM_MAJOR_VERSION "${ARROW_LLVM_VERSION}")
79+
STRING(REGEX REPLACE "^[0-9]+\\.([0-9]+)" "\\1" ARROW_LLVM_MINOR_VERSION "${ARROW_LLVM_VERSION}")
7880
find_package(ClangTools)
7981
if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR CLANG_TIDY_FOUND)
8082
# Generate a Clang compile_commands.json "compilation database" file for use
@@ -123,6 +125,10 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
123125
"Use ccache when compiling (if available)"
124126
ON)
125127

128+
option(ARROW_USE_LD_GOLD
129+
"Use ld.gold for linking on Linux (if available)"
130+
OFF)
131+
126132
option(ARROW_USE_TSAN
127133
"Enable Thread Sanitizer checks"
128134
OFF)

‎cpp/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,15 +292,15 @@ In addition to the arrow dependencies, gandiva requires :
292292
On Ubuntu/Debian you can install these requirements with:
293293

294294
```shell
295-
sudo apt-add-repository -y "deb http://llvm.org/apt/trusty/ llvm-toolchain-trusty-6.0 main"
295+
sudo apt-add-repository -y "deb http://llvm.org/apt/trusty/ llvm-toolchain-trusty-7.0 main"
296296
sudo apt-get update -qq
297-
sudo apt-get install llvm-6.0-dev
297+
sudo apt-get install llvm-7.0-dev
298298
```
299299

300300
On macOS, you can use [Homebrew][1]:
301301

302302
```shell
303-
brew install llvm@6
303+
brew install llvm@7
304304
```
305305

306306
The optional `gandiva` libraries and tests can be built by passing

‎cpp/cmake_modules/FindClangTools.cmake‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,8 @@ set(CLANG_TOOLS_SEARCH_PATHS
5757
"${HOMEBREW_PREFIX}/bin")
5858

5959
find_program(CLANG_TIDY_BIN
60-
NAMES clang-tidy-4.0
61-
clang-tidy-3.9
62-
clang-tidy-3.8
63-
clang-tidy-3.7
64-
clang-tidy-3.6
65-
clang-tidy
60+
NAMES clang-tidy-${ARROW_LLVM_VERSION}
61+
clang-tidy-${ARROW_LLVM_MAJOR_VERSION}
6662
PATHS ${CLANG_TOOLS_SEARCH_PATHS} NO_DEFAULT_PATH
6763
)
6864

@@ -74,35 +70,34 @@ else()
7470
message(STATUS "clang-tidy found at ${CLANG_TIDY_BIN}")
7571
endif()
7672

77-
if (CLANG_FORMAT_VERSION)
73+
if (ARROW_LLVM_VERSION)
7874
find_program(CLANG_FORMAT_BIN
79-
NAMES clang-format-${CLANG_FORMAT_VERSION}
75+
NAMES clang-format-${ARROW_LLVM_VERSION}
76+
clang-format-${ARROW_LLVM_MAJOR_VERSION}
8077
PATHS ${CLANG_TOOLS_SEARCH_PATHS} NO_DEFAULT_PATH
8178
)
8279

8380
# If not found yet, search alternative locations
8481
if ("${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND")
85-
STRING(REGEX REPLACE "^([0-9]+)\\.[0-9]+" "\\1" CLANG_FORMAT_MAJOR_VERSION "${CLANG_FORMAT_VERSION}")
86-
STRING(REGEX REPLACE "^[0-9]+\\.([0-9]+)" "\\1" CLANG_FORMAT_MINOR_VERSION "${CLANG_FORMAT_VERSION}")
8782
if (APPLE)
8883
# Homebrew ships older LLVM versions in /usr/local/opt/llvm@version/
89-
if ("${CLANG_FORMAT_MINOR_VERSION}" STREQUAL "0")
84+
if ("${ARROW_LLVM_MINOR_VERSION}" STREQUAL "0")
9085
find_program(CLANG_FORMAT_BIN
9186
NAMES clang-format
92-
PATHS "${HOMEBREW_PREFIX}/opt/llvm@${CLANG_FORMAT_MAJOR_VERSION}/bin"
87+
PATHS "${HOMEBREW_PREFIX}/opt/llvm@${ARROW_LLVM_MAJOR_VERSION}/bin"
9388
NO_DEFAULT_PATH
9489
)
9590
else()
9691
find_program(CLANG_FORMAT_BIN
9792
NAMES clang-format
98-
PATHS "${HOMEBREW_PREFIX}/opt/llvm@${CLANG_FORMAT_VERSION}/bin"
93+
PATHS "${HOMEBREW_PREFIX}/opt/llvm@${ARROW_LLVM_VERSION}/bin"
9994
NO_DEFAULT_PATH
10095
)
10196
endif()
10297

10398
if ("${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND")
10499
# binary was still not found, look into Cellar
105-
file(GLOB CLANG_FORMAT_PATH "${HOMEBREW_PREFIX}/Cellar/llvm/${CLANG_FORMAT_VERSION}.*")
100+
file(GLOB CLANG_FORMAT_PATH "${HOMEBREW_PREFIX}/Cellar/llvm/${ARROW_LLVM_VERSION}.*")
106101
find_program(CLANG_FORMAT_BIN
107102
NAMES clang-format
108103
PATHS "${CLANG_FORMAT_PATH}/bin"
@@ -119,7 +114,7 @@ if (CLANG_FORMAT_VERSION)
119114
execute_process(COMMAND ${CLANG_FORMAT_BIN} "-version"
120115
OUTPUT_VARIABLE CLANG_FORMAT_FOUND_VERSION_MESSAGE
121116
OUTPUT_STRIP_TRAILING_WHITESPACE)
122-
if (NOT ("${CLANG_FORMAT_FOUND_VERSION_MESSAGE}" MATCHES "^clang-format version ${CLANG_FORMAT_MAJOR_VERSION}\\.${CLANG_FORMAT_MINOR_VERSION}.*"))
117+
if (NOT ("${CLANG_FORMAT_FOUND_VERSION_MESSAGE}" MATCHES "^clang-format version ${ARROW_LLVM_MAJOR_VERSION}\\.${ARROW_LLVM_MINOR_VERSION}.*"))
123118
set(CLANG_FORMAT_BIN "CLANG_FORMAT_BIN-NOTFOUND")
124119
endif()
125120
endif()

0 commit comments

Comments
 (0)