Kratos: Enhancement: add "-debug" to the version number when compiled in debug mode

[marandra]

I am just bringing back an idea proposed in the past (by @maceligueta I think) about adding some kind of "debug" label to the version number when compiled in debug mode. I think it will help save time, frustrations and problems.

[roigcarlo]

Good idea, but it has some implementation spikes that we have to discuss.

Compilers cannot know if something is compiled in debug or release mode. This needs to be done using some custom defines.

We have 3 of them:
NDEBUG (The standard)
KRATOS_DEBUG
DO_NOT_DEFINE_NDEBUG

My proposal would be to use NDEBUG as the "master switch".
Do we have to do the same with KRATOS_DEBUG and DO_NOT_DEFINE_NDEBUG

[pooyan-dadvand]

I'm not sure which should be the master switch.... I would like to have KRATOS_DEBUG which removes the NDEBUG and KRATOS_FAST_DEBUG which leaves the NDEBUG

But what @marandra is saying is different. The idea is to have the build type in the version number of Kratos, so one can see if the compiled binary is in debug or release (to see the performance problem of the user from log)

[roigcarlo]

Agree, that is my point too, my question was more in the lines of when to add that trailing. We cannot know If the code has been compiled with -g or -0O, to put some examples. If we want to do something like:

#define KRATOS_SHA1_NUMBER  3712594c8

#ifndef ????? (KRATOS_DEBUG?)
    #define KRATOS_VERSION  "5.0.0-3712594c8"
#else
    #define KRATOS_VERSION  "5.0.0-3712594c8-debug"
#endif 

Or maybe pass the -DCMAKE_BUILD_TYPE to the compiler and use that...

[jcotela]

Would something like this work? (not sure if needed to undefine)

#define KRATOS_VERSION_SUFFIX ""
#ifdef KRATOS_FAST_DEUBUG
  #undef KRATOS_VERSION_SUFFIX
  #define KRATOS_VERSION_SUFFIX "-fast-debug"
#endif
#ifdef KRATOS_DEBUG
  #undef KRATOS_VERSION_SUFFIX
  #define KRATOS_VERSION_SUFFIX "-debug"
#endif

[pooyan-dadvand]

👍 for @jcotela

[roigcarlo]

After looking at some options @jcotela and I have discover that we can add custom build types to CMake by adding:

# Custom compiler modes
SET( CMAKE_CXX_FLAGS_TESTDEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DKRATOS_BUILD_SUFFIX=TestDebug" CACHE STRING
    "Flags used by the C++ compiler during maintainer builds."
    FORCE )
SET( CMAKE_C_FLAGS_TESTDEBUG "${CMAKE_C_FLAGS_DEBUG} -IamATestFlag" CACHE STRING
    "Flags used by the C compiler during maintainer builds."
    FORCE )

And then:

-DCMAKE_BUILD_TYPE=TestDebug 

*Noteice the -DKRATOS_BUILD_SUFFIX=TestDebug That will be the suffix in the git number

Which solves pretty much everything.
We only have to decide which levels of debug we want and what to enable in every level:

A proposal:

• Release ( -O3 )
• RelWithDebInfo ( -g -O2 )
• FastDebug ( -g -O2 -DKRATOS_DEBUG )
• Debug ( -g -O0 -DKRATOS_DEBUG )
• FullDebug ( -g -O0 -DKRATOS_DEBUG -_DNDEBUG )

[maceligueta]

In linux, it would be great if we can add the following compilation flag -D_GLIBCXX_DEBUG, which activates many verifications of the STL libraries. At least for the FullDebug option.

[RiccardoRossi]

+1 for @maceligueta on FullDebug

i would simplify options to Debug (maybe doing the same as FastDebug) and FullDebug

btw, does -_DNDEBUG remove the NDEBUG definition??

[jcotela]

@RiccardoRossi in full debug we pass whatever flag puts the full set of checks. You'll have to give us the specific name :P

[jcotela]

I uploaded a first test of @roigcarlo's suggestion to branch feature-new-build-modes (minimally tested, not ready for production).
I'm setting it to the following:

