-
Notifications
You must be signed in to change notification settings - Fork 208
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 broken rviz_ogre_vendor::RenderSystem_GL target #920
Remove broken rviz_ogre_vendor::RenderSystem_GL target #920
Conversation
Your analysis of the problem makes sense to me. But I find two things weird:
With the latter thing said, should we be trying to |
That's a good point, and it looks like my explanation is wrong. The libraries aren't found on Foxy (Focal) either.
Probably not - and since the target has never worked I think we could remove it and backport it. |
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
e071244
to
ef60930
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this seems completely reasonable to me for Rolling. Though I might suggest that once you merge this, you immediately do a release of rviz into Rolling to ensure that there is no downstream breakage.
I'm more nervous about backporting it to Humble. Like you, I think nobody was actually depending on this, but I'm still worried about the side-effects. I'm not totally sure how to alleviate those worries, but I thought I would bring it up.
See ros2/rviz#920 for discussion and root-cause analysis
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Remove broken rviz_ogre_vendor::RenderSystem_GL target (#920) Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * If vendored assimp is present, always prefer that (#970) The existing behavior is to always prefer the system's assimp package if it is sufficient, even if the vendored assimp package is present. This is very confusing behavior when combined with FORCE_BUILD_VENDOR_PKG. Additionally, the double find_package calls can result in strange behavior if both calls deal create or modify targets, as targets created by the first call will be present when the second call tries to create them. Signed-off-by: Scott K Logan <logans@cottsay.net> Co-authored-by: Chris Lalancette <clalancette@gmail.com> --------- Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Signed-off-by: Scott K Logan <logans@cottsay.net> Co-authored-by: Shane Loretz <sloretz@openrobotics.org> Co-authored-by: Scott K Logan <logans@cottsay.net> Co-authored-by: Chris Lalancette <clalancette@gmail.com>
This fixes an issue that happens on the ROS 2 buildfarm, but has gone unnoticed because nothing uses these targets. It was noticed in
bazel_ros2_rules
: RobotLocomotion/drake-ros#188The issue is these find library calls are looking for libraries that don't have theEdit: Incorrect.lib
prefix. The version of CMake on jammy no longer looks for libraries without thelib
prefix on Linux by default. This restores that behavior.This should be backported to Humble.