Add proper export controls for publicly visible libraries

[undisputed-seraphim]

This merge request seeks to implement backward-compatible CMake export control code for OpenRAVE.

Until this merge, OpenRAVE did not proper CMake export control files that follows CMake standards, just a haphazardly written and installed openrave-config.cmake file with just enough variables set to provide paths to the libraries. This meant that downstream applications had to re-find required dependencies, and re-set variables that were required but not exposed by OpenRAVE, which leads to a lot of mindless copying and pasting without verifying that any of that CMake code makes sense.

Now there is an incoming need for the repo to interop with other build systems (e.g. Bitbake), that requires more robust export controls when used with CMake. In particular, applications that wish to link with OpenRAVE libraries via the Bitbake build process need to be able to find the libraries, and its dependencies recursively, via root prefixes that vary by project. Existing behaviour does not support this requirement. Important libraries such as Boost are also exposed by Bitbake through standardized variables offered through CMake, and is wholly incompatible with the current code in CMakeLists.txt to look for Boost.

Going forward, projects should link against OpenRAVE with this syntax: target_link_libraries(my_project PUBLIC OpenRAVE::libopenrave) This alone will allow my_project to inherit all the flags that needs to be set, paths to the headers, and required dependencies, without any further variable setting. (Projects can still choose to override those values if they need to.)

Support for the old behaviour is still available to prevent mass breakage of existing code. The variables required for old behaviour are set in the resulting openrave-config.cmake file (see changes to openrave-config.cmake.in); think of it as 'training wheels' for behaviour migration. In the far future, it is trivial to simply remove those variables to enforce new behaviour, if desired.

Pipeline #521867

[undisputed-seraphim]

Latest: Pipeline #526050

[cielavenir]

Pipeline #532641

[edit] required after controllercommoncpp update

[rdiankov]

@undisputed-seraphim can you put this on a local branch of rdiankov/openrave so that others I can edit? and change this pull request's source branch. thanks

[cielavenir]

@rdiankov it is confusing but it exists on rdiankov/openrave's origin/liboon/neo-cmake-exports

[undisputed-seraphim]

Yeah it's already in this repository.

[rdiankov]

thanks

[cielavenir]

@undisputed-seraphim Actually this pull request broke very plain openrave installation, where 3rdparty/collada-2.4.0 and 3rdparty/fparser-4.5.2 are statically linked.

-- Configuring done
CMake Error: install(EXPORT "openrave-targets" ...) includes target "libopenrave" which requires target "fparser" that is not in any export set.
CMake Error: install(EXPORT "openrave-targets" ...) includes target "libopenrave-core" which requires target "fparser" that is not in any export set.
CMake Error: install(EXPORT "openrave-targets" ...) includes target "libopenrave-core" which requires target "collada15reader" that is not in any export set.
CMake Error: install(EXPORT "openrave-targets" ...) includes target "libopenrave-core" which requires target "collada15reader" that is not in any export set.
-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

Now installing https://github.com/rdiankov/collada-dom and https://github.com/rdiankov/fparser are mandated.

Now, to fix this, add_library(STATIC) ones have to be added to PRIVATE. My current state is like this:

diff --git a/src/libopenrave-core/CMakeLists.txt b/src/libopenrave-core/CMakeLists.txt
index bfd44d8c5..8289f54ec 100644
--- a/src/libopenrave-core/CMakeLists.txt
+++ b/src/libopenrave-core/CMakeLists.txt
@@ -3,7 +3,7 @@ if( HAVE_MKSTEMP )
   add_definitions("-DHAVE_MKSTEMP")
 endif()

-set(OPENRAVE_CORE_LIBRARIES ${openrave_libraries} ${OPENRAVE_CURL_LIBRARIES})
+set(OPENRAVE_CORE_LIBRARIES ${OPENRAVE_CURL_LIBRARIES})
 set(openrave_core_SOURCES openrave-core.cpp environment-core.h openrave-core.h ravep.h  xmlreaders-core.cpp genericcollisionchecker.cpp genericphysicsengine.cpp genericrobot.cpp multicontroller.cpp generictrajectory.cpp jsonparser/jsoncommon.cpp jsonparser/jsonreader.cpp jsonparser/jsonwriter.cpp jsonparser/jsondownloader.cpp)

 if( libpcrecpp_FOUND )
