New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: support for external rocksdb #12467

Merged
merged 1 commit into from Jan 17, 2017

Conversation

Projects
None yet
4 participants
@bassam
Member

bassam commented Dec 13, 2016

add support for building with an external rocksdb. Some distros are now starting to support rocksdb (ubuntu for example http://packages.ubuntu.com/yakkety/librocksdb-dev). For Rook scenarios we'd like to build all ceph's external dependencies separately (in a build container). It also simplifies the build process as we don't have to passthrough toolchains etc. the ExternalProject_add.

The default is still to build from the submodule.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Dec 13, 2016

@bassam
In another PR (about embedded) you already introduced merge_libs and merge_static_libraries, but I seem to remember that function did not really work on FreeBSD.
Is it something you wrote yourself, or is it part of the cmake distribution.
Espec in the last case, I have to start looking at the Cmake source and perhaps fix it there.

@bassam

This comment has been minimized.

Member

bassam commented Dec 13, 2016

@wjwithagen which part of merge_static_libraries is broken? It should use standard ar to merge archives. its not part of the standard cmake distro. let me know if you'd like me to look at it.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Dec 13, 2016

@bassam
Eh, I'd have to switch embedded building on again and see what it yelled at me.
So it's a receipe that you build/borrowed, so lets not bother in this thread then because this is about RocksDB. I'll ping you when it bites me again.

@liewegas liewegas added the build/ops label Dec 14, 2016

@bassam

This comment has been minimized.

Member

bassam commented Jan 11, 2017

@tchaikov does this seem reasonable to you?

@@ -0,0 +1,18 @@
# Find the native Rocksdb includes and library
# This module defines
# ROCKSDB_INCLUDE_DIRS, where to find Rocksdb.h, Set when

This comment has been minimized.

@tchaikov

tchaikov Jan 12, 2017

Contributor

s/ROCKSDB_INCLUDE_DIRS/ROCKSDB_INCLUDE_DIR/

s/Rocksdb.h/rocksdb/db.h/

This comment has been minimized.

@bassam

bassam Jan 12, 2017

Member

fixed.

add_library(rocksdb STATIC IMPORTED)
add_dependencies(rocksdb rocksdb_ext)
set_property(TARGET rocksdb PROPERTY IMPORTED_LOCATION "${CMAKE_CURRENT_BINARY_DIR}/rocksdb/librocksdblib.a")

This comment has been minimized.

@tchaikov

tchaikov Jan 12, 2017

Contributor

wondering if we should also

set(ROCKSDB_LIBRARIES rocksdb)

here and use ${ROCKSDB_LIBRARIES} instead of rocksdb.

This comment has been minimized.

@bassam

bassam Jan 12, 2017

Member

good idea. fixed.

@bassam

This comment has been minimized.

Member

bassam commented Jan 16, 2017

@tchaikov rebased on master to resolve conflict. also the last jenkins build ran out of disk space. hopefully all good now.

cmake: support for external rocksdb
add support for building with an external rocksdb.

Signed-off-by: Bassam Tabbara <bassam.tabbara@quantum.com>

@tchaikov tchaikov self-assigned this Jan 17, 2017

@tchaikov tchaikov merged commit 88ed82f into ceph:master Jan 17, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@bassam bassam deleted the bassam:pr-external-rocksdb branch Jan 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment