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 <prefix>/lib/rtf to LD_LIBRARY_PATH ? #95

Closed
traversaro opened this issue Jun 22, 2018 · 26 comments · Fixed by #393
Closed

Add <prefix>/lib/rtf to LD_LIBRARY_PATH ? #95

traversaro opened this issue Jun 22, 2018 · 26 comments · Fixed by #393

Comments

@traversaro
Copy link
Member

Otherwise, I don't think RTF plugins for YARP are working, see robotology/yarp#1375 .

@drdanz
Copy link
Member

drdanz commented Jul 2, 2018

This is a workaround. I'd rather fix RTF instead...

@traversaro
Copy link
Member Author

In which sense it is a workaround? How do you instruct RTF on where to find its plugins? A manifest file? A RTF-specific env variable?

@drdanz
Copy link
Member

drdanz commented Jul 5, 2018

Setting LD_LIBRARY_PATH is usually considered a bad practice...
I'd like to standardize some mechanism in shlibpp, but I don't know exactly how yet

@diegoferigo
Copy link
Member

I'm interested in this, but I have no idea on how to achieve it without LD_LIBRARY_PATH.

@traversaro
Copy link
Member Author

traversaro commented Jul 5, 2018

@claudiofantacci
Copy link
Collaborator

I'm interested in this, but I have no idea on how to achieve it without LD_LIBRARY_PATH.

Me too! Just came through this "problem" as well while setting up the VM for EPR. I don't really like putting things in LD_LIBRARY_PATH.

@drdanz
Copy link
Member

drdanz commented Jul 13, 2018

I'm interested in this, but I have no idea on how to achieve it without LD_LIBRARY_PATH.

The idea should be to have some configuration or some manifest file to locate the library, and then open it using the full path.

Unfortunately there are quite a lot of issues with this:

  1. Adding further environment variables is probably not such a good idea.
  2. If you write a manifest file, it should not contain absolute paths in order to make the package re-locatable
  3. multi config generators (i.e. MSVC, Xcode) put the libraries in subfolders
  4. on windows, usually, the dll has a postfix d in the name
  5. every system uses custom mechanisms and default paths (just to make a couple of examples, ldconfig is used on linux, the PATH variable is searched on windows)

This is what YARP does, just to make an example.

diegoferigo added a commit to diegoferigo/docker_images that referenced this issue Jul 18, 2018
@traversaro
Copy link
Member Author

I think we should add it until we found a better solution.

@traversaro
Copy link
Member Author

This is what YARP documentation suggests after all https://github.com/robotology/yarp/blame/v3.0.0/doc/rtf_plugins.md#L14 .

@diegoferigo
Copy link
Member

Waiting a smarter solution, which will take time and brain power, I would add it as well.

@traversaro
Copy link
Member Author

Several projects define their own *_PLUGIN_PATH environmental variable:

It would be nice if there could be a consensus among these kind of projects to support a PLUGIN_PATH environmental variable such that it could be just set once for each "non-global" prefix.

However, sticking with the robotology-superbuild problem, we have this problem for two projects:

  • RTF
  • BlockFactory

A possible solution would be if these two projects defined their own RTF_PLUGIN_PATH and BLOCKFACTORY_PLUGIN_PATH environmental variables. Even if the projects use different environmental variables, it would probably make sense to add a FindSharedLibrary-like to sharedlibpp to find an arbitrary plugin given a established search path (that could be customized by specifying the <project>_PLUGIN_PATH env variable name.

If instead we want to try to establish a consensus on a shared environmental variable, we could make it easy to use PLUGIN_PATH for sharedlibpp, hoping that it is not conflicting with any existing env variable. : )

@diegoferigo
Copy link
Member

I had in mind exactly the approach used in ignition. I found it great also for what concern the tests, where the plugin folder inside the build directory was added in the search path automatically, without the need to manually operate on LD_LIBRARY_PATH.

Despite PLUGIN_PATH is appealing, for the time being, at least for what concern BlockFactory, I would proceed with a BLOCKFACTORY_PLUGIN_PATH env variable, maybe matched with custom cmake commands for installing the plugin library in the right folder.

@traversaro
Copy link
Member Author

