Wip cmake make check #7912

alimaredia · 2016-03-03T20:46:07Z

As long as these commit's don't break make check that jenkins runs, and the numerous other changes are agreed upon, there's no reason this CMake work shouldn't go be in ASAP

alimaredia · 2016-03-03T20:47:48Z

@cbodley scrutanize away!

alimaredia · 2016-03-03T20:49:42Z

src/vstart.sh

+        [ -z "$CEPH_LIB" ] && CEPH_LIB=.libs
+        [ -z $EC_PATH ] && EC_PATH=$CEPH_LIB
+        [ -z $CS_PATH ] && CS_PATH=$CEPH_LIB
+        [ -z $OBJCLASS_PATH ] && OBJCLASS_PATH=$CEPH_LIB


@jcsp here are the changes made to vstart.sh. A good place to look at where some of these variables are defined is cmake/modules/AddCephTest.cmake

No objection from me to any of the vstart stuff here (I'm assuming you've tried it and it all works) -- can you separate it into its own commit and bring it over to #7908?

Might also be worth checking that none of the other regular cmake users (@oritwas @tchaikov perhaps?) have any comments on the change to build everything in bin/ and lib/ before we merge it in #7908

I don't have any problem with building everything in bin/ and lib/

dillaman · 2016-03-03T21:06:58Z

src/test/common/test_blkdev.cc

