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

[Windows] strip 'lib' prefix when loading library. #176

Open
wants to merge 2 commits into
base: melodic-devel
Choose a base branch
from

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Feb 25, 2020

This is a follow up of #140.

Many ROS packages are originally targeted for Linux and there is a convention to put the library prefix lib when describing <library path="lib/libplugin">. However, on Windows, the shared library doesn't have such naming convention. As a result, class loader is not able to find the library by names.

This pull request is to propose that class loader should also search the library name without lib prefix on Windows, which is as a mitigation to help downstream packages don't need to modify their plugin XML but still can load the plugin binaries.

This pull request is also fix an little problem that stripAllButFileFromPath() is returning the path separator in the result string. Based on the context of getAllLibraryPathsToTry(), the path separator seems to be an extra.

@seanyen
Copy link
Contributor Author

seanyen commented Feb 25, 2020

@nuclearsandwich This is a rework for #140. And this time I only touch up getAllLibraryPathsToTry() and hope this won't break any interfaces.

@seanyen seanyen changed the title [Windows] lib strip for Windows. [Windows] strip 'lib' prefix when loading library. Feb 25, 2020
@nuclearsandwich nuclearsandwich self-assigned this Mar 5, 2020
@seanyen
Copy link
Contributor Author

seanyen commented Apr 3, 2020

@nuclearsandwich Friendly ping. And any feedback is appreciated. I hope this change won't be slipped for the next pluginlib release window. :)

@seanyen
Copy link
Contributor Author

seanyen commented May 22, 2020

@nuclearsandwich @sloretz Following up. Any feedback would be appreciated.

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

Successfully merging this pull request may close these issues.

2 participants