You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@ip-10-146-233-104.ec2.internal> on 2016/01/09 00:32:16 UTC

[kudu-CR] cmake: add support for and require out-of-tree builds

Hello Mike Percy, Todd Lipcon,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/1755

to review the following change.

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

cmake: add support for and require out-of-tree builds

One of cmake's selling points is its ease of isolating source trees from
build directories so that one tree can be used for multiple builds, each
configured differently. With judicious git use, each build can even be
based on different git hashes. Many cmake-based projects take care to
preserve this isolation, some even go as far as prohibiting in-tree builds
so that developers are really forced to isolate build files from source
files. Today's Kudu does not build out-of-tree; this patch fixes that, and
then some.

For isolation to work we must respect the difference between
CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR. For in-tree builds they are
identical, and perhaps that's why, over time, we've chosen the former when
we should have chosen the latter. The bulk of the patch deals with this, and
with the resulting fallout (e.g. with generated .pb.h files now in the build
directory, it needs to be added to include_directories()).

The other major (and disruptive) part of the patch is this: like some
cmake-based projects, we now prohibit the use of the source tree as a build
directory. This ensures that all build output is well isolated from source
code. It also helps deal with an annoying cmake quirk: if the source tree is
used as a build directory and thus winds up containing a CMakeCache.txt
file, future out-of-tree builds will fail. This change forced updates to
various "end to end" build scripts. The most annoying was make_site.sh,
which I'm still not sure if I got right.

Finally, some additional notes:
- Build output is no longer in .gitignore, so that we'll notice if it leaks
  into the source tree.
- With support for multiple build directories now baked-in, I've removed the
  build/<build_type> approximation we used to do. Build output continues to
  go into a 'latest' subdirectory for compatibility with existing scripts.
- Due to the loss of the direct link between the build directory and source
  tree, unit tests and their resources must now be copied to the build
  directory. This was already the case for executables, but not for scripts,
  which were executed from the source tree. Sadly, add_test() doesn't create
  a cmake target, so I was forced to use file(COPY...), which executes the
  copy during cmake (rather than make).

Change-Id: I76e17a1102b79ac0e25a505b54347db3bb436ede
---
M .gitignore
M CMakeLists.txt
M README.adoc
M build-support/jenkins/build-and-test.sh
M build-support/run-test.sh
M cmake_modules/FindKRPC.cmake
M cmake_modules/FindProtobuf.cmake
M docs/installation.adoc
M docs/support/scripts/make_docs.sh
M docs/support/scripts/make_site.sh
M python/kudu/tests/test_kudu.py
M python/setup.py
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client_samples-test.sh
M src/kudu/client/client_symbol-test.sh
M src/kudu/codegen/CMakeLists.txt
M src/kudu/scripts/benchmarks.sh
M src/kudu/scripts/tpch.sh
M src/kudu/twitter-demo/CMakeLists.txt
M src/kudu/twitter-demo/parser-test.cc
M src/kudu/util/CMakeLists.txt
21 files changed, 294 insertions(+), 271 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/1755/1
-- 
To view, visit http://gerrit.cloudera.org:8080/1755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I76e17a1102b79ac0e25a505b54347db3bb436ede
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@cloudera.com>