@@ -71,8 +71,8 @@ set_target_properties(libopenrave-core PROPERTIES OUTPUT_NAME openrave${OPENRAVE
                                   COMPILE_FLAGS "${LIBOPENRAVE_COMPILE_FLAGS} -DOPENRAVE_CORE_DLL_EXPORTS -DOPENRAVE_CORE_DLL"
                                   LINK_FLAGS "${LIBOPENRAVE_LINK_FLAGS}")
 target_link_libraries(libopenrave-core
-  PRIVATE boost_assertion_failed ${IVCON_LIBRARY}
-  PUBLIC libopenrave ${OPENRAVE_CORE_LIBRARIES}
+  PRIVATE boost_assertion_failed ${IVCON_LIBRARY} ${openrave_libraries} ${OPENRAVE_CORE_LIBRARIES}
+  PUBLIC libopenrave
 )
 install(TARGETS libopenrave-core
   EXPORT openrave-targets
@@ -117,8 +117,8 @@ if( OPT_STATIC )
                                                 COMPILE_FLAGS "${LIBOPENRAVE_COMPILE_FLAGS} "
                                                 LINK_FLAGS "${LIBOPENRAVE_LINK_FLAGS}")
   target_link_libraries(libopenrave-core_static
-    PRIVATE boost_assertion_failed ${IVCON_LIBRARY}
-    PUBLIC libopenrave_static ${OPENRAVE_CORE_LIBRARIES}
+    PRIVATE boost_assertion_failed ${IVCON_LIBRARY} ${OPENRAVE_CORE_LIBRARIES}
+    PUBLIC libopenrave_static
   )
   install(TARGETS libopenrave-core_static
     EXPORT openrave-targets
diff --git a/src/libopenrave/CMakeLists.txt b/src/libopenrave/CMakeLists.txt
index 157c881af..92d7905aa 100644
--- a/src/libopenrave/CMakeLists.txt
+++ b/src/libopenrave/CMakeLists.txt
@@ -64,8 +64,8 @@ set_target_properties(libopenrave PROPERTIES OUTPUT_NAME openrave${OPENRAVE_LIBR
                                   LINK_FLAGS "${LIBOPENRAVE_LINK_FLAGS} ${FPARSER_LINK_FLAGS}")
 target_compile_definitions(libopenrave PRIVATE "OPENRAVE_STATIC_PLUGINS=${OPENRAVE_STATIC_PLUGINS}")
 target_link_libraries(libopenrave
-  PRIVATE boost_assertion_failed static_plugins openrave-md5 crlibm
-  PUBLIC LibXml2::LibXml2 Boost::filesystem Boost::thread ${CMAKE_DL_LIBS} ${openrave_libraries}
+  PRIVATE boost_assertion_failed static_plugins openrave-md5 crlibm ${openrave_libraries}
+  PUBLIC LibXml2::LibXml2 Boost::filesystem Boost::thread ${CMAKE_DL_LIBS}
 )
 install(TARGETS libopenrave
   EXPORT openrave-targets
@@ -110,8 +110,8 @@ if( OPT_STATIC )
     add_dependencies(libopenrave_static check_libm_accuracy-native) # forces check to be run before libopenrave is compilde
   endif()
   target_link_libraries(libopenrave_static
-    PRIVATE boost_assertion_failed static_plugins openrave-md5 crlibm
-    PUBLIC LibXml2::LibXml2 Boost::filesystem Boost::thread ${openrave_libraries}
+    PRIVATE boost_assertion_failed static_plugins openrave-md5 crlibm ${openrave_libraries}
+    PUBLIC LibXml2::LibXml2 Boost::filesystem Boost::thread
   )
   install(TARGETS libopenrave_static
     EXPORT openrave-targets

At least now cmake succeeds, but I will try to check the root cause further.

Lastly, I'm sorry, I should have tested this change in my own build infra.

/cc @lazydroid

[cielavenir]

https://stackoverflow.com/a/57096197/2641271 could be used