Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11176 )
Change subject: [build] Move default sanitizer options into build from shell scripts ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/11176/8/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/11176/8/CMakeLists.txt@1221 PS8, Line 1221: set(KRB5_REALM_OVERRIDE -Wl,-U,krb5_realm_override_loaded krb5_realm_override) > The old comment said that macOS's krb5 is new enough that it doesn't need t It said the reason it didn't do it was because the flag was not supported. Since I found a supported flag, I added it for consistency. This makes Mac behave the same way as Linux. http://gerrit.cloudera.org:8080/#/c/11176/8/build-support/run-test.sh File build-support/run-test.sh: http://gerrit.cloudera.org:8080/#/c/11176/8/build-support/run-test.sh@115 PS8, Line 115: # By default the sanitizers use addr2line utility to symbolize reports. : # llvm-symbolizer is faster, consumes less memory and produces much better reports. : # https://github.com/google/sanitizers/wiki/SanitizerCommonFlags : SYMBOLIZER_PATH="$SOURCE_ROOT/thirdparty/installed/uninstrumented/bin/llvm-symbolizer" : for SANITIZER in ASAN LSAN MSAN TSAN UBSAN; do : VAR_NAME="${SANITIZER}_OPTIONS" : # Only add the external_symbolizer_path if it is not set already. : if [[ ${!VAR_NAME} != *"external_symbolizer_path="* ]]; then : export ${SANITIZER}_OPTIONS="$(echo ${!VAR_NAME}) external_symbolizer_path=$SYMBOLIZER_PATH" : fi : done > Can all of the shell versions of this snippet be decomposed into a helper s There are only 2 copies right now, and I think this one can go away once we move the local llvm-symbolizer config to the binary. http://gerrit.cloudera.org:8080/#/c/11176/8/src/kudu/cfile/CMakeLists.txt File src/kudu/cfile/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/11176/8/src/kudu/cfile/CMakeLists.txt@58 PS8, Line 58: set(KUDU_TEST_LINK_LIBS ${KUDU_MIN_TEST_LIBS} cfile) > It would be nice to enforce this by making KUDU_TEST_LINK_LIBS additive onl I tried modifying the build with functions and macros all failing to work. I will take another crack at it in the morning. -- To view, visit http://gerrit.cloudera.org:8080/11176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9eab2d3ef0b107ae9a4e971bf8a6514bf425f645 Gerrit-Change-Number: 11176 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 24 Aug 2018 03:49:59 +0000 Gerrit-HasComments: Yes
