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

[ros2] library suffix hack #60

Merged
merged 1 commit into from
Nov 4, 2017
Merged

Conversation

mikaelarguedas
Copy link
Member

workaround for #31

See #31 for original issue description and a potential real fix description.
This is needed for #59 tests to pass in Debug mode

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes it is correct, but this lgtm. pluginlib does some tricks to handle this too.

@mikaelarguedas
Copy link
Member Author

Sometimes it is correct

Yeah it seems correct for many external projects. Out of curiosity, do you know why we don't produce libraries with the trailing d in ROS 2 ? (I mean is it a design decision we made ?)

@wjwwood
Copy link
Member

wjwwood commented Nov 3, 2017

I guess because cmake doesn't do it for you. It's purely a convention that we just didn't opt into. Also it's not required most of the time on Linux and macOS. It's most problematic on Windows which often builds into separate folders anyways. For distribution on Windows specifically it might be useful when you want to side-by-side install the release and debug versions.

@mikaelarguedas
Copy link
Member Author

Thanks @wjwwood for the feedback. Given that we almost always ship a single version of the lib (relwithdebinfo) it makes sense not to bother adding the suffix complexity to facilitate side by side install.

I'll merge this and leverage it in #59 then

@mikaelarguedas mikaelarguedas merged commit 16b37e9 into ros2 Nov 4, 2017
@mikaelarguedas mikaelarguedas deleted the ros2_debug_library_suffix branch November 4, 2017 00:31
return ".dll";
#else
return Poco::SharedLibrary::suffix();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @dirk-thomas :
"Wouldn't it be nice if the debug shared libraries could be loaded for debugging?"

To keep in mind when thinking about addressing #31, it would be great to give users control about what libraries to find and load rather than making the decision for them one way or another

@mikaelarguedas mikaelarguedas mentioned this pull request Nov 7, 2017
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