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

Always build OpenCV with CMAKE_BUILD_TYPE=Release (fix #14) #15

Merged

Conversation

meyerj
Copy link

@meyerj meyerj commented Jan 12, 2018

... to fix RelWithDebInfo builds of derived packages.
There might be several other options to fix the problem, e.g. by patching upstream OpenCV.

I verified that bloom properly exports the patch and reapplies it for each new release (in kinetic) once this pull request would be merged. The same patch should be applied to debian/lunar/opencv3, which has the same version and is probably also affected.

@meyerj
Copy link
Author

meyerj commented Jan 30, 2018

The binary Debian package build from this branch solved to problem described in #14 for us (in Ubuntu Xenial using ROS kinetic):
https://github.com/meyerj/opencv3-release/releases/download/debian%2Fros-kinetic-opencv3_3.3.1-2_xenial/ros-kinetic-opencv3_3.3.1-2xenial_amd64.deb

Steps to build the package locally (for any supported Ubuntu/Debian version, requires python-bloom and other build dependencies to be installed):

cd /tmp
git clone https://github.com/meyerj/opencv3-release.git
cd opencv3-release
git-bloom-generate -y rosdebian --prefix release/kinetic kinetic -i 2
# This may take a while and eventually bloom will ask you for the version to be released (3.3.1)...

# Then checkout the newly created or updated tag, e.g.
git checkout debian/ros-kinetic-opencv3_3.3.1-2_xenial
# ... and finally build the package with
debuild -us -uc -b

The deb file will be created in the parent folder. The same approach should also work for lunar (untested).

Any chance to have this merged soon and trigger another release for the ROS build farm? I assume the upstream version can stay the same.

@tfoote
Copy link
Contributor

tfoote commented Feb 1, 2018

Thanks for the ping @meyerj . @vrabaud I saw you were working on a new release but rolled it back. ros/rosdistro#16773 Will you have a chance to look at this soon? Otherwise I can look at rolling a new version.

@pbeeson
Copy link

pbeeson commented Feb 7, 2018

Bumping to see if we can finally get a working version distributed.

@tfoote
Copy link
Contributor

tfoote commented Feb 21, 2018

After the next Kinetic sync I plan to test deploying it with this patch. @vrabaud Since it seems you're busy these days do you have any notes from your previous releases that I could follow since I know you use a slightly non-standard release mechanism?

@tfoote tfoote merged commit 5fcca65 into ros-gbp:debian/kinetic/opencv3 Feb 24, 2018
tfoote added a commit to ros/rosdistro that referenced this pull request Feb 24, 2018
@tfoote
Copy link
Contributor

tfoote commented Feb 24, 2018

Related: ros-perception/vision_opencv#193

tfoote added a commit to ros/rosdistro that referenced this pull request Feb 24, 2018
* opencv3 3.3.1-3 for kinetic
With the fix from:  ros-gbp/opencv3-release#15
@tfoote
Copy link
Contributor

tfoote commented Mar 16, 2018

Ok, after several tries I believe that this patched version of opencv3 is now in kinetic 3.3.1-5 ros/rosdistro#17136 There was a bunch of learning and some forced updates required. Right now that version has landed in the testing repo for amd64 builds and should be in the arm builds by the morning.

@pbeeson @meyerj could you try it out to verify it's resolved?

@pbeeson
Copy link

pbeeson commented Mar 16, 2018

After pulling opencv3 and cv-bridge from the Shadow Repository, my code compiles without needing any special CMake magic to work around the previous bug.

Great!

@meyerj
Copy link
Author

meyerj commented Mar 16, 2018

@pbeeson @meyerj could you try it out to verify it's resolved?

Seems to work for me, too, but I noticed that with 3.3.1-5 the OpenCV libraries are installed to /opt/ros/kinetic/lib/x86_64-linux-gnu now, while with 3.3.1-0 it used to be /opt/ros/kinetic/lib. The former is typically not in the LD_LIBRARY_PATH and I am not sure whether this might break stuff like relocated install spaces.

@tfoote
Copy link
Contributor

tfoote commented Mar 20, 2018

Thanks, I think that path is ok as that's a standard using the GNUInstallDirs and is the layout that the system installation uses as well.

And found that the full path for the libraries are correctly embedded in the /opt/ros/kinetic/share/OpenCV-3.3.1-dev/OpenCVModules-release.cmake file.

I checked that the find_package calls all work.

tfoote@snowman:/tmp/test Last: [0] (0s Seconds)
$ cat CMakeLists.txt 
cmake_minimum_required (VERSION 2.6)
project (test_opencv3)

find_package(OpenCV REQUIRED)
message(STATUS "OpenCV VERSION: ${OpenCV_VERSION}")
message(STATUS "OpenCV Components: ${OpenCV_LIB_COMPONENTS}")
message(STATUS "OpenCV Libraries: ${OpenCV_LIBRARIES}")
tfoote@snowman:/tmp/test Last: [0] (0s Seconds)
$ cmake .
-- OpenCV VERSION: 3.3.1
-- OpenCV Components: opencv_calib3d;opencv_core;opencv_dnn;opencv_features2d;opencv_flann;opencv_highgui;opencv_imgcodecs;opencv_imgproc;opencv_ml;opencv_objdetect;opencv_photo;opencv_shape;opencv_stitching;opencv_superres;opencv_video;opencv_videoio;opencv_videostab;opencv_viz;opencv_aruco;opencv_bgsegm;opencv_bioinspired;opencv_ccalib;opencv_cvv;opencv_datasets;opencv_dpm;opencv_face;opencv_fuzzy;opencv_hdf;opencv_img_hash;opencv_line_descriptor;opencv_optflow;opencv_phase_unwrapping;opencv_plot;opencv_reg;opencv_rgbd;opencv_saliency;opencv_stereo;opencv_structured_light;opencv_surface_matching;opencv_text;opencv_tracking;opencv_xfeatures2d;opencv_ximgproc;opencv_xobjdetect;opencv_xphoto
-- OpenCV Libraries: opencv_calib3d;opencv_core;opencv_dnn;opencv_features2d;opencv_flann;opencv_highgui;opencv_imgcodecs;opencv_imgproc;opencv_ml;opencv_objdetect;opencv_photo;opencv_shape;opencv_stitching;opencv_superres;opencv_video;opencv_videoio;opencv_videostab;opencv_viz;opencv_aruco;opencv_bgsegm;opencv_bioinspired;opencv_ccalib;opencv_cvv;opencv_datasets;opencv_dpm;opencv_face;opencv_fuzzy;opencv_hdf;opencv_img_hash;opencv_line_descriptor;opencv_optflow;opencv_phase_unwrapping;opencv_plot;opencv_reg;opencv_rgbd;opencv_saliency;opencv_stereo;opencv_structured_light;opencv_surface_matching;opencv_text;opencv_tracking;opencv_xfeatures2d;opencv_ximgproc;opencv_xobjdetect;opencv_xphoto
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/test

@meyerj
Copy link
Author

meyerj commented Mar 21, 2018

Thanks, I think that path is ok as that's a standard using the GNUInstallDirs and is the layout that the system installation uses as well.

Ah, I was not aware of ros/catkin#624.

tfoote added a commit to ros/rosdistro that referenced this pull request Mar 22, 2018
tfoote added a commit to ros/rosdistro that referenced this pull request Mar 23, 2018
HEllRZA pushed a commit to HEllRZA/rosdistro that referenced this pull request Apr 4, 2018
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