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

Patch pybind11_catkin #19

Closed
Tobias-Fischer opened this issue Dec 15, 2020 · 6 comments
Closed

Patch pybind11_catkin #19

Tobias-Fischer opened this issue Dec 15, 2020 · 6 comments

Comments

@Tobias-Fischer
Copy link
Collaborator

Rather than downloading pybind11 as an external dependency as done upstream (https://github.com/ipab-slmc/pybind11_catkin/blob/master/CMakeLists.txt), we should use pybind11 shipped in conda.

@PeterMitrano would you be happy to create a patch for this?

However, looking at the project, I am not even sure whether we need this package in conda. All it does is copying the pybind stuff into the devel space. But in conda pybind11 is readily available and this step should not be required. Any thoughts @traversaro @wolfv?

@PeterMitrano
Copy link
Contributor

FWIW I started with that package because it was the one I was familiar with, if it doesn't add anything over pybind11 we can just drop it. What would the patch be for?

One advantage of pybind11-catkin can be specified in the package.xml, and therefore installed via rosdep. However that doesn't even work for conda, so that point is moot.

@Tobias-Fischer
Copy link
Collaborator Author

Agreed, I think we can drop it. We can map pybind11-catkin to pybind11 in the conda-forge.yaml. Any thoughts @wolfv?

@traversaro
Copy link
Member

However, looking at the project, I am not even sure whether we need this package in conda. All it does is copying the pybind stuff into the devel space. But in conda pybind11 is readily available and this step should not be required. Any thoughts @traversaro @wolfv?

I think that is case that will repeat again. ROS/ROS2 .deb packages are basically a distribution of software on the top of another set of distributions (Ubuntu/Debian) and they need sometimes to vendor packages that otherwise are packaged in the "base" distro to have a recent version of a given dependency. With conda-forge, the latest dependency should be available (or can be made available) directly in conda-forge, so I totally agree that for these vendored packages it make sense to just map them to the corresponding conda-forge package.

A similar issue is there for packages that do not have any ROS-specific dependency, and that the authors package in the build farm just because it is an easy and familiar way to easily get .deb packages, such as https://github.com/ethz-adrl/ifopt . For those we can package them in RoboStack, but I imagine that in the future someone could be interested in package them in conda-forge directly.

@PeterMitrano
Copy link
Contributor

I like the idea of just mapping it -- if it truly provides the same thing, then it would be nice if conda install ros-noetic-pybind11-catkin worked, the user doesn't need to know what's happening

@wolfv
Copy link
Member

wolfv commented Dec 16, 2020

One other issue is that the catkin package installs some automatically generated cmake files to find pybind11_catkin etc.

We have the same problem with the libyaml vendor and other deps in ROS2 and I'm not yet sure what the right solution is since it might involve quite a bit of patching. Or we find a good solution to fix the vendor packages in "shim" packages...

@Tobias-Fischer
Copy link
Collaborator Author

See RoboStack/robostack.github.io#17 for the "meta-issue"

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

4 participants