orocos-toolchain / orogen

Code generator for components and type handling in Rock - the Robot Construction Kit - and the Orocos Toolchain
http://rock-robotics.org
Other
4 stars 35 forks source link

orogen: Merge toolchain-2.7 and master branches for the 2.8 release #23

Open meyerj opened 9 years ago

meyerj commented 9 years ago

The master and rock1408 branches of orogen diverged from toolchain-2.7 significantly (or vice-versa):

Which commits from which branch should be included in a new toolchain-2.8 branch for the upcoming release?

Related questions:

doudou commented 9 years ago

Apart from the now usual 'ros-specific stuff goes into its own branch now', there are two commits that I worry a bit about:

https://github.com/orocos-toolchain/orogen/commit/e6c2b9153abb4de521bd4ff0eca8ff7013965768 replaces the plain cmake by the RTT macros. Not against the principle at all, but I'd like to finally have a description of what orocos_typekit does do before I would venture there.

1900a23 is a definite no as it breaks the usual orogen workflow (where make regen is a common occurence). I guess you could simply add a custom 'regen' target that depends on all the specific typekit targets.

70e131f seem to have already been cherry-picked and 23d1d31 I just cherry-picked

meyerj commented 9 years ago

For the general 'ros-specific stuff goes into its own branch now' issues, see https://github.com/orocos-toolchain/utilrb/issues/10.

metaruby could become a 3rd-party ROS package as orogen depends on it now. See https://github.com/rock-core/tools-metaruby/issues/4.

libxslt is listed as a dependency in manifest.xml, but there are no direct references within orogen. It is only used indirectly via nokogiri. Is this correct?

doudou commented 9 years ago

libxslt is listed as a dependency in manifest.xml, but there are no direct references within orogen. It is only used indirectly via nokogiri. Is this correct?

Yes. And it burns my eyes to have it here. Was added for ROS so that the nokogiri gem installs properly. autoproj can specify these kind of constraints in its osdeps files, so it does not need it.

You'll also need to separate <rosdep ...> from <package ...> and deal with the optional tag ... Same as https://github.com/orocos-toolchain/utilrb/issues/10

meyerj commented 9 years ago

I prepared a branch where I merged toolchain-2.8 with master including my manifest-cleanup pull request #24: https://github.com/meyerj/orogen/tree/toolchain-2.8-manifest-cleanup

As for typelib and orogen there are almost no differences anymore in the manifest.xml and package.xml files, but the templates folder differs significantly:

How can we reach a consensus here? Or will we live with the diverged branches?

As already stated by @doudou above, the main change in toolchain-2.8 is the transition from plain cmake to the Ororocs cmake macros. This update is unavoidable for ROS and catkin compatibility as the build targets do not only have to be installed in the correct folder, but also built in the correct folder in order to have a working configuration before the actual install step (called the devel-space in catkin). The whole purpose of the orocos_* macros is to make sure that components, libraries, typekits etc. are built and installed with the correct defaults for the respective library type and build system and that they are automatically linked to whatever library in any package that has been imported with orocos_use_package(...). You can still overwrite target properties afterwards or link to additional libraries, if needed.

The best documentation that I found is http://www.orocos.org/wiki/orocos/toolchain/getting-started/cmake-and-building and in the RTT Cheat Sheet, which are still up-to-date as far as I can see. I agree that all the cmake macros and magic behind them should be much better documented and specified.

The distinct cmake target names introduced in 1900a23 are also required as with catkin multiple packages could be built in the same cmake top-level project in a single build directory. Readding a common regen target as suggested should solve this issue.

Like in utilrb, the CMakeLists.txt in master could be removed if not needed for Autoproj/Rock or synchronized with toolchain-2.8.

meyerj commented 9 years ago

I readded the global regen target to the toolchain-2.8 branch in 9998ce0d3f50594097215b5f92b190a28625a435. make regen will now regenerate all typekits within the same cmake workspace.