orocos-toolchain / ocl

Orocos Component Library
Other
16 stars 33 forks source link

orocreate-pkg updates for rosbuild/catkin compatibility #2

Closed meyerj closed 10 years ago

meyerj commented 10 years ago

This pull request collects updates for the orocreate-pkg utility.

The first patch only adds the ORO_USE_ROSBUILD variable to the generated Makefile to enforce in-source building (see https://github.com/orocos-toolchain/rtt/pull/19). The Makefile is not used by catkin, so the generated packages will still be compiled in the catkin devel-space if they are part of a catkin workspace. Also changed INSTALL_PATH to ORO_DEFAULT_INSTALL_PREFIX as a comment in UseOROCOS-RTT.cmake says it is only for backwards compatibility.

The following issues still need to be resolved/discussed:

For full compatibility we would also have to replace all occurrences of <depend package="rtt"/> in package manifests by <rosdep name="rtt"/> or alternatively install rtt.pc wrappers that forward to the default target in some package (equally for all packages in orocos_toolchain). Otherwise rosbuild/rospack will fail to find the compile flags and libraries for these packages in 2.7.

jbohren commented 10 years ago
  • Should orocreate-pkg continue to create rosbuild packages (generate a Makefile and a manifest.xml) or only catkin (package.xml)?

Maybe it should take an argument, or we could split it into orocreate-rosbuild-pkg and orocreate-catkin-pkg.

  • Should we only create the package.xml file instead of the Makefile and manifest.xml files if orocreate-pkg was invoked with orocreate_pkg --catkin? Or print a message that tells the user to delete either the package.xml file or Makefile and manifest.xml?

Yeah, I don't think we should get into the messy world of hybrid buildsystems for client packages.

  • What happens in the new ros.import(...) operation if both manifest files exist, manifest.xml and package.xml? The current version only looks at the package.xml file in this case, so updating only the manifest.xml has no effect on recursive package loading.

Right now it only parses manifest.xml if package.xml is not found. I think that's fine behavior, and we can explicitly say that we don't support hybrid buildsystems.

https://github.com/orocos/rtt_ros_integration/blob/hydro-devel/rtt_ros/src/rtt_ros_service.cpp#L122

  • Should we add an --upgrade flag that tries to upgrade a single package or a whole tree of packages created with an earlier version of orocreate-pkg? Currently it would only replace the Makefile with the new version based on a checksum test to find Makefiles generated by orocreate-pkg.

In my experience building such automatic upgrade tools can take a lot of effort to get robust.

jbohren commented 10 years ago

For full compatibility we would also have to replace all occurrences of <depend package="rtt"/> in package manifests by <rosdep name="rtt"/> or alternatively install rtt.pc wrappers that forward to the default target in some package (equally for all packages in orocos_toolchain). Otherwise rosbuild/rospack will fail to find the compile flags and libraries for these packages in 2.7.

I'm going to write up the changes between 2.6 and 2.7 and see if we can break as little as possible by adding back in some backwards-compatibility and deprecation warnings.

smits commented 10 years ago

merged in toolchain-2.7 and master