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: in case of bad "ALLOCATOR" selected issue warning #17422
Conversation
Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
src/CMakeLists.txt
Outdated
@@ -295,6 +295,9 @@ elseif(ALLOCATOR STREQUAL "jemalloc") | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
elseif(ALLOCATOR STREQUAL "libc") | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
else() | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
message(WARNING "Possibly incorrect allocator selected:" ${ALLOCATOR}) |
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.
Can we raise an error here?
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.
yes, we should error out here with an error message.
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.
Please don't.
FreeBSD has jemalloc as default allocator.
Defining it however, will show that the additional tools are not available in ways that are expected.
So for example rocksdb starts doing wrong things.
If you want to error out, then please make an exemption for FreeBSD.
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.
it's always set in $top_srcdir/CMakeLists.txt
.
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.
You are right. So FreeBSD ends in libc.
Why not error out earlier, like in $top_srcdir/CMakeLists.txt
?
@aclamk ping? |
src/CMakeLists.txt
Outdated
@@ -295,6 +295,9 @@ elseif(ALLOCATOR STREQUAL "jemalloc") | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
elseif(ALLOCATOR STREQUAL "libc") | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
else() | |||
set(TCMALLOC_srcs perfglue/disabled_heap_profiler.cc) | |||
message(FATAL_ERROR "Possibly incorrect allocator selected:" ${ALLOCATOR}) |
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, add a space after :
. BTW, could put the variable right into the string to print, also could use a stronger tone here. like
message(FATAL_ERROR "Unsupported allocator selected: ${ALLOCATOR}")
Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
d4b31a2
to
4831010
Compare
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.
as long as shaman is happy
Signed-off-by: Adam Kupczyk akupczyk@redhat.com