Fix building OGRE / OGRE2 from source in colcon workspace

[iche033]

Signed-off-by: Ian Chen [email protected]

🦟 Bug fix

Summary

Problem: When building ogre2 in a colcon workspace, we ran into an error about not finding an ogre 1.x component. The problem is the cmake module for ogre 1.x is looking into the ogre2 source directory in the colcon workspace instead of the system installed one.

I also had to fix the values returned by the OGRE_LIBRARIES variable. It needs to be the full path to the library. We are actually doing something similar ogre2 cmake module and also in the logic for ogre 1.x on windows. Not sure why the issue only surfaced in the colcon workspace setup mentioned above.

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[iche033]

@darksylinc can you give this a try?

[adlarkin]

I tested this with ign-rendering6 (Fortress). When using the ign-cmake2 branch, I saw the following build warning:

CMake Warning at /ogre_source_ws/install/share/cmake/ignition-cmake2/cmake2/IgnConfigureBuild.cmake:53 (message):
   CONFIGURATION WARNINGS:
   -- Skipping component [ogre]: Missing dependency [IgnOGRE] (Components: RTShaderSystem, Terrain, Overlay).
      ^~~~~ Set SKIP_ogre=true in cmake to suppress this warning.

When I used the branch in this PR, the warning disappeared, so I believe that this resolves the issue for me.

---

On a related note, I was looking over the FindIgnOGRE.cmake file, and believe there may be an issue with the following code block: https://github.com/ignitionrobotics/ign-cmake/blob/f559887e98a2ced7bb89606cb3cfcba4be098ff5/cmake/FindIgnOGRE.cmake#L67-L74

I believe that line 72 should be the following instead (notice how the quotes are removed from _pkgconfig_invoke_result so that we can get the value of this variable):

elseif (NOT _pkgconfig_invoke_result STREQUAL "")

Is this a correct assumption? If so, should we make this fix in this PR as well?

[iche033]

I believe that line 72 should be the following instead (notice how the quotes are removed from _pkgconfig_invoke_result so that we can get the value of this variable):

good catch. I've removed the quotes. 3969bcc

[adlarkin]

I'm approving this since it appears to resolve the build warnings for me, but maybe we can get confirmation from at least one other person before merging.

[darksylinc]

I'm afraid it's worse now. Colcon won't complain (ok it does because it doesn't find libOgreHlmsPbs_d and libOgreHlmsUnlit_d.so) but now workarounding the bugs won't fix anything.

Colcon just refuses to install libignition-rendering-ogre2 but says everything is fine.

A couple notes:

I'm building with colcon build --cmake-args -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Debug -DBUILD_DOCS=OFF --merge-install notice CMAKE_BUILD_TYPE=Debug

I'm patching ign-rendering with this so that Debug builds and runs ok:

diff --git a/ogre2/src/CMakeLists.txt b/ogre2/src/CMakeLists.txt
index 44b8b5a5..b19a115f 100644
--- a/ogre2/src/CMakeLists.txt
+++ b/ogre2/src/CMakeLists.txt
@@ -1,3 +1,8 @@
+if( UNIX )
+	# lld is much faster than ld for linking
+	set( CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" )
+endif()
+
 # Collect source files into the "sources" variable and unit test files into the
 # "gtest_sources" variable.
 ign_get_libsources_and_unittests(sources gtest_sources)
@@ -33,6 +38,9 @@ target_link_libraries(${ogre2_target}
     ${OPENGL_LIBRARIES}
     IgnOGRE2::IgnOGRE2)
 
+target_compile_definitions(${ogre2_target} PRIVATE $<$<CONFIG:Debug>:DEBUG=1 _DEBUG=1>)
+
+
 set (versioned ${CMAKE_SHARED_LIBRARY_PREFIX}${PROJECT_NAME_LOWER}-${engine_name}${CMAKE_SHARED_LIBRARY_SUFFIX})
 set (unversioned ${CMAKE_SHARED_LIBRARY_PREFIX}${PROJECT_NAME_NO_VERSION_LOWER}-${engine_name}${CMAKE_SHARED_LIBRARY_SUFFIX})
 

and only have libogre-1.9-dev installed but not libogre-2.1-dev. I suspect this could be the reason. I have it uninstalled because if the system one (built for Release) gets mixed with the custom one (built for Debug) weird crashes appear

Another thing I noticed is that some of the paths were wrong:

1. Hlms ones were corrected by me.
2. Overlay is pointing to 1.9's system one
3. pkgcfg is pointing to 1.9's system OgreMain

I will now try a non-debug build to see if that works.

Update:

Release build worked fine!
I guess that when I try to manually workaround libOgreHlmsPbs_d & libOgreHlmsUnlit_d, the new code permanently gives up trying to build Ogre2 instead of trying again.

Update 2:

The following patch fixed Debug for me (on top of your PR):

diff --git a/cmake/FindIgnOGRE2.cmake b/cmake/FindIgnOGRE2.cmake
index c0be08f..d05e678 100644
--- a/cmake/FindIgnOGRE2.cmake
+++ b/cmake/FindIgnOGRE2.cmake
@@ -217,6 +217,8 @@ if (NOT WIN32)
   foreach(component ${IgnOGRE2_FIND_COMPONENTS})
     find_library(OGRE2-${component}
       NAMES
+        "Ogre${component}_d.${OGRE2_VERSION}"
+        "Ogre${component}_d"
         "Ogre${component}.${OGRE2_VERSION}"
         "Ogre${component}"
       HINTS ${OGRE2_LIBRARY_DIRS})

The _d files must come first or else 1.9's system libOgreOverlay will take precedence over 2.1's libOgreOverlay_d

PS I suspect what prevented my workarounds from working anymore is: set(OGRE_FOUND false) but I didn't verify.

[darksylinc]

Btw just add the suggested patch in #174 (comment) and it would be good to go

[iche033]

applied patch in ca041f9, thanks!

[adlarkin]

New patch seems good to me, thanks @darksylinc!