-
Notifications
You must be signed in to change notification settings - Fork 51
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
Find IgnOGRE once, then warn if version is too new #428
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Codecov Report
@@ Coverage Diff @@
## ign-rendering5 #428 +/- ##
===============================================
Coverage 57.77% 57.77%
===============================================
Files 161 161
Lines 15947 15947
===============================================
+ Hits 9213 9214 +1
+ Misses 6734 6733 -1
Continue to review full report at Codecov.
|
CMake Warning appears to be fixed in CI
INTEGRATION_camera test failed - is that known flaky?
|
@sloretz I think this warning was addressed with: ign-rendering#411 and it should be forward ported |
forward ported in #424 |
@sloretz, I think the forward port should fix the issue. Can we close this? related changes in the forward port: |
Closing, fix is being forward ported |
🦟 Bug fix
Fixes #400
Summary
Note: I have not tested this locally.
This is an idea to fix #400. I would expect it to get rid of the CMake warning about missing Ogre when the installed version is
1.9.0
. It doesn't look like Ignition CMake ore Ignition Rendering pass theEXACT
argument to the realfind_package()
call for eitherIgnOGRE
orOGRE
. Because of that, the compatibility is up toOGRE
, and it usesSameMajorVersion
compatibility. That meansign_find_package(IgnOGRE VERSION 1.9.0 ...)
should find OGRE1.x
wherex >= 9
. This PR checksOGRE_VERSION
after findingIgnOGRE
, and only then warns if it is newer than the supported version.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