#583 CMake legacy compatibility issue with unconditional setting of -NDEBUG

[Oliver Schneider]

In the main CMakeLists.txt the CMAKE_CXX_FLAGS value gets unconditionally appended a -DNDEBUG.

I think this is wrong, because it disregards the multi-config CMake generators such as Ninja Multi-Config and practically all Visual Studio generators. The outer check for CMAKE_BUILD_TYPE falls through, because only single-config generators set the boolean.

diff --git c/CMakeLists.txt w/CMakeLists.txt
index e0064ea90..dafe68160 100644
--- c/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -82,15 +82,18 @@ if(GDCM_VERSION_MINOR MATCHES "[02468]$")
   # Are we building a release branch / tag (read: even number)?
   # By default dashboard are expected to run with Design by Contract on
   # to trigger any of the assert, but on the other hand no user really
   # can figure out they need to change this value
   # So unless the user *specifically* requested a particular cmake_build_type
   # do the work internally and append the NDEBUG def flag (hopefully portable)
   if(NOT CMAKE_BUILD_TYPE)

-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNDEBUG")
+    get_cmake_property(is_multi_config_generator GENERATOR_IS_MULTI_CONFIG)
+    if (NOT is_multi_config_generator)
+      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNDEBUG")
+    endif()
   endif()
   # Since we are on a release branch, chance is that people don't care about testing
   # let's disable it for them
   set(GDCM_BUILD_TESTING_DEFAULT OFF)
 else()
   set(GDCM_BUILD_TESTING_DEFAULT ON)
 endif()
@@ -114,47 +117,51 @@ endif()

 if(BUILD_SHARED_LIBS)
   set(NAMELINK_ONLY NAMELINK_ONLY)
   set(NAMELINK_SKIP NAMELINK_SKIP)
 endif()

Technically it'd also be better to use add_compile_definitions, but that got added in CMake 3.12.

Overall it seems the CMake infastructure should be overhauled and legacy support curtailed to the 3.x version range, perhaps even above 3.0!

[Oliver Schneider]

There's another section catering legacy CMake just below:

 #-----------------------------------------------------------------------------
+#[[
 # Compatibility with older usage of EXECUTABLE_OUTPUT_PATH and LIBRARY_OUTPUT_PATH.
 # This should be removed in the future.
 if(EXECUTABLE_OUTPUT_PATH)
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${EXECUTABLE_OUTPUT_PATH})
   message(WARNING "EXECUTABLE_OUTPUT_PATH is deprecated. Use CMAKE_RUNTIME_OUTPUT_DIRECTORY instead.")
 endif()
 if(LIBRARY_OUTPUT_PATH)
   set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBRARY_OUTPUT_PATH})
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBRARY_OUTPUT_PATH})
   message(WARNING "LIBRARY_OUTPUT_PATH is deprecated. Use CMAKE_LIBRARY_OUTPUT_DIRECTORY and CMAKE_ARCHIVE_OUTPUT_DIRECTORY instead.")
 endif()
+]]

 if(NOT CMAKE_RUNTIME_OUTPUT_DIRECTORY)
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/bin)
 endif()
 if(NOT CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/bin)
 endif()
 if(NOT CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/bin)
 endif()

+#[[
 # Set for legacy internal usage of EXECUTABLE_OUTPUT_PATH and LIBRARY_OUTPUT_PATH
 if (NOT EXECUTABLE_OUTPUT_PATH)
   set(EXECUTABLE_OUTPUT_PATH ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
 endif()
 if (NOT LIBRARY_OUTPUT_PATH)
   if (BUILD_SHARED_LIBS)
     set(LIBRARY_OUTPUT_PATH ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
   else()
     set(LIBRARY_OUTPUT_PATH ${CMAKE_ARCHIVE_OUTPUT_DIRECTORY})
   endif()
 endif()
+]]

 #-----------------------------------------------------------------------------

(this is the diff to the commented out version we're using)

[Oliver Schneider]

https://github.com/malaterre/GDCM/pull/218