Question: setting CMAKE_BUILD_TYPE

[mgruhler]

This is a question about style, rather than about right or wrong

What we do sometimes in our packages is to provide a default build type: something along the lines of

if(NOT CMAKE_BUILD_TYPE)
  set(CMAKE_BUILD_TYPE Release)
endif()

This still allows for setting the build type from the command line and has been discussed in various threads on the CMake Mailing Lists and in other places as a viable solution. Also, it is not forbidden by the CMake Coding Standard on the catkin docs (http://docs.ros.org/indigo/api/catkin/html/user_guide/standards.html).
Granted, this is merely for ease-of-use and might be error-prone, if not done carefully.

catkin_lint reports this as an error and thus stops the build, if integrated in the build process.

What about setting this to a warning? Or do you feel the above is bad style and should not under any circumstances be done?

(Love this tool, btw. Thanks!)

[roehling]

In my opinion, CMAKE_BUILD_TYPE should be treated like CMAKE_CXX_FLAGS, i.e. it is a matter of policy and/or the build architecture requirements, which may be different for different users and thus should not be imposed by the package maintainer.

I agree that an if() guarded set(CMAKE_BUILD_TYPE) is rather well-behaved and should be a warning or notice rather than an error; unfortunately catkin_lint cannot (yet) evaluate if() statements sufficiently well to distinguish the "kind-of-okay" construct from the "no-no-no-never-do-this" one.

Personally, I prefer to set all these variables in the src/CMakeLists.txt file (where you integrate catkin_lint into the build). This makes the variables "workspace-global" without cluttering your packages. For reference, my current src/CMakeLists.txt looks like this:

cmake_minimum_required(VERSION 2.8.3)

find_program(CATKIN_LINT catkin_lint)
if(CATKIN_LINT)
    execute_process(COMMAND "${CATKIN_LINT}" "${CMAKE_SOURCE_DIR}" RESULT_VARIABLE lint_result)
    if(NOT ${lint_result} EQUAL 0)
        message(FATAL_ERROR "catkin_lint failed")
    endif()
endif()

set(CMAKE_CXX_FLAGS_DEVEL "-Wall -Wextra -Wno-ignored-qualifiers -Wno-invalid-offsetof -Wno-unused-parameter -O3 -g" CACHE STRING "Devel build type CXX flags")
set(CMAKE_C_FLAGS_DEVEL "-Wall -Wextra -Wno-unused-parameter -O3 -g" CACHE STRING "Devel build type C flags")
set(CMAKE_SHARED_LINKER_FLAGS_DEVEL "-Wl,-z,defs" CACHE STRING "Devel build type shared library linker flags")
set(CMAKE_EXE_LINKER_FLAGS_DEVEL "-Wl,-z,defs" CACHE STRING "Devel build type executable linker flags")

if(NOT CMAKE_BUILD_TYPE)
    message(STATUS "Using default CMAKE_BUILD_TYPE=Devel")
    set(CMAKE_BUILD_TYPE Devel)
endif()

include(toplevel.cmake)

As you can see, I invented my own build type "Devel" which uses optimization but still adds debug symbols. I set up the warnings according to my personal preferences and made sure the compiler complains about missing symbols even when linking shared libraries (which GCC does not by default).

[mgruhler]

Thanks for the fast feedback.

Actually, I thought that the answer would be along the line of this. Especially knowing that catkin_lint cannot check if() statements the way it would be required. So treating CMAKE_BUILD_TYPE differently than it currently is, would relax the current policy and thus possibly introduce very bad/unintended behaviour.
Your rationale for treating CMAKE_BUILD_TYPE like CMAKE_CXX_FLAGS is, IMHO, justified and fine for me.
Thus, I'll close this.

Also, I really like the idea of setting these variables in src/CMakeLists.txt. This actually never occured to me ;-) I'll probably do that for myself following your idea.