Skip to content
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

Merged
merged 2 commits into from
Feb 28, 2019
Merged

add local_setup files #993

merged 2 commits into from
Feb 28, 2019

Conversation

dirk-thomas
Copy link
Member

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.

Copy link

@jubeira jubeira left a 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 and top_ws.
  • I built and sourced bottom_ws, and then I built top_ws.
  • I sourced top_ws/devel/local_setup.bash and top_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 and CMAKE_PREFIX_PATH look good: for the local script I only see paths below top_ws, and for the regular script I see paths below both top_ws and bottom_ws.
  • PATH also looks consistent; the local script doesn't modify the default path, and the regular script adds bottom_ws/devel/bin.
  • My question is about ROS_PACKAGE_PATH. For the regular script, the source spaces for top_ws and bottom_ws were added, but for the local script none of them were. Is that expected? Considering the observations above for LD_LIBRARY_PATH, PYTHONPATH, PKG_CONFIG_PATH and CMAKE_PREFIX_PATH I would have expected top_ws source space to be added for the local script to ROS_PACKAGE_PATH.

cmake/templates/local_setup.bash.in Show resolved Hide resolved
@dirk-thomas
Copy link
Member Author

My question is about ROS_PACKAGE_PATH. For the regular script, the source spaces for top_ws and bottom_ws were added, but for the local script none of them were. Is that expected? Considering the observations above for LD_LIBRARY_PATH, PYTHONPATH, PKG_CONFIG_PATH and CMAKE_PREFIX_PATH I would have expected top_ws source space to be added for the local script to ROS_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 catkin where as ROS_PACKAGE_PATH is being set by an environment hook of a specific package. The setup files don't know anything about ROS_PACKAGE_PATH.

I would expect though that the workspace containing the package ros_environment with provides that environment hook to set the ROS_PACKAGE_PATH for that workspace.

Copy link

@jubeira jubeira left a 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants