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
Pass compiler and flags to compile RocksDB into build #10418
Conversation
set(EXTRA_CXXFLAGS -fPIC -Wno-unused-variable -DNDEBUG) | ||
if(FREEBSD) | ||
set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Irocksdb/include -I/usr/local/include) | ||
set(MAKEFLAGS ${MAKEFLAGS}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tchaikov
It is possible to run the buidl with make V=1 VERBOSE=1
to get e more verbose output of what is going on.
And the build in RocksDB supports that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean, you set MAKEFLAGS
to MAKEFLAGS
. this is a no-op.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tchaikov
Right.... Turns out that that whole block is bogus. It stems back from the time that I also had rocksdb installed as package. And then includefiles started to be mixed. And linking was a mess as well.
Looks like things have improved on all ends.
So the only things really required to be set are NDEBUG and CXX.
I'll fix.
4a82607
to
4160c47
Compare
4160c47
to
801b63f
Compare
@@ -637,9 +637,23 @@ install(TARGETS ceph-mon DESTINATION bin) | |||
if(NOT ALLOCATOR STREQUAL "jemalloc") | |||
set(disable_jemalloc "DISABLE_JEMALLOC=1") | |||
endif() | |||
|
|||
set(ROCKSDB_EXTRA_CXXFLAG -fPIC -Wno-unused-variable) | |||
if(NOT CMAKE_BUILD_TYPE MATCHES "Debug") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, could be more consistent with existing way to check CMAKE_BUILD_TYPE
at line:30
if(NOT CMAKE_BUILD_TYPE STREQUAL Debug)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tchaikov
"Fun" part is that I've already have line 30 with "matches" as well, in another WIP.
That got deleted when we removed the debug options from the Clang commit.
So I actually copied it from there. :)
But I'll make it consistent.
imo, the changes are not specific to FreeBSD or clang, could you revise your commit message and its title? |
1968281
to
2ea097b
Compare
set(ROCKSDB_EXTRA_CXXFLAG -fPIC -Wno-unused-variable) | ||
if(NOT CMAKE_BUILD_TYPE STREQUAL Debug) | ||
set(ROCKSDB_EXTRA_CXXFLAG ${ROCKSDB_EXTRA_CXXFLAG} -DNDEBUG) | ||
endif(NOT CMAKE_BUILD_TYPE MATCHES "Debug") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does not match with if
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 27-7-2016 10:27, Kefu Chai wrote:
In src/CMakeLists.txt
#10418 (comment):@@ -637,9 +637,15 @@ install(TARGETS ceph-mon DESTINATION bin)
if(NOT ALLOCATOR STREQUAL "jemalloc")
set(disable_jemalloc "DISABLE_JEMALLOC=1")
endif()
+
+set(ROCKSDB_EXTRA_CXXFLAG -fPIC -Wno-unused-variable)
+if(NOT CMAKE_BUILD_TYPE STREQUAL Debug)
- set(ROCKSDB_EXTRA_CXXFLAG ${ROCKSDB_EXTRA_CXXFLAG} -DNDEBUG)
+endif(NOT CMAKE_BUILD_TYPE MATCHES "Debug")does not match with |if|.
Aaarrggh... Sorry about that.
I've got this root-canal infection, with splitting headache, for about 2
weeks now. And it is definitly impacting my concentration.
Will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries. take care.
…sDB build - make sure the right compiler is used in CXX - NDEBUG if the current code is not being build with debugging Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
2ea097b
to
80fcbb6
Compare
lgtm once the jenkins returns. |
@wjwithagen i will check #10417 again once you address all the comments to it. |
@tchaikov |
Signed-off-by: Willem Jan Withagen wjw@digiware.nl