Fix for handling of Bullet optimized vs debug libraries in FindMagnumPlugins.cmake

[Squareys]

Hi @mosra !

Here's a small fix for how optimized/debug bullet libraries are handled in FindMagnumPlugins.

From the commit message:

When both debug and release builds of bullet libraries are found, ${BULLET_LIBRARIES} contains "optimized" and "debug" keywords which are not handles by set_property. Using target_link_libraries instead lets the find module appropriately choose whether to use the debug or optimized libraries.

Regards, Jonathan.

[mosra]

Oh, wow, this is allowed? :D Every other target_*() command says

The named <target> must have been created by a command such as add_executable() or add_library() and must not be an IMPORTED target.

... but not this one, apparently. Weird inconsistency. Thanks a lot, will merge later!

[Squareys]

Oh, wow, this is allowed? :D Every other target_*() command says

No, it doesn't :( I guess it was some cmake cache thing that made this work for me temporarily. (sorry!)

I think the way to do this would be to specify imported targets for bullet and dependencies, therefore writing a new FindBullet.cmake instead of using the one provided by CMake?

[mosra]

@Squareys can you try this instead? No custom Find module for Bullet should be needed.

diff --git a/modules/FindMagnumIntegration.cmake b/modules/FindMagnumIntegration.cmake
index 57bcc4b..627764d 100644
--- a/modules/FindMagnumIntegration.cmake
+++ b/modules/FindMagnumIntegration.cmake
@@ -160,8 +160,18 @@ foreach(_component ${MagnumIntegration_FIND_COMPONENTS})
             find_package(Bullet)
             set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
                 INTERFACE_INCLUDE_DIRECTORIES ${BULLET_INCLUDE_DIRS})
-            set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
-                INTERFACE_LINK_LIBRARIES ${BULLET_LIBRARIES})
+
+            # Need to handle special cases where both debug and release
+            # libraries are available (in form of debug;A;optimized;B in
+            # BULLET_LIBRARIES), thus appending them one by one
+            foreach(lib BULLET_DYNAMICS_LIBRARY BULLET_COLLISION_LIBRARY BULLET_MATH_LIBRARY BULLET_SOFTBODY_LIBRARY)
+                set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
+                    INTERFACE_LINK_LIBRARIES ${${lib}})
+                if(${lib}_DEBUG)
+                    set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES $<$<CONFIG:Debug>:${${lib}_DEBUG}>)
+                endif()
+            endforeach()

             set(_MAGNUMINTEGRATION_${_COMPONENT}_INCLUDE_PATH_NAMES MotionState.h)

[Squareys]

@mosra That links both release and debug libraries (in this order, which results in only the release version being used, since symbols are resolved there first). I am guessing you need CONFIG:Release aswell. I will try.

[Squareys]

@mosra There you go, this branch now only contains your diff as a commit with that minor fix.

[mosra]

Hmm, I fear that would not work in case you have only Debug or Release. Can you try this?

diff --git a/modules/FindMagnumIntegration.cmake b/modules/FindMagnumIntegration.cmake
index 57bcc4b..8aea239 100644
--- a/modules/FindMagnumIntegration.cmake
+++ b/modules/FindMagnumIntegration.cmake
@@ -160,8 +160,19 @@ foreach(_component ${MagnumIntegration_FIND_COMPONENTS})
             find_package(Bullet)
             set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
                 INTERFACE_INCLUDE_DIRECTORIES ${BULLET_INCLUDE_DIRS})
-            set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
-                INTERFACE_LINK_LIBRARIES ${BULLET_LIBRARIES})
+
+            # Need to handle special cases where both debug and release
+            # libraries are available (in form of debug;A;optimized;B in
+            # BULLET_LIBRARIES), thus appending them one by one
+            foreach(lib BULLET_DYNAMICS_LIBRARY BULLET_COLLISION_LIBRARY BULLET_MATH_LIBRARY BULLET_SOFTBODY_LIBRARY)
+                if(${lib}_DEBUG)
+                    set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES "$<$<CONFIG:Release>:${${lib}}>;$<$<CONFIG:Debug>:${${lib}_DEBUG}>")
+                else()
+                    set_property(TARGET MagnumIntegration::${_component} APPEND PROPERTY
+                        INTERFACE_LINK_LIBRARIES ${${lib}})
+                endif()
+            endforeach()

             set(_MAGNUMINTEGRATION_${_COMPONENT}_INCLUDE_PATH_NAMES MotionState.h)

[Squareys]

@mosra Alright. I applied your patch and it works at least so far that the original error does not appear anymore.

There is still a link error, which makes BulletIntegration unusable with MSVC2015, though. I posted in the public magnum gitter.

[mosra]

I actually used this patch for a different package (Protobuf) and can confirm that it works. Though later I had to replace $<CONFIG:Release> with $<NOT:$<Config:Debug>> so it works even in case you have CMAKE_BUILD_TYPE set to something else than Debug or Release (e.g. empty string, which for some reason is still CMake default).

[Squareys]

It's $<NOT:$<CONFIG:Debug>>, the case seems to matter here for some reason. Glad I compile checked this, even though I was almost going to skip it :sweat_smile:

[Squareys]

@mosra There you go, should finally be done :)

(But let me quickly fix the author on that one)

[Squareys]

You may merge "your" commit, hehe ;)

[mosra]

@Squareys argh, yeah, of course, you're right. My brain was off at the time ;)

[mosra]

Commited in a481dc3fe123ac791e1fe690711b6196d6e97db3. Thanks for the testing!