Remaining ros patches from 4.1.1 #78
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also makes it relocatable in the process fix stage cmake config and make it relocatable fix stage config file (even for saucy)
|
Lovely, thanks William.
R/
…On Wed, Apr 26, 2017 at 2:26 PM, William Woodall ***@***.***> 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
<695a4a5>
- 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
<0509537>
- 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
<83555f3>
- 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
<9f6e9ba>
- 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
<07ccdc1>
- 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:
#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:
- https://github.com/rtv/Stage/pull/78.patch
- https://github.com/rtv/Stage/pull/78.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#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: ros/catkin#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. |
This was referenced Apr 27, 2017
|
Thanks William. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
libto theLD_LIBRARY_PATH, so if something is installed to/opt/ros/lunar/lib64(for example) then it won't be foundpackage.xmlwhich is added during the release process.Ok, let me know which ones you'd like me to make into pr's.