Adar Dembo has posted comments on this change.

Change subject: cmake: add support for out-of-tree builds
......................................................................


Patch Set 13:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/1755/13/.gitignore
File .gitignore:

Line 11
> don't we still expect ctags/cscope stuff to be in the source tree? at least
Nope, all the output is now in the build tree, where the make commands are 
issued.

But that reminds me, I do need to update ctags/etags/cscope to work now that 
there's some generated source in the build tree and static source in the source 
tree.


http://gerrit.cloudera.org:8080/#/c/1755/13//COMMIT_MSG
Commit Message:

Line 44:   a cmake target, so I was forced to use file(COPY...), which executes 
the
> I thought in the current rev it does a symlink?
Right, I need to update this.


http://gerrit.cloudera.org:8080/#/c/1755/13/README.adoc
File README.adoc:

Line 27: uild directories may be anywhere in the filesystem
       : except for in the sour
> hrm, thought I commented on this but maybe not -- this should more specific
Will clarify.


http://gerrit.cloudera.org:8080/#/c/1755/13/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

Line 118: TEST_LOGDIR="$BUILD_ROOT/test-logs"
> just to confirm: this nets out to the same directory so we shouldn't have t
Yes, it should be the same directory.


http://gerrit.cloudera.org:8080/#/c/1755/13/docs/installation.adoc
File docs/installation.adoc:

Line 243: thirdparty/installed/bin/cmake .. -DCMAKE_BUILD_TYPE=release 
-DCMAKE_INSTALL_PREFIX=/opt/kudu
> Here, and elsewhere, should be ../thirdparty
Yep, good catch.


http://gerrit.cloudera.org:8080/#/c/1755/13/docs/support/scripts/make_site.sh
File docs/support/scripts/make_site.sh:

> Did you test this?
Yes, I ran it end to end. It didn't fail, and it produced some output. How can 
I tell if it's correct?


Line 39: BUILD_ROOT
> Hmm. Why is it necessary to blow away the whole build directory?
It's not strictly necessary, but I was being consistent with other "end-to-end" 
scripts (like tpch.sh and benchmarks.sh) which issue "make clean" before 
running. This is the least error prone way to clean out old makefiles.


-- 
To view, visit http://gerrit.cloudera.org:8080/1755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I76e17a1102b79ac0e25a505b54347db3bb436ede
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Martin Grund <[email protected]>
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to