-    set_block_device_sandbox_dir(root);
+    string root;
+    if (i == 0) {
+      string CEPH_ROOT = getenv("CEPH_ROOT");


Protect against getenv returning nullptr.

alimaredia · 2016-03-03T21:34:46Z

@dachary @tchaikov As long as make check passes in jenkins with automake, do you anticipate any of these changes breaking any parts of the Ceph CI processes (ex: teuthology)?

cbodley · 2016-03-03T21:37:24Z

src/test/cls_rbd/CMakeLists.txt

+add_executable(ceph_test_cls_rbd
+  test_cls_rbd.cc
+  ${CMAKE_SOURCE_DIR}/src/common/TextTable.cc
+  ${CMAKE_SOURCE_DIR}/src/common/secret.c


are these two not part of libcommon? we shouldn't have to recompile them here

ghost · 2016-03-04T02:28:37Z

As long as make check passes in jenkins with automake, do you anticipate any of these changes breaking any parts of the Ceph CI processes (ex: teuthology)?

The only thing that could break teuthology is if you change files in the qa directory or in what is found in the ceph packages.

cbodley · 2016-03-04T14:51:59Z

@alimaredia there are some linker errors with the test_librgw_file* targets with undefined references to heap profiler stuff

they only try to build under make, not make check, so they need to be updated to use add_ceph_unittest()

cbodley · 2016-03-04T15:51:35Z

The only thing that could break teuthology is if you change files in the qa directory or in what is found in the ceph packages.

@dachary qa/workunits/ceph-helpers.sh was changed to support out-of-tree builds (relative paths to binaries were replaced with environment variables). It was pointing to stuff in .libs (which wouldn't come from installed ceph packages), so I would assume that it isn't used by teuthology?

cbodley · 2016-03-04T16:33:14Z

the diff for src/test/CMakeLists.txt is too big for github, so comments here..

464: add_ceph_test(test-ceph-helpers.sh ${CMAKE_SOURCE_DIR}/src/test/test-ceph-helpers.sh)
use ${CMAKE_CURRENT_SOURCE_DIR}

478: add_dependencies(check run-tox) should be run-tox-ceph-disk?

481: add_dependencies(check run-tox) should be run-tox-ceph-detect-init?

483: set_property(TEST run-tox-ceph-disk ...
use add_ceph_test() if it works, or update PYTHONPATH= for cython path

ghost · 2016-03-04T16:43:04Z

qa/workunits/ceph-helpers.sh was changed to support out-of-tree builds (relative paths to binaries were replaced with environment variables). It was pointing to stuff in .libs (which wouldn't come from installed ceph packages), so I would assume that it isn't used by teuthology?

It is used by teuthology, reason why it is in qa/workunits:

workunits/cephtool/test.sh:3:source $(dirname $0)/../ceph-helpers.sh
workunits/rbd/journal.sh:3:. $(dirname $0)/../ceph-helpers.sh
workunits/rbd/rbd-nbd.sh:3:. $(dirname $0)/../ceph-helpers.sh
workunits/rbd/test_admin_socket.sh:7:. $(dirname $0)/../ceph-helpers.sh

Added a CMakeLists.txt into test/rgw. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Added a CMakeLists.txt into test/system. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Added src/librbd and test/librbd CMakeLists.txt's, various previously missing libraries and targets, revised unittest_librbd. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test/mon/osd-crush.sh with CEPH_VAR environment variables set in cmake. NOTE: test is not passing yet, possibly due to the filesystem. this commit is a work in progress Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test/cephtool-test-rados.sh, qa/workunits/rados/test_rados_tool.sh, src/test/vstart_wrapper.sh, with CEPH_FOO environment variables set in cmake. Also edited the CMake path finding logic in vstart.sh with the CEPH_FOO variables. NOTE: test is not passing yet, possibly due to the filesystem. this commit is a work in progress Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test/cephtool-test-mon.sh, qa/workunits/cephtool/test.sh, and test/cephtool-test-mon.sh to work with CEPH_FOO environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in unittest_bufferlist.sh to work with CEPH_FOO environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in shell scripts in test/ceph_objectstore_tool.py and init-ceph.in to work with CEPH_FOO environment variables set in cmake. Also added CEPH_BUILD_DIR environment variable set to CMAKE_BINARY_DIR. It is used in init-ceph and ceph_objectstore_tool.py. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test-erausure-eio.sh test-erasure-code.sh, and replaced .libs in erasure_code unittests with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test_objectstore_memstore.sh with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced .libs in TestClassHandler.cc with CEPH_LIB Signed-off-by: Ali Maredia <amaredia@redhat.com>

Added missing dependencies and link libraries for erasure-decode-non-regression.sh and added absolute paths to ceph_erasure_code_non_regression.cc. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Signed-off-by: Ali Maredia <amaredia@redhat.com>

Replaced relative paths in test_pidfile.sh with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

Signed-off-by: Ali Maredia <amaredia@redhat.com>

Added rbd_mirror libraries, unit tests, executables Signed-off-by: Ali Maredia <amaredia@redhat.com>

Signed-off-by: Ali Maredia <amaredia@redhat.com>

tchaikov · 2016-04-15T03:44:12Z

src/test/librados/CMakeLists.txt

+  )
+set_target_properties(ceph_test_rados_api_list PROPERTIES COMPILE_FLAGS
+  ${UNITTEST_CXX_FLAGS})
+target_link_libraries(ceph_test_rados_api_list


need to link against "global", see 769c0af

see 769c0af Signed-off-by: Kefu Chai <kchai@redhat.com>

tchaikov · 2016-04-15T03:52:48Z

@alimaredia i am trying to get your branch to pass the build at http://gitbuilder.sepia.ceph.com/gitbuilder-ceph-tarball-trusty-amd64-cmake/#origin/wip-cmake-make-check, so i added my fix on top of yours, please feel free to squash it into your corresponding commit(s) (83ff750).

trociny · 2016-04-18T06:11:07Z

src/test/librados_test_stub/TestClassHandler.cc

-  DIR *dir = ::opendir(".libs");
+  const char* env = getenv("CEPH_LIB");
+  std::string CEPH_LIB(env ? env : "lib");
+  DIR *dir = ::opendir(CEPH_LIB.c_str());
  if (dir == NULL) {


@alimaredia @cbodley Now if CEPH_LIB env is not set, "lib" is used instead of previously used ".libs". Was it intentionally?

% ./unittest_rbd_mirror 2016-04-18 09:10:23.392541 7fb9f8fdeb00 -1 WARNING: the following dangerous and experimental features are enabled: * [==========] Running 58 tests from 11 test cases. [----------] Global test environment set-up. [----------] 1 test from TestMockImageReplayer [ RUN ] TestMockImageReplayer.Blah test/librados_test_stub/TestClassHandler.cc: In function 'void librados::TestClassHandler::open_all_classes()' thread 7fb9f8fdeb00 time 2016-04-18 09:10:23.500613 test/librados_test_stub/TestClassHandler.cc: 52: FAILED assert(false)

@trociny This was done intentionally. ".libs" is an autotools convention and since we are moving toward using CMake. The "lib" was chosen because all the libraries in a CMake build go to ${CMAKE_BUILD_DIR}/lib.

I would love it if you transitioned to CMake and CTest (the environment variables are predefined there all you have to do is run ctest -V -R unittest_rbd_mirror but I understand your concern.

One thing I can do is add a check for if opendir(".libs") is true i can set *dir to that to make sure this test and others with the similar revisions work without running make check in the automake build. How does that sound?

@alimaredia If it is intentional, I am fine with it. Not sure it is necessary to do any additional checks for compatibility, it might only increase the transition period. A more useful thing I think is to announce the new way to developers. It may already be done and I just missed it? At least a message in ceph-dev would be nice.

@trociny @alimaredia I added the automatic setting of CEPH_LIB using .libs in my pull request #8649 just for the ceph_objectstore_tool.py test.

@trociny I've made the appropriate adjustments in #8770. I did end up changing "lib" to ".libs" since that is a good default that CEPH_LIB should revert back to for the time being

alimaredia mentioned this pull request Mar 3, 2016

cmake: Cython modules building with cmake #7908

Closed

alimaredia reviewed Mar 3, 2016
View reviewed changes

liewegas added the build/ops label Mar 3, 2016

dillaman reviewed Mar 3, 2016
View reviewed changes

cbodley reviewed Mar 3, 2016
View reviewed changes

cbodley mentioned this pull request Mar 4, 2016

debian: package librgw_file* tests #7930

Merged

alimaredia force-pushed the wip-cmake-make-check branch 3 times, most recently from de1619f to 96ea965 Compare March 4, 2016 20:45

alimaredia added 23 commits April 14, 2016 20:48

cmake: moved tests into test/rgw dir

803def3

Added a CMakeLists.txt into test/rgw. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: moved tests into test/system dir

54eb382

Added a CMakeLists.txt into test/system. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: librbd fixes

8e3e3f4

Added src/librbd and test/librbd CMakeLists.txt's, various previously missing libraries and targets, revised unittest_librbd. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: working on rados-striper.sh test

d189270

Replaced relative paths in test/mon/osd-crush.sh with CEPH_VAR environment variables set in cmake. NOTE: test is not passing yet, possibly due to the filesystem. this commit is a work in progress Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: cephtool-test-rados/mds/osd passing in cmake

cf24535

Replaced relative paths in test/cephtool-test-mon.sh, qa/workunits/cephtool/test.sh, and test/cephtool-test-mon.sh to work with CEPH_FOO environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: unittest_bufferlist passing in cmake

f43dd0f

Replaced relative paths in unittest_bufferlist.sh to work with CEPH_FOO environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: erasure-code tests passing

1c8cd00

Replaced relative paths in test-erausure-eio.sh test-erasure-code.sh, and replaced .libs in erasure_code unittests with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: test_objectstore_memstore.sh test passing

49da534

Replaced relative paths in test_objectstore_memstore.sh with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

unittest_blkdev: absolute path for cmake make check

dd0ac99

Signed-off-by: Ali Maredia <amaredia@redhat.com>

unittest_journal: got rid of relative paths

fd978ce

Replaced .libs in TestClassHandler.cc with CEPH_LIB Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: erasure-decode-non-regression.sh passing

cfcbe21

Added missing dependencies and link libraries for erasure-decode-non-regression.sh and added absolute paths to ceph_erasure_code_non_regression.cc. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Protect againist getenv returning nullptr

de97f67

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: test_pidfile.sh test passing

9288e98

Replaced relative paths in test_pidfile.sh with CEPH_VAR environment variables set in cmake. Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Added an absolute path for libs

21ec89f

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Added rbd_mirror targets

60e5fa9

Added rbd_mirror libraries, unit tests, executables Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Add librgw_file tests

a7b6c01

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Building ceph-disk from source code

4fc06be

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Building ceph-detect-init from source code

b4de745

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: CMake environment variables added to automake

56c37ea

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: remove repeat OSD.cc

e0bda0b

Signed-off-by: Ali Maredia <amaredia@redhat.com>

cmake: Add cython_modules to make check

8ddfcb1

Signed-off-by: Ali Maredia <amaredia@redhat.com>

alimaredia force-pushed the wip-cmake-make-check branch from c4d220c to 8ddfcb1 Compare April 15, 2016 00:48

tchaikov reviewed Apr 15, 2016
View reviewed changes

cmake: link ceph_test_rados_api_list against libglobal

28b91f2

see 769c0af Signed-off-by: Kefu Chai <kchai@redhat.com>

cbodley merged commit 69f8174 into master Apr 15, 2016

trociny reviewed Apr 18, 2016
View reviewed changes

tchaikov deleted the wip-cmake-make-check branch April 19, 2016 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wip cmake make check #7912

Wip cmake make check #7912

alimaredia commented Mar 3, 2016

alimaredia commented Mar 3, 2016

alimaredia Mar 3, 2016

jcsp Mar 3, 2016

oritwas Mar 4, 2016

dillaman Mar 3, 2016

alimaredia commented Mar 3, 2016

cbodley Mar 3, 2016

ghost commented Mar 4, 2016

cbodley commented Mar 4, 2016

cbodley commented Mar 4, 2016

cbodley commented Mar 4, 2016

ghost commented Mar 4, 2016

tchaikov Apr 15, 2016

tchaikov commented Apr 15, 2016

trociny Apr 18, 2016

alimaredia Apr 18, 2016

trociny Apr 19, 2016

dzafman Apr 19, 2016

alimaredia Apr 26, 2016

Wip cmake make check #7912

Wip cmake make check #7912

Conversation

alimaredia commented Mar 3, 2016

alimaredia commented Mar 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alimaredia commented Mar 3, 2016

Choose a reason for hiding this comment

ghost commented Mar 4, 2016

cbodley commented Mar 4, 2016

cbodley commented Mar 4, 2016

cbodley commented Mar 4, 2016

ghost commented Mar 4, 2016

Choose a reason for hiding this comment

tchaikov commented Apr 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment