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 10:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11176/8/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/11176/8/CMakeLists.txt@1279
PS8, Line 1279:
> extra space here
Done


http://gerrit.cloudera.org:8080/#/c/11176/8/CMakeLists.txt@1280
PS8, Line 1280: AGS "$
> could be
Done


http://gerrit.cloudera.org:8080/#/c/11176/9/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/11176/9/CMakeLists.txt@1281
PS9, Line 1281:
> Do we still need these two in the list now that we have our fancy macro?
yeah, these are still the base KUDU_MIN_TEST_LIBS.


http://gerrit.cloudera.org:8080/#/c/11176/9/CMakeLists.txt@1284
PS9, Line 1284:
> You mean KUDU_TEST_LINK_LIBS here.
Done


http://gerrit.cloudera.org:8080/#/c/11176/9/CMakeLists.txt@1291
PS9, Line 1291: # Prepend SANITIZER_OPTIONS_OVERRIDE if this is a sanitizer 
build.
> Have you looked into cmake's list() functions? They may simplify this a bit
Done


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: # Set a 15-minute timeout for tests run via 'make test'.
             : # This keeps our jenkins builds from hanging in the case that 
there's
             : # a deadlock or anything.
             : #
             : # NOTE: this should be kept in sync with the default value of 
ARG_TIMEOUT
             : # in the definition of ADD_KUDU_TEST in the top-level 
CMakeLists.txt.
             : KUDU_TEST_TIMEOUT=${KUDU_TEST_TIMEOUT:-900}
             :
             : # Allow for collecting core dumps.
             : KUDU_TEST_ULIMIT_CORE=${KUDU_TEST_ULIMIT_CORE:-0}
             : ulim
> I added TODOs to follow up and remove both of these with a patch that pushe
I learned a bit more about CMake and pushed this down into the build and 
removed it from both locations.


http://gerrit.cloudera.org:8080/#/c/11176/9/src/kudu/benchmarks/CMakeLists.txt
File src/kudu/benchmarks/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/11176/9/src/kudu/benchmarks/CMakeLists.txt@59
PS9, Line 59: SET_KUDU_TEST_LINK_LIBS(tpch)
> Not using the macro here?
Done



--
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: 10
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Fri, 24 Aug 2018 17:17:59 +0000
Gerrit-HasComments: Yes

Reply via email to