-
Notifications
You must be signed in to change notification settings - Fork 280
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
add local_setup files #993
Conversation
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.
I tested the code and it's looking good. I do have a question though; here's what I did to test it (question at the bottom):
- I created two test workspaces, namely
bottom_ws
andtop_ws
. - I built and sourced
bottom_ws
, and then I builttop_ws
. - I sourced
top_ws/devel/local_setup.bash
andtop_ws/devel/setup.bash
from two new different terminals.
Then I analyzed the paths using env | grep path -i
. Here's the conclusions:
LD_LIBRARY_PATH
,PYTHONPATH
,PKG_CONFIG_PATH
andCMAKE_PREFIX_PATH
look good: for the local script I only see paths belowtop_ws
, and for the regular script I see paths below bothtop_ws
andbottom_ws
.PATH
also looks consistent; the local script doesn't modify the default path, and the regular script addsbottom_ws/devel/bin
.- My question is about
ROS_PACKAGE_PATH
. For the regular script, the source spaces fortop_ws
andbottom_ws
were added, but for the local script none of them were. Is that expected? Considering the observations above forLD_LIBRARY_PATH
,PYTHONPATH
,PKG_CONFIG_PATH
andCMAKE_PREFIX_PATH
I would have expectedtop_ws
source space to be added for the local script toROS_PACKAGE_PATH
.
This is certainly counter intuitive. The reason for the different behavior is that the other variables are being set by the setup logic generated by I would expect though that the workspace containing the package ros_environment with provides that environment hook to set the |
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.
Thanks for the clarification; I don't have any other comments so LGTM!
The existing setup files extend the environment with paths of the workspace they are located in as well as all chained workspaces. The newly added local setup files only extend the environment with paths of a single workspace. This enables the user to specifically select which workspaces to source - independently on how they have been chained during the build.