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

[ROS-O] fix package.xml bugs and compatibility with modern systems #97

Open
wants to merge 4 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

v4hn
Copy link

@v4hn v4hn commented Aug 11, 2024

@jonbinney I'll pick you up on your promise here. 😎

Concerning the patches:

  • missing build_export declarations was introduced with the last merged patch Reduce boost and eigen dependency scope #87
  • rostest was plainly missing
  • the setuptools change works for a very long time already, no concern here
  • the c++11 declaration breaks modern logcxx (and by now also other major libraries) requiring c++14 (and 17).

lepalom and others added 4 commits July 19, 2024 11:50
setup from distutils is deprecated and will be removed eventually.
It already breaks on Debian testing.
Copy link
Contributor

@jonbinney jonbinney left a comment

Choose a reason for hiding this comment

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

LGTM, these are good changes and should be applied to noetic

@jonbinney
Copy link
Contributor

Compiles without warning and tests pass on Noetic/20.04. I'm not a maintainer for this package though - @clalancette would you consider merging this? I'd suggest creating a noetic-devel branch and merging this into that branch to avoid possibly breaking anyone's archaic built-from-source copies of old ros versions.

@v4hn
Copy link
Author

v4hn commented Aug 13, 2024

Thanks for the review.

I'd suggest creating a noetic-devel branch and merging this into that branch to avoid possibly breaking anyone's archaic built-from-source copies of old ros versions.

The first two commits are plain bugs in the kinetic-devel branch that should ideally be fixed there and someone's "archaic built-from-source" setup failed due to the missing export_depend in downstream packages.

The setuptools patch should be compatible with Ubuntu 16.04, but I did not verify and it might be problematic.
The c++ version patch is not directly compatible with Ubuntu 16.04.

Feel free to rearrange the patches upstream or ask me to provide different PRs.

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