Repository: incubator-impala Updated Branches: refs/heads/master 2598e3b26 -> be8d15122
IMPALA-4701: make distcc work reliably with clang The previous compiler-switching method in distcc had some problems: * It duplicated logic from CMakeLists.txt in choosing flags to pass to clang * In order to switch to ASAN, you needed to remember to change distcc settings. * The wrapper script approach interacted badly with ccache - GCC and Clang-generated artifacts would be treated as equivalent by ccache, meaning that if you accidentally build with the wrong compiler setting and the artifacts got into the cache you needed to clear ccache. Instead of using environment variables to set the compiler, we now pass the compiler as an argument to distcc.sh and set things up in CMakeLists.txt the same way as ccache. Switching to/from clang builds now requires no extra step (aside from cleaning out the cmake-generated files with clean.sh). Also changes the name of the config file Testing: Tested switching between debug and asan builds locally. Change-Id: I199b245fb14b6c3484b66339a7d4b37d74755af7 Reviewed-on: http://gerrit.cloudera.org:8080/6493 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/fd1b40d5 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/fd1b40d5 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/fd1b40d5 Branch: refs/heads/master Commit: fd1b40d5719745200dcb41ac248f1ab7042c1164 Parents: 2598e3b Author: Tim Armstrong <[email protected]> Authored: Mon Mar 27 09:38:39 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Mon Apr 3 22:48:05 2017 +0000 ---------------------------------------------------------------------- be/CMakeLists.txt | 13 ++++++++++-- bin/clean.sh | 2 +- bin/distcc/README.md | 12 ----------- bin/distcc/distcc.sh | 28 ++++++++++++-------------- bin/distcc/distcc_env.sh | 46 +++++++++++++++++++++---------------------- 5 files changed, 47 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd1b40d5/be/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt index 03c3663..6f3acff 100644 --- a/be/CMakeLists.txt +++ b/be/CMakeLists.txt @@ -144,9 +144,9 @@ SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${LLVM_CFLAGS}") # Use ccache when found and not explicitly disabled by setting the DISABLE_CCACHE envvar. find_program(CCACHE ccache) +set(RULE_LAUNCH_PREFIX) if (CCACHE AND NOT DEFINED ENV{DISABLE_CCACHE}) - set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache) - set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) + set(RULE_LAUNCH_PREFIX ccache) if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER" OR "${CMAKE_BUILD_TYPE}" STREQUAL "TIDY" OR "${CMAKE_BUILD_TYPE}" STREQUAL "UBSAN") @@ -160,6 +160,15 @@ if (CCACHE AND NOT DEFINED ENV{DISABLE_CCACHE}) endif() endif() +if(DEFINED ENV{IMPALA_DISTCC_ENABLED}) + if ($ENV{IMPALA_DISTCC_ENABLED} STREQUAL "true") + SET(RULE_LAUNCH_PREFIX "${RULE_LAUNCH_PREFIX} $ENV{IMPALA_HOME}/bin/distcc/distcc.sh") + endif() +endif() + +set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ${RULE_LAUNCH_PREFIX}) +set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ${RULE_LAUNCH_PREFIX}) + # Thrift requires these definitions for some types that we use add_definitions(-DHAVE_INTTYPES_H -DHAVE_NETINET_IN_H -DHAVE_NETDB_H) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd1b40d5/bin/clean.sh ---------------------------------------------------------------------- diff --git a/bin/clean.sh b/bin/clean.sh index 7d5b315..8c009c7 100755 --- a/bin/clean.sh +++ b/bin/clean.sh @@ -80,4 +80,4 @@ FIND_ARGS=("$IMPALA_HOME" -iname '*cmake*' -not -name CMakeLists.txt \ if [[ -n "$IMPALA_TOOLCHAIN" ]]; then FIND_ARGS+=(-not -path "$IMPALA_TOOLCHAIN/*") fi -find "${FIND_ARGS[@]}" | xargs rm -Rf +find "${FIND_ARGS[@]}" -exec rm -Rf {} + http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd1b40d5/bin/distcc/README.md ---------------------------------------------------------------------- diff --git a/bin/distcc/README.md b/bin/distcc/README.md index d6fe8b7..df1913a 100644 --- a/bin/distcc/README.md +++ b/bin/distcc/README.md @@ -66,18 +66,6 @@ to switch back ``` switch_compiler distcc ``` - -### Switch to clang++ -Clang is faster and gives better error messages. This setup is still somewhat -experimental. -``` -switch_compiler clang -``` -to switch back -``` -switch_compiler gcc -``` - ### Second time If you open a new terminal and attempt to build with "make" or "bin/make_impala.sh", that will fail. To fix: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd1b40d5/bin/distcc/distcc.sh ---------------------------------------------------------------------- diff --git a/bin/distcc/distcc.sh b/bin/distcc/distcc.sh index a1136e8..9173706 100755 --- a/bin/distcc/distcc.sh +++ b/bin/distcc/distcc.sh @@ -17,7 +17,17 @@ # specific language governing permissions and limitations # under the License. -if [[ -z "$DISTCC_HOSTS" || -z "$IMPALA_REAL_CXX_COMPILER" ]]; then +set -euo pipefail + +CXX_COMPILER="$1" +shift + +if [[ ! -x "$CXX_COMPILER" ]]; then + echo Configured compiler "$CXX_COMPILER" is not executable. + exit 1 +fi + +if [[ -z "$DISTCC_HOSTS" || -z "$CXX_COMPILER" ]]; then # This could be sourced here and the build would work but the parallelization (-j) # should be wrong at this point and it's too late to fix. DIR=$(dirname "$0") @@ -41,22 +51,10 @@ fi CMD= CMD_POST_ARGS= -if $IMPALA_USE_DISTCC; then +if $IMPALA_DISTCC_LOCAL; then CMD="distcc ccache" fi -GCC_ROOT="$TOOLCHAIN_DIR/gcc-$IMPALA_GCC_VERSION" -case "$IMPALA_REAL_CXX_COMPILER" in - gcc) CMD+=" $GCC_ROOT/bin/g++";; - clang) # Assume the compilation options were setup for gcc, which would happen using - # default build options. Now some additional options need to be added for clang. - CMD+=" $TOOLCHAIN_DIR/llvm-$IMPALA_LLVM_ASAN_VERSION/bin/clang++" - CMD+=" --gcc-toolchain=$GCC_ROOT" - # -Wno-unused-local-typedef needs to go after -Wall - # -Wno-error is needed, clang generates more warnings than gcc. - CMD_POST_ARGS+=" -Wno-unused-local-typedef -Wno-error";; - *) echo "Unexpected IMPALA_REAL_CXX_COMPILER: '$IMPALA_REAL_CXX_COMPILER'" 1>&2 - exit 1;; -esac +CMD+=" $CXX_COMPILER" exec $CMD "$@" $CMD_POST_ARGS http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/fd1b40d5/bin/distcc/distcc_env.sh ---------------------------------------------------------------------- diff --git a/bin/distcc/distcc_env.sh b/bin/distcc/distcc_env.sh index 173cc18..d92044a 100644 --- a/bin/distcc/distcc_env.sh +++ b/bin/distcc/distcc_env.sh @@ -66,13 +66,13 @@ DISTCC_HOSTS+=" --localslots_cpp=$(nproc)" DISTCC_HOSTS+=" --randomize" DISTCC_HOSTS+=" ${BUILD_FARM}" -# The compiler that distcc.sh should use: gcc or clang. -: ${IMPALA_REAL_CXX_COMPILER=} -export IMPALA_REAL_CXX_COMPILER +# Set to true to enable the distcc.sh wrapper script. Takes effect when CMake is next run. +export IMPALA_DISTCC_ENABLED=true -# Set to false to use local compilation instead of distcc. -: ${IMPALA_USE_DISTCC=} -export IMPALA_USE_DISTCC +# Set to false to make distcc.sh use local compilation instead of distcc. Takes effect +# immediately if the distcc.sh wrapper script is used. +: ${IMPALA_DISTCC_LOCAL=} +export IMPALA_DISTCC_LOCAL # Even after generating make files, some state about compiler options would only exist in # environment vars. Any such vars should be saved to this file so they can be restored. @@ -80,12 +80,12 @@ if [[ -z "$IMPALA_HOME" ]]; then echo '$IMPALA_HOME must be set before sourcing this file.' 1>&2 return 1 fi -IMPALA_COMPILER_CONFIG_FILE="$IMPALA_HOME/.impala_compiler_opts" +IMPALA_COMPILER_CONFIG_FILE="$IMPALA_HOME/.impala_compiler_opts_v2" # Completely disable anything that could have been setup using this script and clean # the make files. function disable_distcc { - export IMPALA_CXX_COMPILER=default + export IMPALA_DISTCC_ENABLED=false export IMPALA_BUILD_THREADS=$(nproc) save_compiler_opts if ! clean_cmake_files; then @@ -97,8 +97,7 @@ function disable_distcc { } function enable_distcc { - export IMPALA_CXX_COMPILER="$DISTCC_ENV_DIR"/distcc.sh - switch_compiler distcc gcc + switch_compiler distcc export IMPALA_BUILD_THREADS=$(distcc -j) if ! clean_cmake_files; then echo Failed to clean cmake files. 1>&2 @@ -116,27 +115,27 @@ function clean_cmake_files { return 1 fi # Copied from $IMPALA_HOME/bin/clean.sh. - find "$IMPALA_HOME" -iname '*cmake*' -not -name CMakeLists.txt \ - -not -path '*cmake_modules*' \ - -not -path '*thirdparty*' | xargs rm -rf + FIND_ARGS=("$IMPALA_HOME" -iname '*cmake*' -not -name CMakeLists.txt \ + -not -path "$IMPALA_HOME/cmake_modules*" \ + -not -path "$IMPALA_HOME/thirdparty*") + if [[ -n "$IMPALA_TOOLCHAIN" ]]; then + FIND_ARGS+=(-not -path "$IMPALA_TOOLCHAIN/*") + fi + find "${FIND_ARGS[@]}" -exec rm -Rf {} + } function switch_compiler { for ARG in "$@"; do case "$ARG" in "local") - IMPALA_USE_DISTCC=false + IMPALA_DISTCC_LOCAL=false IMPALA_BUILD_THREADS=$(nproc);; distcc) - IMPALA_USE_DISTCC=true + IMPALA_DISTCC_LOCAL=true IMPALA_BUILD_THREADS=$(distcc -j);; - gcc) IMPALA_REAL_CXX_COMPILER=gcc;; - clang) IMPALA_REAL_CXX_COMPILER=clang;; *) echo "Valid compiler options are: - 'local' - Don't use distcc and set -j value to $(nproc). (gcc/clang) remains unchanged. - 'distcc' - Use distcc and set -j value to $(distcc -j). (gcc/clang) remains unchanged. - 'gcc' - Use gcc. (local/distcc remains unchanged). - 'clang' - Use clang. (local/distcc remains unchanged)." 2>&1 + 'local' - Don't use distcc and set -j value to $(nproc). + 'distcc' - Use distcc and set -j value to $(distcc -j)." 2>&1 return 1;; esac done @@ -146,10 +145,9 @@ function switch_compiler { function save_compiler_opts { rm -f "$IMPALA_COMPILER_CONFIG_FILE" cat <<EOF > "$IMPALA_COMPILER_CONFIG_FILE" -IMPALA_CXX_COMPILER=$IMPALA_CXX_COMPILER +IMPALA_DISTCC_ENABLED=$IMPALA_DISTCC_ENABLED IMPALA_BUILD_THREADS=$IMPALA_BUILD_THREADS -IMPALA_USE_DISTCC=$IMPALA_USE_DISTCC -IMPALA_REAL_CXX_COMPILER=$IMPALA_REAL_CXX_COMPILER +IMPALA_DISTCC_LOCAL=$IMPALA_DISTCC_LOCAL EOF }
