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

remove libopencv-dev and find_package(OpenCV) #172

Closed
wants to merge 1 commit into from

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Jan 4, 2016

when we install ros-indigo-image-proc and try to compile cv_bridge against opencv3, image_proc in /opt/ros/indigo try to link cv_bridge in catkin_ws and that is linked against opencv3.

in this PR, we changed

  • remove libopencv-dev from package.xml, depending on cv_bridge is ok
  • if we depend on cv_bridge, we do not have to find_package(OpenCV) in each CMakeLists.txt
  • add SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) , so /opt/ros/indigo/lib/libimage_proc.so uses cv_bridge in /opt/ros/indigo, not in catkin_ws, this may need discussion.

this pr requires ros-perception/vision_opencv#105

@trainman419
Copy link

This issue is an architectural feature/problem with newer versions of ROS, and affects all ROS packages which provide C/C++ libraries. (You'll run into similar issues if your try to build roscpp in a workspace that is overlaid on /opt/ros)

rosbuild used RPATH to guarantee link consistency, but RPATH is not allowed according to the debian packaging guidelines, so the use of RPATH was removed and replaced with the use of the LD_LIBRARY_PATH environment variable.

Modifying core ROS libraries to RPATH will negatively affect their ability to be accepted into mainline Debian or Ubuntu in the future, so it probably isn't a good idea. In addition, setting the cmake flag for RPATH globally may affect other packages in the workspace, which is generally not a good idea.

@trainman419
Copy link

Many of these packages depend directly on OpenCV by including the OpenCV headers and linking directly against the OpenCV libraries. Removing the explicit dependency on OpenCV is not a good idea; it means that these packages depend on a transient dependency, which is brittle.

@trainman419
Copy link

The standard ROS practice for building a package from source is to also build all downstream packages from source in the same workspace.

@vrabaud
Copy link
Contributor

vrabaud commented Jan 4, 2016

@trainman419 pointed at a lot of very good reasons not to accept this PR (thx !).
Now, how can we fix your problem @k-okada : you will (unfortunately) always have some OpenCV around and rosdep only allows for one version of a lib. Also, REP 003 defined only one version of OpenCV for Indigo and Jade. I packaged OpenCV3 as a "bonus".
Unfortunately, the only solution I see is to recompile everything that depends on OpenCV. I know, that can be big but the low-level libs (image_transport, image_proc) were really cleaned from useless dependencies. I am trying to get those compiling with OpenCV3 as it goes too.
I am pushing for OpenCV3 in Kinetic by default. Please voice up ! ros-infrastructure/rep#106
Letting it open if somebody has a better suggestion.

@k-okada
Copy link
Contributor Author

k-okada commented Jan 5, 2016

Thanks for comments,

  1. I thought using RPATH in ROS system was good feature and realized that was removed at some point, so your explanation really help me to understand situation,
  2. as for removing explicit dependency to OpenCV, I think this will help you, specially when you want to switch from upstream opencv package to opencv3 ros package. so how about removing libopencv-dev and find_package(OpenCV) from package.xml and CMakeLists.txt. If you find_package(cv_bridge) then I think we still can use OpenCV_INCLUDE_DIRS;

One possibility is to provide ros-indigo-cv-bridge2 and ros-indigo-cv-bridge3 and set conflict each other.

@vrabaud
Copy link
Contributor

vrabaud commented Jan 10, 2016

  1. makes sense: especially if OpenCV3 becomes the default in Kinetic, packages will most likely rely on cv_bridge to not maintain several branches.

To get ros-indigo-cv-bridge2 and ros-indigo-cv-bridge3 conflict sure, but then we would need image_pipeline, image_common, image_transport and many more to have both versions. And it's an all or nothing situation.

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.

None yet

3 participants