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

toolchain-2.9: fix rosdep compatibility of manifest.xml and merge recent changes to package.xml #109

Merged
merged 2 commits into from
Nov 14, 2018

Conversation

meyerj
Copy link
Member

@meyerj meyerj commented Jun 26, 2018

I updated the package.xml, which - as far as I know - is only used by ROS tools, and merged the recent dependency changes from master. Like this, it can be compiled using ROS and catkin.

I am not sure about the changes in manifest.xml. Most likely they would break the autoproj bootstrap procedure, right? If yes, the alternative would be to submit a PR to https://github.com/ros/rosdistro/tree/master/rosdep/rosdep.yaml and add the key ruby-dev there.

The differences between toolchain-2.9 and master are actually quite small. Is it desirable to submit individual PRs targeting master and only keep the ROS/catkin-specific changes in toolchain-2.x? Most non-catkin patches have been contributed by @smits.

Ideally the patches related to catkin and ROS-builds are not breaking non-ROS builds and catkin as a dependency is optional. See also related discussion in #23.

@doudou
Copy link
Contributor

doudou commented Jun 26, 2018

If yes, the alternative would be to submit a PR to https://github.com/ros/rosdistro/tree/master/rosdep/rosdep.yaml and add the key ruby-dev there.

That would be better. ruby-dev got re-added because ruby is not ruby-dev in Rock.

About merging the 2.9 changes in master: I would prefer that you don't do anything catkin-related if catkin is not present in the CMakeLists.txt, for instance:

  • generate and install package.xml
  • install the env hook file

manifest.xml Outdated

<!-- rosdep key `ruby-dev` is not known to ROS.
Depend on `ruby` instead, which includes development files. -->
<!-- <rosdep name="ruby-dev" /> -->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doudou Where exactly is key ruby-dev defined in AutoProj? I could not find it in either lib/autoproj/default.osdeps or package_set/rock.osdeps. Is it directly considered to be an OS package if not found elsewhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in autoproj's default.osdeps. It is defined there, as autoproj itself needed to install it.

@meyerj
Copy link
Member Author

meyerj commented Jul 19, 2018

About merging the 2.9 changes in master: I would prefer that you don't do anything catkin-related if catkin is not present in the CMakeLists.txt, for instance:

generate and install package.xml

We could install that file conditionally, but the behavior should be consistent within rtt, ocl, utilrb, typelib and rtt_typelib.

install the env hook file

The env-hook concept from catkin has been mimicked for Orocos in orocos-toolchain/orocos_toolchain#13, to replace the original env.sh script in the orocos_toolchain meta repository. The new setup.sh script would source package-specific env-hooks scripts found in <install-space>/etc/orocos/profile.d/ in setup.sh:79, while catkin does the same for env-hooks found in <install-space>/etc/catkin/profile.d (ROS users only). The latter gets only installed if catkin has been found during installation.

Are there any alternative ways to populate environment variables (PKG_CONFIG_PATH, RTT_COMPONENT_PATH, TYPELIB_USE_GCCXML, etc.) required for Orocos Toolchain packages I am not aware of?

@doudou
Copy link
Contributor

doudou commented Jul 20, 2018

Are there any alternative ways to populate environment variables (PKG_CONFIG_PATH, RTT_COMPONENT_PATH, TYPELIB_USE_GCCXML, etc.) required for Orocos Toolchain packages I am not aware of?

Well ... autoproj for once does not use these env.sh files, and I'd rather not have to explain why /etc is polluted by files nobody ever heard about on our side.

@meyerj
Copy link
Member Author

meyerj commented Nov 14, 2018

Key ruby-dev has been added to the rosdep database in ros/rosdistro#18302. So no changes to manifest.xml required. What is left is only a minor cleanup of package.xml.

@meyerj meyerj merged commit 89063c5 into toolchain-2.9 Nov 14, 2018
@meyerj meyerj deleted the toolchain-2.9-fix-manifest-xml branch November 14, 2018 15:45
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

Successfully merging this pull request may close these issues.

2 participants