Adar Dembo has posted comments on this change. Change subject: Do not add thirdparty/bin to PATH in during builds ......................................................................
Patch Set 6: (5 comments) I presume you've checked that symbolization still works properly, for both ASAN and TSAN. Checking the latter is easy (i.e. remove the supressions); you might need to induce a use-after-free or equivalent for the former. http://gerrit.cloudera.org:8080/#/c/1678/6/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: Line 137: export PPROF_PATH=$(pwd)/thirdparty/installed/bin/pprof Could you use THIRDPARTY_BIN here? http://gerrit.cloudera.org:8080/#/c/1678/6/build-support/run-test.sh File build-support/run-test.sh: Line 89: export ASAN_SYMBOLIZER_PATH=$ROOT/thirdparty/clang-toolchain/bin/llvm-symbolizer So why clang-toolchain and not installed? Ideally we want the path to the clang that we know was used to build Kudu be it llvm 3.4.2 from thirdparty, clang-toolchain, or a system clang. But I guess we can't figure that out here, so this is an approximation? And when we move to C++11 and update llvm 3.4.2 to 3.7, we can go back to installed? Line 142: | c++filt \ I think we can drop c++filt for the same reason, right? IIRC it was added along with asan_symbolize.py. http://gerrit.cloudera.org:8080/#/c/1678/6/src/kudu/scripts/benchmarks.sh File src/kudu/scripts/benchmarks.sh: Line 170: export PPROF_PATH=$BASE_DIR/thirdparty/installed/bin/pprof Use THIRDPARTY_BIN here too. http://gerrit.cloudera.org:8080/#/c/1678/6/src/kudu/scripts/tpch.sh File src/kudu/scripts/tpch.sh: Line 92: export PPROF_PATH=$(pwd)/thirdparty/installed/bin/pprof And here. -- To view, visit http://gerrit.cloudera.org:8080/1678 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fabe02686eca5e1ec3dbf901d95f36d0443dc41 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
