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
export library path for library interfaces #135
Conversation
While this patch work for e.g. I am not yet sure how we can get information about the targets from the export names... |
4b30ab4
to
dd707cc
Compare
I updated the patch to introduce the option Please see ament/ament_index#30 for a usage example. |
@stonier / @clalancette can either of you try / review this? |
I'm trying it out now, will report back in a bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this out with the problematic packages, and found that adding HAS_LIBRARY_TARGET
fixes the issues we encountered. I have one comment about improving the documentation, but it otherwise looks good to me, so I'll approve.
@@ -19,6 +19,9 @@ | |||
# ``install(TARGETS ... EXPORT name ...)``. | |||
# The ``install(EXPORT ...)`` invocation is handled by this macros. | |||
# | |||
# :param HAS_LIBRARY_TARGET: if set, an environment variable is being defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a bit clearer as:
:param HAS_LIBRARY_TARGET: if set, an environment variable will be defined
so the library can be found by runtime executables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My text contains a wrong word: executables
was probably a left over from previous iterations.
For your suggested text the question would then be: what are "runtime executables"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering the same thing, I think I mistook the meaning. How about:
:param HAS_LIBRARY_TARGET: if set, an environment variable will be defined
so the library can be found at runtime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in c0075ba.
Thanks @clalancette. Been meaning to catch up on this each night this week, just never got there. |
Fixes #132. Connect to #132.
CI builds (without repeating tests):