Merge toolchain-2.9 into master and discard the toolchain-2.x branches? #135
Conversation
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]> Conflicts: bindings/ruby/CMakeLists.txt
Signed-off-by: Ruben Smits <[email protected]> Conflicts: bindings/ruby/CMakeLists.txt
Signed-off-by: Johannes Meyer <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
…hain package to here This is only effective if typelib is installed to a catkin install space. Signed-off-by: Johannes Meyer <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
… made it hard to compile it ourselves
Conflicts: CMakeLists.txt bindings/ruby/CMakeLists.txt package.xml typelib/pluginmanager.cc
These headers are installed to the RUBY_EXTENSIONS_INSTALL_DIR (e.g. lib/ruby/1.9.1/x86_64-linux) instead of include/. Are these headers actually required by other packages or are they internal? Signed-off-by: Johannes Meyer <[email protected]>
Conflicts: package.xml
This reverts commit 598eed9. typelib.hh is private. typelib_ruby.hh is public. Signed-off-by: Johannes Meyer <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
Signed-off-by: Ruben Smits <[email protected]>
…into toolchain-2.8
Conflicts: CMakeLists.txt
Check orocos-toolchain/orocos_toolchain#13. Signed-off-by: Johannes Meyer <[email protected]>
ruby/config.h is installed in /usr/include/x86_64-linux-gnu and not in RUBY_INCLUDE_PATH. RUBY_INCLUDE_DIRS contains both required include directories. Signed-off-by: Johannes Meyer <[email protected]>
This commit partially reverts 9bc91be. rosdep cannot handle <package depend="..."> tags for non-catkin dependencies in manifest.xml and still prefers to read manifest.xml over package.xml if the former exists.
…anges to package.xml
…est-xml toolchain-2.9: fix rosdep compatibility of manifest.xml and merge recent changes to package.xml
Apparently the environment variable is not used anymore by typelib.
| @@ -15,7 +15,7 @@ endif (NOT TYPELIB_HARDCODED_PLUGIN_PATH) | |||
| set_source_files_properties(endianness.cc PROPERTIES | |||
| COMPILE_FLAGS -fno-strict-aliasing) | |||
|
|
|||
| target_link_libraries(typeLib ${CMAKE_DL_LIBS} ${Boost_FILESYSTEM_LIBRARIES} ${Boost_SYSTEM_LIBRARIES}) | |||
| target_link_libraries(typeLib pthread ${CMAKE_DL_LIBS} ${Boost_FILESYSTEM_LIBRARIES} ${Boost_SYSTEM_LIBRARIES} ${Boost_THREAD_LIBRARIES}) | |||
There was a problem hiding this comment.
Boost mutexes are used in typelib/utilmm/singleton/server.hh and hence the library must be linked to Boost Thread. Could be that boost::recursive_mutex is header-only if compiled in Release mode, but we observed linker errors in the past, at least in Debug mode.
| <rosdep name="libxml2" /> | ||
| <rosdep name="pkg-config" /> | ||
| <rosdep name="ruby-dev" /> | ||
| <rosdep name="ruby-backports" /> |
There was a problem hiding this comment.
This is required to find the matching keys in the rosdep database.
Alternatively the file could be renamed to autoproj.xml in the future, if that is an acceptable path forward for you.
| cmdline << "-m32" | ||
| else | ||
| cmdline << "-m64" | ||
| end |
There was a problem hiding this comment.
Likely can be discarded if gccxml is obsolete anyway.
| @@ -62,12 +62,13 @@ ENDIF(NOT LibXML_FOUND) | |||
|
|
|||
| CONFIGURE_FILE(typelib.pc.in typelib.pc @ONLY) | |||
| INSTALL(FILES ${CMAKE_BINARY_DIR}/typelib.pc DESTINATION lib/pkgconfig) | |||
| INSTALL(FILES package.xml DESTINATION share/typelib) | |||
There was a problem hiding this comment.
Installation of package.xml is required to make ROS tools happy.
|
I don't see any blocker in there. @g-arjones ? |
|
About the What confuses me is: If you are not using typelib/orogen, doesn't it mean that the whole integration with the rest of your orocos/rtt/ros toolchain is falling behind ? So why not deprecate it once and for all and call it a day ? |
That is what I already suggested earlier, for example in orocos-toolchain/orocos_toolchain#37 (comment). So basically to not consider all packages together as "the Orocos Toolchain" anymore. That is de-facto the case, and only very few changes in one toolchain package actually require synchronized changes in another. So better to maintain, version and release them as independent projects, where orogen and rtt_typelib just depend on (ideally) any recent version of RTT. If there is agreement to go down that path, then basically the orocos_toolchain repository would reach its end-of-life, or only provide log4cpp, rtt and ocl. If that is purely my choice, then so be it, purely for practical reasons and without that I want to offend anyone. Furthermore all macros related to But obviously there are some users who want to build typelib and orogen with ROS tools like catkin (#133), and that might be worth to support independent of Orocos? Actually releasing packages into ROS, such that the buildfarm builds and provides Debian, Ubuntu and Fedora packages, is optional and would put some restrictions on the versions of Ruby and other dependencies that can be used (following the ROS target platforms). But whether to support source builds with ROS tools or not is then up to you or another maintainer to decide. Basically the only requirements are a |
Nope, looks good to me as well. |
I tried to keep them up-to-date in the past, but without actually using typelib (or orogen) myself in the last years. I also do not know about gccxml vs. castxml.
Ideally the extra branches would not even be needed. The only non-trivial differences are in
manifest.xmlandpackage.xml, and related to the old problem that the same filename is used by AutoProj and ROS tools like rosdep. The latter still prefersmanifest.xml(dry) overpackage.xml(wet) if it exists, and handles<depend package="...">(other ROS packages) and<rosdep name="...">(packages provided via apt, pip, gems etc.) differently. That is unfortunate, the author of the package should not have to care and it depends on the build environment, which is probably why the difference has been removed forpackage.xmlin REP-0127 (latest specification is REP-0149). Additionally, AutoProj introduced useful new features like<depend_optional>, but which are not understood by ROS tools and cause errors. And last but not least, all the keys need to be either packages with apackage.xmlfile that can be added to the workspace and built from source, or listed in the rosdep database maintained at https://github.com/ros/rosdistro/tree/master/rosdep.So overall some coordination is required for every dependency update, to not break the build for either AutoProj or for ROS/catkin users.
The version numbers are indeed not so important, especially when building from source. But the version has to be increased for each new release into ROS - provided that this is still desirable. The last release of typelib was into ROS indigo back in 2015 (orocos-gbp/typelib-release). In any case the version numbers and release cycles should IMHO not be coupled to RTT or Orocos Toolchain anymore (and hence no
toolchain-2.xbranches). So please choose whatever you prefer for typelib and other packages you maintain (orocos-toolchain/orogen#140 (comment)).Other changes (master...toolchain-2.9) are actually trivial and could be either reverted or applied to the
masterbranch?package.xmlinCMakeLists.txtand update the outdated maintainer email address[email protected](replaced by[email protected], which currently forwards to [email protected])Makefileis obsolete and has been removed (5e8b7c8)Boost_THREAD_LIBRARIES(40cd3a8)?See also orocos-toolchain/orogen#140 and orocos-toolchain/rtt#321. It would be very nice if we can find a solution for those kind of problems in the future, but I do not see how this can be done without deprecating
manifest.xmlin favor ofautoproj.xmlandpackage.xml(orocos-toolchain/rtt#321 (comment)), and keeping both up-to-date? If there is anything I can do (other than merging the two approved pull requests above), please let me know.Originally posted by @meyerj in #133 (comment)