Bug #58279 Incorrect enabling of UNIV_DEBUG in debug builds
Submitted: 18 Nov 2010 9:44 Modified: 12 Jan 2011 21:38
Reporter: John Embretsen Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.5.6-m3-Celosia OS:Any
Assigned to: Vasil Dimov CPU Architecture:Any
Tags: cmake

[18 Nov 2010 9:44] John Embretsen
Description:
In June the following change was implemented in mysql-trunk-innodb, with the purpose of enabling UNIV_DEBUG on all debug platforms (in Pushbuild):

=== modified file 'storage/innobase/CMakeLists.txt'
--- storage/innobase/CMakeLists.txt     2010-05-07 20:37:34 +0000
+++ storage/innobase/CMakeLists.txt     2010-06-23 10:02:49 +0000
@@ -40,6 +40,10 @@
   ENDIF()
 ENDIF()
 
+# Enable InnoDB's UNIV_DEBUG if MySQL's WITH_DEBUG[_FULL] is defined
+IF(WITH_DEBUG OR WITH_DEBUG_FULL)
+  ADD_DEFINITIONS("-DUNIV_DEBUG")
+ENDIF()
 
 IF(NOT MSVC)
 # either define HAVE_IB_GCC_ATOMIC_BUILTINS or not

=== (End of diff)

Commit comment:

revno: 2995.25.27
revision-id: vasil.dimov@oracle.com-20100623100249-e4bbn3gf1py6260h
parent: vasil.dimov@oracle.com-20100623071721-l8mend26quaybmwb
committer: Vasil Dimov <vasil.dimov@oracle.com>
branch nick: mysql-trunk-innodb
timestamp: Wed 2010-06-23 13:02:49 +0300
message:
  Enable InnoDB's UNIV_DEBUG if MySQL's WITH_DEBUG[_FULL] is defined
                
  This will make PB2 test InnoDB with UNIV_DEBUG on its *_debug platforms
                  
  Discussed with:       Marko (on IRC)

=== (End of commit comment)

This was merged to 5.5 and trunk some weeks later.

There is an issue with this implementation, and that is that WITH_DEBUG is not a CMake option that makes sense in multi-configuration environments, most notably Visual Studio and XCode. See Bug#58231 for a discussion of why this way of setting build flags for debug is not recommended, and why it also makes Unix (Makefile)  builds more complex and confusing than necessary.

Potential impact is for example that UNIV_DEBUG is by default not enabled in Windows debug builds, although it was intended to be.

See also http://forge.mysql.com/wiki/CMake#Debug-only_options

How to repeat:
For example, on Unix, one expects to get a debug build by specifying a "Debug" build type for cmake:

mkdir bld-debug
cd bld-debug
cmake -DCMAKE_BUILD_TYPE=Debug ..

[check if UNIV_DEBUG is enabled]

Due to the way things like UNIV_DEBUG are included, one has to specify -DWITH_DEBUG=1 to get a "proper" debug build with all intended inclusions, making the CMAKE_BUILD_TYPE setting redundant. This was not the intention.

cmake -DWITH_DEBUG ..

On Windows and Mac, neither CMAKE_BUILD_TYPE nor WITH_DEBUG make much sense. Instead one specifies build type at compile time, and for all debug cruft to be included properly one has to use CMAKE_{C,CXX}_FLAGS_DEBUG in the CMakeLists.txt files.

Suggested fix:
Quoting from Bug#58231:

Using IF(WITH_DEBUG) or IF(CMAKE_BUILD_TYPE MATCHES "Debug") to set compile options in CMakeLists.txt is harmful and does not work. 

IF(CMAKE_BUILD_TYPE MATCHES "Debug") might work but not on Windows with Visual Studio and not on OSX  with Xcode. It works only with "*Makefile" generators.

One can/should/must always use CMAKE_{C,CXX}_FLAGS_DEBUG to set debug-only options. Violations to this rule, should be fixed.

---

In CMakeLists.txt, use this instead:

SET(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}  -DUNIV_DEBUG")
and if necessary:
SET(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}  -DUNIV_DEBUG")
[19 Nov 2010 13:33] Vasil Dimov
After reading all posts here and on the related Bug#58231 the solution seems pretty obvious to me, I am going to commit this patch to mysql-5.5-innodb:

--- cut ---
diff --git i/storage/innobase/CMakeLists.txt w/storage/innobase/CMakeLists.txt
index 587e39d..f0bfd74 100644
--- i/storage/innobase/CMakeLists.txt
+++ w/storage/innobase/CMakeLists.txt
@@ -40,10 +40,9 @@ IF(UNIX)
   ENDIF()
 ENDIF()
 
-# Enable InnoDB's UNIV_DEBUG if MySQL's WITH_DEBUG is defined
-IF(WITH_DEBUG)
-  ADD_DEFINITIONS("-DUNIV_DEBUG")
-ENDIF()
+# Enable InnoDB's UNIV_DEBUG for debug builds
+SET(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -DUNIV_DEBUG")
+SET(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DUNIV_DEBUG")
 
 IF(NOT MSVC)
 # either define HAVE_IB_GCC_ATOMIC_BUILTINS or not
--- cut ---

Thanks for the thorough explanation!
[22 Dec 2010 21:31] Bugs System
Pushed into mysql-trunk 5.6.1 (revid:alexander.nozdrin@oracle.com-20101222212842-y0t3ibtd32wd9qaw) (version source revid:alexander.nozdrin@oracle.com-20101222212842-y0t3ibtd32wd9qaw) (merge vers: 5.6.1) (pib:24)
[8 Jan 2011 15:10] Bugs System
Pushed into mysql-5.5 5.5.9 (revid:vasil.dimov@oracle.com-20110108150508-gpanhz48z8069qot) (version source revid:vasil.dimov@oracle.com-20110108150048-b1y9m8xe72hay0ch) (merge vers: 5.5.9) (pib:24)
[12 Jan 2011 21:38] John Russell
Adding to change log:

The command to create a debug build (cmake -DWITH_DEBUG ...) now
automatically sets the InnoDB debugging flag UNIV_DEBUG on all
platforms. Formerly, the UNIV_DEBUG flag might not be set for Windows
platforms with Visual Studio and not on OS X with Xcode.
[13 Jan 2011 0:19] Vladislav Vaintroub
@John(Russel)

CMAKE_BUILD_TYPE=Debug is specific to Makefiles. We(and CMake) support non-Makefiles as well, and CMAKE_BUILD_TYPE does not have any effect here. Here is why.

If you have something that is not makefiles (Visual Studio projects or Xcode projects), then there are different mechanism to enable debug build at the *build* time, rather than at the configure (cmake) time. 

Put it simpler, with VS (and Xcode) you run "cmake" once and get a project that has debug and release configurations, and you can click in GUI or provide command line parameter for devenv/xcodebuild to compile either for debug or release. 

With makefiles, you cannot do that. You need to run cmake once for release builds, and again (in a different build directory) for debug builds. You cannot tweak build type at  the build time (i.e there is no such thing as "make DEBUG=yes")

What that means in context of the bug, is that CMAKE_C{XX}_FLAGS_DEBUG is only portable way to define debug-only flags in CMake, as this works for both Makefiles and non-Makefiles.
[13 Jan 2011 7:40] Vasil Dimov
Just remove "cmake -DWITH_DEBUG ...)" from

"The command to create a debug build (cmake -DWITH_DEBUG ...) now
automatically sets the InnoDB debugging flag UNIV_DEBUG on all
platforms. Formerly, the UNIV_DEBUG flag might not be set for Windows
platforms with Visual Studio and not on OS X with Xcode."