Improve CMakeLists documentation re: CMAKE_BUILD_TYPE

[palemieux]

@michaeldsmith It looks like CMAKE_CXX_FLAGS_ASAN is not used anywhere?

OpenJPH/CMakeLists.txt

Line 109 in 4beac80

set(CMAKE_CXX_FLAGS_ASAN

[michaeldsmith]

Lines 109-112 of CMakeLists.txt are:

set(CMAKE_CXX_FLAGS_ASAN
  "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1"
  CACHE STRING "Flags used by the C++ compiler during AddressSanitizer builds."
  FORCE)

Defining CMAKE_CXX_FLAGS_ASAN helps when running the Address Sanitizer (ASAN) that is part of GCC. The compiler uses the value of CMAKE_CXX_FLAGS_ASAN when cmake is run using the asan build type, i.e.
cmake .. -DCMAKE_BUILD_TYPE=asan

You can see the values of CMAKE_CXX_FLAGS_ASAN get used if you run cmake using the asan build type and then run make in verbose mode:
cmake .. -DCMAKE_BUILD_TYPE=asan
make VERBOSE=1

...
[  2%] Building CXX object src/core/CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o
cd /tmp/OpenJPH/build/src/core && /usr/bin/c++ -DOJPH_ALIGNED_ALLOC_EXISTS -D_FILE_OFFSET_BITS=64 -Dopenjph_EXPORTS -I/tmp/OpenJPH/src/core/openjph -fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1 -std=gnu++14 -fPIC -fexceptions -Wall -Wextra -Wconversion -Wunused-parameter -MD -MT src/core/CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o -MF CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o.d -o CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o -c /tmp/OpenJPH/src/core/codestream/ojph_codeblock.cpp
...

If instead, you run cmake with no build type specified and then make is run in verbose mode,
cmake ..
make VERBOSE=1

you can see the CMAKE_CXX_FLAGS_ASAN values that appear in the verbose make output above
-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1
are replaced with default values
-O3 -DNDEBUG

...
[  2%] Building CXX object src/core/CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o
cd /tmp/OpenJPH/build/src/core && /usr/bin/c++ -DOJPH_ALIGNED_ALLOC_EXISTS -D_FILE_OFFSET_BITS=64 -Dopenjph_EXPORTS -I/tmp/OpenJPH/src/core/openjph -O3 -DNDEBUG -std=gnu++14 -fPIC -fexceptions -Wall -Wextra -Wconversion -Wunused-parameter -MD -MT src/core/CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o -MF CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o.d -o CMakeFiles/openjph.dir/codestream/ojph_codeblock.cpp.o -c /tmp/OpenJPH/src/core/codestream/ojph_codeblock.cpp
...

[michaeldsmith]

CMAKE_CXX_FLAGS_ASAN is documented here

https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_CONFIG.html

[palemieux]

Ok. TIL.

The documentation should really be improved, e.g., by explicitly listing the build CONFIGS that are supported. Would be more useful than ## Added by Michael Smith :)

Also, why is the following needed?

if (NOT CMAKE_BUILD_TYPE)
  set(CMAKE_BUILD_TYPE "Release")

This stuff makes the makefile really hard to follow and debug IMHO -- having just spent quality time getting this to work with oss-fuzz.

[aous72]

I agree that it is better to have something instead of ## Added by Michael Smith :)

For

if (NOT CMAKE_BUILD_TYPE)
  set(CMAKE_BUILD_TYPE "Release")

I do not remember now, but at some point, I read somewhere that you must not leave CMAKE_BUILD_TYPE empty.
A quick search reveals that, without defining CMAKE_BUILD_TYPE, no optimization flags (-O2) and no debug flags (-g) are used.
The generated code is not useful for release and not useful for debug; therefore it is recommended to to have such a line.
See for example
https://www.kitware.com//cmake-and-the-default-build-type/

[aous72]

Apparently, this should be before project()

[palemieux]

If the objective is to make sure the user explicitly chooses a CMAKE_BUILD_TYPE, then I would exit if none is chosen, instead of choosing an unusual default.

[aous72]

See

# set a default build type if not set
if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
  message(STATUS "Setting build type to 'Release' as none was specified.")
  set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build." FORCE)
  # Set the possible values of build type for cmake-gui
  set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
    "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
endif()

in
https://github.com/AcademySoftwareFoundation/openexr/blob/main/cmake/OpenEXRSetup.cmake

[palemieux]

This type of logic increases the complexity of the makefile with no clear benefit for the user -- unless of course "none" is not a valid value in which case the build should simply fail.