place debug option behind cmake switch buildtype=debug, instead of co… #758
Conversation
|
We used to set build type debug by default. The actual inclusion of debug flags under this type tripped the CI tests. It also makes the CI tests take much longer and cost more. I changed the build type to release. |
suvarchal
left a comment
suvarchal
left a comment
There was a problem hiding this comment.
G.Catch!!
TODO: Some what related to this there is some nesting and repetition of compilers and machine blocks for instance for the same compiler: gnu options on levante and on ubuntu differ some parts are machine specific and is needed but others should be common like link time optimization like -flto and it does make a difference as #775 showed runs in ci env and failed with gnu on levante. when organized better so all the machine specific can go into env files and rest in src/CMakeLists.txt so that we remove clutter.
There was a problem hiding this comment.
i wonder if we should just remove -DCMAKE_BUILD_TYPE=.... from ./configure.sh because 1. ./configure.sh also takes cmake args so gives flexibility for user to use ./configure.sh blahblah -DCMAKE_BUILD_TYPE=Debug if they want to. 2. in CMakeLists.txt build type is set to be Release in case the variable is not set.
| #) | ||
| ## if use -fsanitize=undefined,address you also need ... PRIVATE -lubsan) | ||
| #target_link_libraries(${PROJECT_NAME} PRIVATE -lubsan) | ||
| # debugging related compiler flags (activated for Debug build type) |
There was a problem hiding this comment.
we should use address sanitizer definitely for testing new code to check if we are allocating badly and creating leaks but I will skip -fsanitize for now as it need libasan to also be installed and present, this possibly need a cmake module to check this. so would skip this for now just, and ubsan seems to be a typo? should be -lasan or -static-libasan to compile options. More over any use of asan also need a env variable or someting to trace specific issue like ASAN_OPTIONS=detect_leaks=1 for detecting leaks. so this should be possibly done differently for it to be useful.
…mments