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

camera_calibration_parsers fails to build on OS X --> unnecessary flags? #40

Closed
helenol opened this issue Oct 20, 2015 · 2 comments · Fixed by #41
Closed

camera_calibration_parsers fails to build on OS X --> unnecessary flags? #40

helenol opened this issue Oct 20, 2015 · 2 comments · Fixed by #41

Comments

@helenol
Copy link
Contributor

helenol commented Oct 20, 2015

I think these lines are no longer necessary:
https://github.com/ros-perception/image_common/blob/hydro-devel/camera_calibration_parsers/CMakeLists.txt#L18-L22

as the brew install yaml-cpp-0.3 command links the libraries properly (as does brew install yaml-cpp which is the 0.5 version, for that matter). (Yosemite, indigo).

Without those lines, it works even with yaml-cpp 0.5 verison which is now standard.

This is the issue referenced here: http://wiki.ros.org/jade/Installation/OSX/Homebrew/Source#os.2BAC8-OSX.2BAC8-Homebrew.2BAC8-troubleshooting.camera_calibration_parsers_build_problems
and is one of the reasons we've had to keep yaml 0.3 around even though 0.5 is the new standard.

I'll make a pull request into hydro-devel if this seems reasonable?

@vrabaud
Copy link
Contributor

vrabaud commented Oct 20, 2015

Well, please figure that out with @njzhangyifei a303896
I know you guys are on a rolling release so I'll accept your PR.

@njzhangyifei
Copy link
Contributor

I agree with removing those lines as they look unnecessary now.

These lines were added because there isn't a FindYamlCpp.cmake back then (see issue ros/cmake_modules#30). If you look at #31, you will find that PR adds a FindYamlCpp.cmake that will work with Mac OS. With that, the same cmake logic should work for both Linux and Mac.

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 a pull request may close this issue.

3 participants