[Noetic] create relay scripts in devel space to ensure correct Python shebang

[sloretz]

PEP 394 recommends python scripts have their shebangs re-written to a specific version of python when they're installed. The CMake macro catkin_install_python() does this; however, when the devel space is sourced the script is executed from the src space which does not have rewritten shebangs. This PR creates a relay script in the devel space to make sure the right version of python is invoked. It reuses an existing relay script created by catkin for scripts passed in the scripts argument to setup() in a setup.py.

I think this is appropriate, but I would not release this into kinetic because it changes the behavior of rosrun if the script is executable in the src space. For example, urdfdom_py installs a script display_urdf which is executable and has a shebang #!/usr/bin/env python. When there is also a relay script in the devel space then rosrun complains about there being multiple options.

$ rosrun urdfdom_py display_urdf 
[rosrun] You have chosen a non-unique executable, please pick one of the following:
1) /home/sloretz/ws/noetic/devel_isolated/urdfdom_py/lib/urdfdom_py/display_urdf
2) /home/sloretz/ws/noetic/src/urdfdom_py/scripts/display_urdf
#?

A solution is to remove the executable permissions from display_urdf in src space, which I think is fine for Noetic, but it would mean changing downstream code in an already released distro if this PR was released into kinetic or melodic.

Another solution would be to make rosrun just take the first option, which is what roslaunch does, but that seems like a large change for a stable distro like kinetic.

This is similar to ros/ros_comm#1830, but it does not solve that particular issue since tests shouldn't be installed.

[dirk-thomas]

Please see my comment in the ros_comm ticket.

I would consider both of the following anti-goals:

• require maintainers to branch for Noetic
• change files in the source space during the build

That leaves the option to relax the non-unique check of rosrun in combination with a relay script in the devel space...

The case where this would not be sufficient is if a script is only used for testing but is not being installed. For those scripts no CMake is invoked and therefore we can't generate the relay script automatically. Either the package needs to be touched to generate a relay script in the devel space for this or the (launch) test needs to be updated to consider ROS_PYTHON_VERSION. (I don't see a way to satisfy this case without modifying the package itself.)

[sloretz]

Please see my comment in the ros_comm ticket.

Roger. I'll try to reply to the parts relevant to installing python scripts here, and respond to parts about test scripts in the ros_comm ticket.

That leaves the option to relax the non-unique check of rosrun in combination with a relay script in the devel space...

In the case there are multiple non-unique executable we should only allow to automatically pick the first one if (and only if) there are exactly two executables (this should be trivial to check) - one in the devel space and one on the source space (not sure if this is possible to check?)

Assuming the devel space check is possible, is the proposal to modify rosbash in Kinetic to avoid branching catkin for Noetic?

[dirk-thomas]

The change to rosbash could either land on a to-be-create noetic-devel branch or on the existing kinetic-devel branch if we think the relaxed check would be ok for older distros. I don't think that choice affects catkin?

[sloretz]

The change to rosbash could either land on a to-be-create noetic-devel branch or on the existing kinetic-devel branch if we think the relaxed check would be ok for older distros. I don't think that choice affects catkin?

I think it affects catkin in that this PR should be released only to distros that the rosbash change is released to.

[sloretz]

@dirk-thomas With ros/ros#233 open, I think both are ready to be reviewed together.

[dirk-thomas]

The overall patch LGTM.

Since this will need to target a to-be-created noetic-devel branch I would prefer to separate this PR into two: one targeting kinetic-devel which applies all the generic changes, and one which contains the noetic specific logic to create the relay script in the devel space.

In parallel I will try to get other code cleanup changes landed to avoid diverging these two branches.

[sloretz]

I would prefer to separate this PR into two: one targeting kinetic-devel which applies all the generic changes, and one which contains the noetic specific logic to create the relay script in the devel space.

I split this PR into #1054 (generic changes) and rebased this one. Once #1054 is merged and noetic-devel is created from it, this PR can be retargeted to noetic-devel and will have just one commit.