Despite PLUGIN_PATH is appealing, for the time being, at least for what concern BlockFactory, I would proceed with a BLOCKFACTORY_PLUGIN_PATH env variable, maybe matched with custom cmake commands for installing the plugin library in the right folder.

I totally agree. Do you think anyhow the logic for finding the library can be added to the sharedlibpp ? This may be a quite useful to easily generalize the approach to RTF.

@diegoferigo
Copy link
Member

In robotology/blockfactory#32 I edited the shlibpp version vendored by blockfactory to support the following:

  • A generic SHLIBPP_PLUGIN_PATH env variable to specify extra plugin paths
  • Extending the plugin search path by calling a new method

@traversaro
Copy link
Member Author

For reference, apparently in ROS's world this "finding the plugin path" is handled by pluginlib ( https://github.com/ros/pluginlib ) while most of the logic that we have in shlibpp.
Interestingly, the directory for searching for plugins are found by append lib (or bin in ROS2 and the latest ROS from Microsoft) to some sort of prefi. For ROS1, the CMAKE_PREFIX_PATH env variable is used, while in ROS2 the get_package_prefix's ament_cpp function. See the relevant code:

@diegoferigo
Copy link
Member

Using CMAKE_PREFIX_PATH is actually a good choice, at least for installed projects. In my opinion tho having a *_PLUGIN_PATH variable can be useful for developers whenever you want to use the build tree, just to make an example.

@traversaro
Copy link
Member Author

Using CMAKE_PREFIX_PATH is actually a good choice, at least for installed projects.

I think it really depends on how the project is installed. If the plugin is installed by the .deb package in the /usr prefix for execution in a system that is not used for development, the need for setting CMAKE_PREFIX_PATH (in a system that may not have even CMake installed) seems a bit odd. I guess it could make sense to always (i.e. hardcode) the search in the relevant sub-directories of the path in CMAKE_SYSTEM_PREFIX_PATH (see https://github.com/Kitware/CMake/blob/0479ae492a6bc6058ae86d3ce01ac26fcc71c744/Modules/Platform/UnixPaths.cmake#L24 for the definition in *nix system).

@diegoferigo
Copy link
Member

at least for installed projects

Yes, I meant installed from sources.

If the project is installed from a packaged archive, the plugin might go in something like /usr/lib/x86_64-linux-gnu/ or /usr/lib/x86_64-linux-gnu/<subfolder> as discussed in robotology/blockfactory#34, am I wrong? Also the latter case would not need any env variable to be set, since we could exploit an ld.so.conf-like configuration.

@traversaro
Copy link
Member Author

Great, I did not realized this was already supported by https://github.com/robotology/blockfactory/pull/32/files#diff-ca9447c1aff58642a23f971df7d32a95R138 .

@drdanz
Copy link
Member

drdanz commented Feb 13, 2019

Can we try to synchronize the versions of shlibpp that we have around? AFAIK there are 5 at the moment:

  • robotology/shlibpp
  • robotology/yarp
  • robotology/robot-testing
  • robotology/wbtoolbox
  • robotology/block-factory

@diegoferigo
Copy link
Member

@drdanz You can strikeout wb-toolbox since from the v5 version (current master) it uses the version vendored by blockfactory. Did you have a look by any chance to the improvements of robotology/blockfactory#32?

@traversaro
Copy link
Member Author

I think the changes in robotology/blockfactory#32 are good to be submitted to https://github.com/robotology-playground/sharedlibpp .

@diegoferigo
Copy link
Member

Regardless of the possibility to make shlibpp a dependency of yarp / rtf / etc, merging those features upstream would be a nice addition. Once upstream is aligned, we can add shlibpp to the superbuild, initially as blockfactory dependency.

cc @S-Dafarra is interested in this merge too

@drdanz
Copy link
Member

drdanz commented Mar 14, 2019

Did you have a look by any chance to the improvements of robotology/blockfactory#32?

Sorry, I didn't have time yet... Maybe you could open a PR on shlibpp and we can discuss the changes there?

@traversaro
Copy link
Member Author

cc @Nicogene as he may be interested in all the shlibpp-related discussion.

@traversaro
Copy link
Member Author

A fix for this issue is provided in #393 . A "proper" fix for RTF will be tracked in robotology/robot-testing-framework#109 .

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