Using ${prefix}/lib in package.xml plugin_path is wrong.

[peci1]

This is a port of https://bitbucket.org/osrf/gazebo_tutorials/pull-requests/552/using-prefix-lib-in-packagexml-plugin_path to github.

The PR did not get finally decided on bitbucket, so I'd like to continue here. One suggestion was to use the change proposed here, and another was to completely avoid setting plugin_path in package.xml and use env hooks instead.

Martin Pecka, Nov 2019

An example of a project that uses the suggested plugin_path="${prefix}/lib" is e.g. https://github.com/pal-robotics/realsense_gazebo_plugin . When you launch gazebo with this plugin on ROS_PACKAGE_PATH, the debug log of gazebo_ros.paths_plugin says:

[DEBUG] [1575894134.676531222]: plugin path /opt/ros/cras_subt/src/realsense_gazebo_plugin/lib

That’s apparently wrong, the lib directory cannot be under the sources dir. This will probably be a problem anywhere a devel space is used.

Louise Poubel, Jan 2020

@peci1, it looks like this PR addresses the same concern as f8df1c2 in a different way. What do you think about the other approach, @peci1?

Martin Pecka, Jan 2020

I’m pretty sure there isn’t any single relative path against ${prefix} that would work in all cases. For basic packages that are located directly under src/, the solution suggested in f8df1c2 will work with all of catkin_make, catkin_make_isolated and catkin tools. As well as for install packages. But for packages that are located in some subfolder of src/, that approach wouldn’t work.

According to the source, ${prefix} will always expand to the path to the package. In install spaces, that is share/package_name. So ${prefix}/../../lib is share/package_name/../../lib, which expands to lib, which is correct. But in devel spaces, ${prefix} points to the path to the package source, which may be many levels deep under src/. E.g. for a package located at src/ethzasl_icp_mapping/ethzasl_icp_mapper (to be concrete), ${prefix}/../../lib is src/ethzasl_icp_mapping/ethzasl_icp_mapper/../../lib, which expands to src/lib, which is obviously incorrect. And we could go even deeper…

The solution I propose works in all cases except when there are multiple locations for the same library and the first one found by catkin isn’t correct. But that, I’d say, is an ill-defined scenario. If I get it correctly, catkin_find should respect ROS_PACKAGE PATH.

And, also, my solution doesn’t work on Windows. But I don’t think gazebo_ros_pkgs are supposed to work under windows anyways, are they? Solving this issue on Windows would probably need some work on the side of rospack, e.g. to provide another expansion variable for the lib folder, or for calling catkin_find in a platform-independent way (there’s still the dirname we need to call, but in worst case it could be substituted by appending /.., although I don’t like directory paths like lib/libgazebo_realsense_plugin.so/..).

Louise Poubel, Jan 2020

Thank you for the explanation, I see your point about nested packages. I've never used catkin_find, so I'm not sure about all the drawbacks. One which I can see right now is that the library name must be hardcoded in the package.xml file, which is prone to errors since the name could change on CMakeLists.

In fact, lately I've been seeing env hooks being the recommended way of setting environment variables. They're generated by cmake, so the install paths can be derived directly, and they're introspectable after sourcing the workspace. Maybe that should be what we recommend here as well. I've also seen people who prefer just setting the environment variables directly on launch files.

But I don’t think gazebo_ros_pkgs are supposed to work under windows anyways, are they?

It's my understanding that they work, but I haven't tried it.

Martin Pecka, Jan 2020

You’re right that env hooks might be better for this use case. I’ve never seen ones working on Windows, but maybe Sean can come up with something… Env hooks are definitely closest to the place where the paths are decided/generated. And it would solve nicely problems like ros-simulation/gazebo_ros_pkgs#993 .

[chapulina]

The original PR is backed up here: https://osrf-migration.github.io/gazebo_tutorials-gh-pages/#!/osrf/gazebo_tutorials/pull-requests/552

[peci1]

I could not push to this branch to resolve conflicts, so I created a new PR which supersedes this one: #107.