Closed wjwwood closed 7 years ago
Lovely, thanks William.
R/
On Wed, Apr 26, 2017 at 2:26 PM, William Woodall notifications@github.com wrote:
These are the leftover patches that ROS Lunar applied to 4.1.1 after rebasing on top of the current master. I don't recommend taking all of these upstream, but instead I just wanted to make the pr to show @rtv https://github.com/rtv what is left over. I'll make another pull request when we decide which to take and which to continue applying as patches when we release into ROS.
- 695a4a5 https://github.com/rtv/Stage/commit/695a4a57f832cd0e74b436d96e302c7efa2a0783
- this commit was fixed upstream, but my version of the patch also included some missing headers
- I recommend you take this one, but I can make an isolated pr for it if you like
- 0509537 https://github.com/rtv/Stage/commit/0509537f51a72fc9fff8e6fa309a525e69096f52
- this commit uses a generator expression to figure out what the real location of the stage library is when placing it in the cmake config file, it also makes the entire result relocatable by using relative paths
- I recommend you take this one, but I can make an isolated pr for it if you like
- 83555f3 https://github.com/rtv/Stage/commit/83555f35be08ef74bcd4d980f9c1922840d82007
- I don't even remember why this change was made, but it seems harmless to change
- I don't think it is important, but I can make a pr if you like
- 9f6e9ba https://github.com/rtv/Stage/commit/9f6e9bae3c51480d7f4c1b329493bbe30bc21839
- This one was done, I think, because of a limitation of catkin where it doesn't add anything by lib to the LD_LIBRARY_PATH, so if something is installed to /opt/ros/lunar/lib64 (for example) then it won't be found
- I would recommend we keep this one as a patch in the ROS release pipeline, since it doesn't seem general enough to merge upstream
- 07ccdc1 https://github.com/rtv/Stage/commit/07ccdc18cb3c61cb652d307ba79d565309839809
- This is just a rule to install the package.xml which is added during the release process.
- I would recommend not taking this patch.
Ok, let me know which ones you'd like me to make into pr's.
You can view, comment on, or merge this pull request online at:
https://github.com/rtv/Stage/pull/78 Commit Summary
- Adding install rule for package.xml
- Do not install to lib64, even on x86_64 machines
- quote prefix in stage.pc.in
- determine the location of the stage library using a generator expression
- fixing abs ambiguity compiler error
File Changes
- M CMakeLists.txt https://github.com/rtv/Stage/pull/78/files#diff-0 (25)
- M cmake/internal/FindOS.cmake https://github.com/rtv/Stage/pull/78/files#diff-1 (36)
- M libstage/world.cc https://github.com/rtv/Stage/pull/78/files#diff-2 (3)
- M stage-config.cmake.in https://github.com/rtv/Stage/pull/78/files#diff-3 (2)
- M stage.pc.in https://github.com/rtv/Stage/pull/78/files#diff-4 (2)
Patch Links:
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rtv/Stage/pull/78, or mute the thread https://github.com/notifications/unsubscribe-auth/AACCnds3SeJTUqwxUSY4bfzIDJTS3Mnmks5rz7Z9gaJpZM4NJch4 .
-- Richard Vaughan Autonomy Lab / Computing Science / Simon Fraser University
Good work, thanks William.
RE: 9f6e9ba
Player had the same logic, which is kind of hacky and not great. It was recently replaced with GNUInstallDirs. A similar approach might help for Stage.
@jpgr87 After looking again, I think catkin may support this case now: https://github.com/ros/catkin/pull/624
So maybe we can just drop 9f6e9ba altogether.
OK, so please make a PR of the first three patches, I'll apply it, do a little bit more tidying up and make a release.
Ok, I'll do that this evening.
Thanks William.
@rtv did you mean to merge this pr? https://github.com/rtv/Stage/pull/80 is still open.
These are the leftover patches that ROS Lunar applied to 4.1.1 after rebasing on top of the current master. I don't recommend taking all of these upstream, but instead I just wanted to make the pr to show @rtv what is left over. I'll make another pull request when we decide which to take and which to continue applying as patches when we release into ROS.
lib
to theLD_LIBRARY_PATH
, so if something is installed to/opt/ros/lunar/lib64
(for example) then it won't be foundpackage.xml
which is added during the release process.Ok, let me know which ones you'd like me to make into pr's.