add env vars ROS_PACKAGE_PATH and ROS_ETC_DIR#2
Conversation
|
The second commit aligns the code with the ROS 2 specific PR #3. |
So the goal is that |
| @@ -0,0 +1,26 @@ | |||
| # generated from ros_environment/env-hooks/1.ros_package_path.sh.em | |||
There was a problem hiding this comment.
The file in roslib has these lines as well, but I don't see them here. Is the omission intended?
# scrub old ROS bin dirs, to avoid accidentally finding the wrong executables
export PATH="`@(PYTHON_EXECUTABLE) -c \"import os; print(os.pathsep.join([x for x in \\\"$PATH\\\".split(os.pathsep) if not any([d for d in ['cturtle', 'diamondback', 'electric', 'fuerte'] if d in x])]))\"`"
There was a problem hiding this comment.
Those are so old distros that I removed them intentionally. None of these legacy ROS distros shares a common platform so I don't expect that they are installed on any current system using current ROS distros.
Master will be removed and there will only be ROS distro specific branches. |
| # ROS_DISTRO value for a workspace, for example to deliberately use a newer version of roslib with | ||
| # an older release or vice-versa, or to define a custom distro (eg, "ROS Banana"). | ||
| from os import environ | ||
| ROS_DISTRO = environ.get("ROS_DISTRO_OVERRIDE", "TODO") |
There was a problem hiding this comment.
What was ROS_DISTRO_OVERRIDE used for in ROS 1?
There was a problem hiding this comment.
for example to deliberately use a newer version of roslib with an older release or vice-versa
Since the environment hooks are not bundled with roslib anymore this use case won't exist anymore.
or to define a custom distro
I added 479033e to keep supporting this corner case.
| set ROS_PACKAGE_PATH=@(CMAKE_SOURCE_DIR)!ROS_PACKAGE_PATH_PARENTS! | ||
| @[else]@ | ||
| REM env variable in installspace | ||
| set ROS_PACKAGE_PATH=@(CMAKE_INSTALL_PREFIX)/share;@(CMAKE_INSTALL_PREFIX)/stacks;%ROS_PACKAGE_PATH_PARENTS% |
There was a problem hiding this comment.
The sh version checks that the "stacks" folder exists before adding it but the bat version doesnt, is it on purpose ?
There was a problem hiding this comment.
This is the existing logic from roslib which I didn't want to change.
| export ROS_ETC_DIR="@(CATKIN_DEVEL_PREFIX)/@(CATKIN_GLOBAL_ETC_DESTINATION)/ros" | ||
| @[else]@ | ||
| # env variable in installspace | ||
| if [ -z "$CATKIN_ENV_HOOK_WORKSPACE" ]; then |
There was a problem hiding this comment.
Is the use of CATKIN_ENV_HOOK_WORKSPACE not supported in the windows case?
There was a problem hiding this comment.
This is the current logic from roslib.
|
lgtm except the differences in behavior pointed out above. If you think it's not worth homogenizing feel free to merge as is 👍 |
tfoote
left a comment
There was a problem hiding this comment.
looks good for reproducing the current behavior, pending the ROS_DISTRO specific branches.
The only change is dropping the older platform cleaning logic which I think is now far enough out of date to be fine.
cafb3c3 to
8395357
Compare
|
The ROS 1 distro specific branches have been created and released:
|
* Get ROS_PACKAGE_PATH from the python * normcase workspace path and eliminate duplicate entries
….em (#13) * Update env. hook batch file to set ros_package_path correctly (#1) * Get ROS_PACKAGE_PATH from the python (#2) * Get ROS_PACKAGE_PATH from the python * normcase workspace path and eliminate duplicate entries * Preserve the content from CMAKE_PREFIX_PATH and remove duplicate entries (#4) * use set to remove duplicate entries in a list
After it has been reviewed and merged I will:
TODOs with1andmelodic/lunar/kinetic