Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10352 )
Change subject: thirdparty: tweak clang compiler flags ...................................................................... Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10352/2/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/10352/2/thirdparty/build-definitions.sh@311 PS2, Line 311: # Depend on zlib from the thirdparty tree. It's an optional dependency for : # LLVM, but a required [1] one for IWYU. : # : # 1. https://github.com/include-what-you-use/include-what-you-use/issues/539 : CLANG_CXXFLAGS="$CLANG_CXXFLAGS -I$PREFIX/include" : CLANG_LDFLAGS="-L$PREFIX/lib -Wl,-rpath,$PREFIX/lib" So it turns out this isn't actually necessary for TSAN builds. If you look at build-thirdparty.sh, you'll see that EXTRA_CXXFLAGS and EXTRA_LDFLAGS are already modified to include the PREFIX directory because every TSAN-instrumented dependency needs to depend on libc++ and libc++abi. I think this code might be easier to follow if this were isolated into the "normal" section above. Then I think you could feed CLANG_LDFLAGS unconditionally into both CMAKE_CXX_FLAGS and CMAKE_EXE_LINKER_FLAGS. On macOS, this would have no effect because CLANG_LDFLAGS is empty in the "normal" case (unless someone overrides LDFLAGS when calling build-thirdparty.sh, but that isn't done). On Linux, this would continue to do the right thing without duplicating the contents of CLANG_LDFLAGS for "tsan" as it currently does now. -- To view, visit http://gerrit.cloudera.org:8080/10352 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6 Gerrit-Change-Number: 10352 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 09 May 2018 18:52:04 +0000 Gerrit-HasComments: Yes