RELEASE:        -O3 -DNDEBUG
RELWITHDEBINFO: -O2 -g -DNDEBUG -DKRATOS_BUILD_SUFFIX=-RelWithDebInfo
FASTDEBUG:      -g -DNDEBUG -DKRATOS_DEBUG -DKRATOS_BUILD_SUFFIX=-FastDebug
DEBUG:          -g -DNDEBUG -DKRATOS_DEBUG -DKRATOS_BUILD_SUFFIX=-Debug
FULLDEBUG:      -g -DKRATOS_DEBUG -DKRATOS_BUILD_SUFFIX=-FullDebug

However, the current implementation has some issues (most importantly, flags are re-added the next time the configure script is added). I'm also not adding @maceligueta's suggested flag yet.

Some specific questions:

• Do we want FAST_DEBUG to define KRATOS_DEBUG (right now there is no difference with debug)?
• @roigcarlo: I didn't set the -O2, -O0 options (the ones there are CMake defaults). If I add them, are they compiler-independent?

[pooyan-dadvand]

@jcotela as you are mentioning the FAST_DEBUG and DEBUG are the same. I would go for DEBUG and FULL_DEBUG and leave the fast debug for when we have some difference.

[maceligueta]

Talking about flags and options for the different compilations, I would like to link this to the Isse #369.
Maybe we can remove -ffast-math from the Debug compilations.

[pooyan-dadvand]

👍

[jcotela]

@maceligueta If we want to include/not include the -ffast-math flag from the different compilation options using the method described here, the way to go would be removing it from the configure.sh and add it to the RELEASE and RELWITHDEBINFO modes. However, this would never overwrite variables passed through configure.sh, so if you define it manually, it will remain there for all build modes (and I think this would be a good thing)

[pooyan-dadvand]

I don't think that having the fast-math in configure is a good idea. (actually IMO configure should be minimal) I would add it to CMake modes.

[jcotela]

I'm pasting this here for future reference: How to define build types that MSVC understands:
https://stackoverflow.com/questions/24460486/cmake-build-type-not-being-used-in-cmakelists-txt

[pooyan-dadvand]

@jcotela thanks for the link. Continuing the discussion, do you keep both fast debug and debug even if they are the same thing at the moment?

[jcotela]

@pooyan-dadvand I still have the two settings in the branch, but I agree that there is no point. Maybe we can remove the FastDebug mode for now and add it later if we see a use case.

By the way, if you check the branch you'll see that there is already a technical solution to the original issue of adding the branch suffixes in the version number, while the flag definition is turning out to be a bit more complicated (I'm a bit uncertain about which flags to set on each compiler/how to let Visual Studio know about the new configuration modes). I think I'll make a simplified PR with only the version number thing while we keep working on the new build modes.

[RiccardoRossi]

hi guys,

take a look to #715

i think there is a potential problem with the GLIBCXX_DEBUG (all the external dependencies need to be compiled with the flag otherwise it segfaults)

[pooyan-dadvand]

@jcotela I agree with removing the fast debug.

@RiccardoRossi we should pack the release in release mode only, aren't we?

[jcotela]

This feature is finally implemented (and merged in PR #722), so I'm closing the issue. Feel free to reopen if there are questions.

[Glovedsasquatchcodes]

@roigcarlo ,
As mentioned in one of the answers above, @jcotela , I tried to understand what should be done to get the debug version libs using CMakeLists.txt (the link used was the following: https://stackoverflow.com/questions/24460486/cmake-build-type-not-being-used-in-cmakelists-txt).

I am working on Ubuntu OS and wanted to know how to generate the Debug libs for Kratos. What changes should be made to the CMakeLists.txt files (and elsewhere, if needed) to be able to do this successfully?

[jcotela]

@Glovedsasquatchcodes to compile a debug version of kratos using the mechanism described in this issue, add

-DCMAKE_BUILD_TYPE=FullDebug  \

to your cmake options in your configure.sh file (not CMakeLists.txt) and configure again. Debug will also work (it has less runtime checks, so it should run a bit faster but be less informative in debugging).