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

update include statements to use new pluginlib and class_loader headers #38

Conversation

mikaelarguedas
Copy link
Contributor

Disclaimer: This in an automated PR, if it is not relevant on this branch, apologies for the inconveniance, feel free close it.


pluginlib and class_loader headers have been refactored and renamed. The previous headers .h have been deprecated in favor of their .hpp equivalent. The new headers are available on all active ROS distributions and the deprecated ones will be removed in a future version of ROS (likely ROS-N).

This PR migrates the include statements to use the non-deprecated ones and should compile for any active ROS distribution starting with Indigo
The migration was done by running the scripts pluginlib_headers_migration.py and class_loader_headers_update.py on this repository.
Note: this will not work for Jade

@v4hn
Copy link
Contributor

v4hn commented Apr 7, 2018

should not be merged yet, because the upstream changes are not rolled out yet.

@mikaelarguedas
Copy link
Contributor Author

Upstream changes have been released and synced on all active distros. From what it looks like the only CI that is failing is Jade, that will not see a new release as it's been EOL for a while now.

@v4hn
Copy link
Contributor

v4hn commented Apr 7, 2018

hae? I see that now, sorry.
But then why did the same fail for moveit kinetic/lunar?

I approve this being merged into a new kinetic-devel branch.
Releases should be done from that branch too then.

@mikaelarguedas
Copy link
Contributor Author

At a glance that ci is based on ros-base docker images that have not been rebuilt yet (last build was 24 days ago, before the last sync).
This is a known problem of the official ros images that is being investigated osrf/docker_images#112.

In the meantime I recommend running an apt upgrade before performing builds in CI to ensure to have up-to-date packages in your.

@mikaelarguedas
Copy link
Contributor Author

I approve this being merged into a new kinetic-devel branch.
Releases should be done from that branch too then.

👍
I targeted the branches used in the lunar distribution file to open these PRs. In the case of this repo, it was jade-devel.
It could also be merged in this branch if you don't want to branch out and assume that people still building Jade are either building the distro from source or using the jade debs (in the former case headers are updated and match,, in the latter nothing changes as no new deb will be created)

@rhaschke rhaschke changed the base branch from jade-devel to kinetic-devel May 31, 2018 15:21
@rhaschke rhaschke merged commit 25ee34c into moveit:kinetic-devel May 31, 2018
@mikaelarguedas mikaelarguedas deleted the mikael/pluginlib_class_loader_header_migration branch May 31, 2018 15:53
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.

3 participants