-
Notifications
You must be signed in to change notification settings - Fork 280
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
Prevent reading non-whitelisted properties of interface targets. #916
Prevent reading non-whitelisted properties of interface targets. #916
Conversation
d90b47f
to
f72a26a
Compare
The issue was most likely triggered by an update to VTK 8.1, and not by a cmake update. VTK 8.1 now exports an interface target called |
f72a26a
to
03760ad
Compare
Fixed property name: |
I also had to recursively resolve any imported/interface targets in That does mean that cycles could cause infinite recursion. If that happens, it would appear that cmake segfaults rather than hitting some recursion limit. |
get_target_property(${lib}_imported ${lib} IMPORTED) | ||
if(${${lib}_imported}) | ||
if(${${lib}_type} STREQUAL "INTERFACE_LIBRARY") |
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 couldn't find any documentation about the "string" INTERFACE_LIBRARY
.
Maybe get_property(... SET/DEFINED)
could be used instead if this can't be relied on?
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.
Documentation is here: https://cmake.org/cmake/help/v3.1/prop_tgt/TYPE.html
It first appeared in cmake 3.1, but since older cmakes will never report "INTERFACE_LIBRARY"
, it is automatically backwards compatible.
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.
Ok, thanks for the reference. I was indeed looking at the 3.0 docs.
Thanks for the patch. |
Currently,
catkin_replace_imported_library_targets(...)
can attempt to read non-whitelisted properties of imported interface libraries. After a recent update on Arch Linux this caused atleastpcl-ros
to fail at the cmake step with the below error.My guess is that this is new behaviour introduced in cmake 3.10.2 for which I couldn't find release notes yet, or otherwise caused by an interface library now showing up in
PCL_LIBRARIES
.This PR fixes the issue by detecting interface libraries and replacing them with their
INTERFACE_LINK_LIBRARIES
property instead ofIMPORTED_LOCATION
.On an unrelated note, imported library targets may also have
IMPORTED_LINK_INTERFACE_LIBRARIES(_$CONFIG)
which don't seem to be handled. If that is desired I'd prefer to keep it out of this PR though.Error message without this PR running cmake on
pcl-ros
: