search

gazebosim / gz-cmake

A set of CMake modules that are used by the C++-based Gazebo projects.
https://gazebosim.org/libs/cmake
Apache License 2.0
24 stars 30 forks source link

[Gazebo9][ign-cmake0] FindOgre.cmake should define OGRE variable names more consistently with OgreConfig.cmake #56

Open osrf-migration opened 5 years ago

osrf-migration commented 5 years ago

Original report (archived issue) by Sean Yen (Bitbucket: seanyen-msft).

This issue was originally reported https://github.com/ms-iot/ROSOnWindows/issues/48.

After some investigation, I found the OGRE variables defined in FindOgre.cmake seems not to be using the same naming convention to OgreConfig.cmake and one mitigation is proposed here: https://bitbucket.org/osrf/gazebo/pull-requests/3090

References:

osrf-migration commented 5 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

Another reference is the FindOGRE.cmake installed by ogre 1.9.0

osrf-migration commented 5 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

libignition-cmake-dev is 0.4.0 in bionic; we can change the source code, but I'm not sure if we can easily change it in the released debians.

osrf-migration commented 5 years ago

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).

The FindOGRE.cmake module in ign-cmake uses pkg-config to look for ogre components on linux and macOS. The pkg-config .pc files are named with a dash instead of an underscore, e.g. OGRE-Terrain. But we should be able to override this in our version and make it consistent with ogre's cmake find module.

osrf-migration commented 5 years ago

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

I regret that we ever used the format Find<Package>.cmake rather than our new convention of FindIgn<Package>.cmake, because these conflicting expectations over what kind of information a cmake find module (or config file) will provide keep biting users when the upstream <Package> developers suddenly decide to start providing config-files but those config-files name their variables differently or don't provide exported targets.

My recommendation would be to add a FindIgnOGRE.cmake file to ign-cmake0 and have all ignition projects (including gazebo) that depend on ign-cmake0 and Ogre update their ign_find_package(~) call to look for IgnOGRE instead of OGRE.

Then we could do a patch release of libignition-cmake-dev, libgazebo-dev, and any other -dev packages that depend on OGRE. I'm not totally clear what the debian release rules would have to say about this, but at least this won't impact API or ABI compatibility, and users for whom everything was already working shouldn't notice any difference as long as all the -dev packages get released at once. (Although maybe some CMakeCaches will need to get deleted, I'm not sure.)

If we decide to go that route then I would recommend simultaneously doing the same thing for UUID (making a FindIgnUUID.cmake file) to resolve issue #40. And we should audit all the other Find<Package>.cmake modules to revise any more potential conflicts before they happen.