widberg / bgfx.cmake

https://github.com/bkaradzic/bgfx.cmake. Independently maintained CMake build scripts for bgfx. Released under public domain.
https://github.com/bkaradzic/bgfx.cmake
Creative Commons Zero v1.0 Universal
286 stars 252 forks source link

Add DEBUG_POSTFIX to CMakeLists.txt files #75

Closed pr0g closed 4 years ago

pr0g commented 4 years ago

Hey there!

First off this repo is excellent and has made my life a lot easier getting setup with bgfx - thank you very much for creating it!

One suggestion I have would be to add the DEBUG_POSTFIX option supported by CMake to either the root CMakeLists.txt file or more granularly to each target in the .cmake files.

It can be set globally:

set(SDL_CMAKE_DEBUG_POSTFIX "d"
    CACHE STRING "Name suffix for debug builds")

or per target:

set_target_properties(
    ${PROJECT_NAME} PROPERTIES DEBUG_POSTFIX "d")

It's incredibly useful when installing libraries as they can be installed to the same folder without overwriting whichever version was there first (e.g. if you build Debug and install, and then build Release and install, the Release .lib/dll files will override the debug ones). You can work around this by creating two separate install locations but then it makes the life of the consuming library/application more difficult.

I'd be interested to hear what you think, I've written up a little more information about it here in the past: https://github.com/pr0g/cmake-examples#debug_postfix

Thanks!

Tom

widberg commented 4 years ago

I would happily accept a pr implementing this suggestion.

pr0g commented 4 years ago

Awesome! :) I'll get one ready later today :+1:

bwrsandman commented 4 years ago

It seems like this option already exists https://github.com/widberg/bgfx.cmake/blob/master/CMakeLists.txt#L33

Testing on Linux, the debug build has a d suffix:

Linking CXX static library libbgfxd.a

It's the BGFX_USE_DEBUG_SUFFIX option and it's on by default.

handsomematt commented 4 years ago

On my fork I simply removed BGFX_USE_DEBUG_SUFFIX https://github.com/widberg/bgfx.cmake/commit/43863b461dd2192c6bb507ab4a4284373b134349

And you just let the user specify CMAKE_DEBUG_POSTFIX - with this (and other commits in my fork I intend to upstream) we have a reusable install interface target that can be used with bgfx_DIR (debug and release): https://github.com/openblack/bgfx.cmake/releases/tag/latest

pezcode commented 4 years ago

On my fork I simply removed BGFX_USE_DEBUG_SUFFIX 43863b4

And you just let the user specify CMAKE_DEBUG_POSTFIX - with this (and other commits in my fork I intend to upstream) we have a reusable install interface target that can be used with bgfx_DIR (debug and release): https://github.com/openblack/bgfx.cmake/releases/tag/latest

Standard CMake options are always preferrable if they apply 👍

bwrsandman commented 4 years ago

As for these options, I would prefer them to be added off by default. We were using bgfx without the postfix and this change basically slipped in unnoticed.