Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
meyerj opened this issue Nov 3, 2014 · 5 comments
Open

Comments

@meyerj
Copy link
Member

meyerj commented Nov 3, 2014

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
Copy link
Contributor

doudou commented Nov 4, 2014

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

e6c2b91 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
Copy link
Member Author

meyerj commented Nov 4, 2014

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

metaruby could become a 3rd-party ROS package as orogen depends on it now. See rock-core/tools-metaruby#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
Copy link
Contributor

doudou commented Nov 5, 2014

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 orocos-toolchain/utilrb#10

@meyerj
Copy link
Member Author

meyerj commented Dec 2, 2014

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 added a commit that referenced this issue Dec 3, 2014
…make workspace

...as suggested here: #23 (comment)

Signed-off-by: Johannes Meyer <[email protected]>
@meyerj
Copy link
Member Author

meyerj commented Dec 3, 2014

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants