CMake: export -DGDAL_DEBUG as PUBLIC for debug builds #11314
Conversation
rouault
added
backport release/3.10
|
Can confirm: Fix works 👍 |
gdal.cmake
Outdated
| @@ -196,6 +196,10 @@ if (MINGW AND BUILD_SHARED_LIBS) | |||
| set_target_properties(${GDAL_LIB_TARGET_NAME} PROPERTIES SUFFIX "-${GDAL_SOVERSION}${CMAKE_SHARED_LIBRARY_SUFFIX}") | |||
| endif () | |||
|
|
|||
| # To also export for external users what we did with add_compile_definitions($<$<CONFIG:DEBUG>:DEBUG>) | |||
| # in cmake/helpers/GdalCompilationFlags.cmake | |||
| target_compile_definitions(${GDAL_LIB_TARGET_NAME} PUBLIC $<$<CONFIG:DEBUG>:DEBUG>) | |||
There was a problem hiding this comment.
I'm not sure if this a good idea. At least as long as the macro is not prefixed with GDAL_ or similar.
There was a problem hiding this comment.
well, DEBUG is used in public GDAL include files, so ...
There was a problem hiding this comment.
This really needs a prefix...
There was a problem hiding this comment.
I think @dg0yt meant to say: DEBUG is such a generic name that you can't be sure that another library isn't using it or being influenced by it in some way. So setting it now could have an effect on other libraries. But if the DEBUG flag is set for the debug build in every case: Why not switch to NDEBUG, which is what the compilers set?
There was a problem hiding this comment.
On Linux, I can mix a debug build of my app with a release lib of GDAL...
There was a problem hiding this comment.
ok, I've revised the PR to export GDAL_DEBUG for BUILD_TYPE=DEBUG, and I've changed the tests in the public headers to test for "#if defined(DEBUG) || defined(GDAL_DEBUG)``. That should be backwards compatible.
There was a problem hiding this comment.
On Linux, I can mix a debug build of my app with a release lib of GDAL...
with the C API yes, but not if you use some parts of the C++ API
There was a problem hiding this comment.
On Linux, I can mix a debug build of my app with a release lib of GDAL...
ok, actually you can. I read the reverse. So... if you use a debug build of GDAL with a release lib of your code (or actually a debug one, that doesn't have -DDEBUG when including GDAL) and you use the GDAL C++ API, then you would run into the issue this PR fixes.
There was a problem hiding this comment.
What do you think about also renaming the macro within the cpp files and GdalCompilationFlags.cmake on this way DEBUG could be completely removed with GDAL 4.0.
There was a problem hiding this comment.
What do you think about also renaming the macro within the cpp files and
GdalCompilationFlags.cmakeon this wayDEBUGcould be completely removed with GDAL 4.0.
That could probably be done in GDAL 3.11. I didn't want to do the full rename in this PR, to keep it in a state where it is backportable to the 3.10 maintenance branch
rouault
changed the title
Fixes #11311
@SunBlack can you give this a try?