Brad King wrote: > On 05/25/2015 08:27 AM, Stephen Kelly wrote: >> it should probably be an error to use TARGET_FILE on an >> INTERFACE target. That doesn't seem to currently be the case. > > We already reject that. I've added a test case to show this in > another topic: > > Tests: Add case for rejecting $<TARGET_FILE:...> on an INTERFACE library > http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=89253992
Great, thanks. >> Could you add a unit test showing what happens when using that genex with >> a target which has this property? > > The changes include a test that it is an error to set the > proposed new property on any target other than an interface > library, and the genex fails as above for interface libraries. Cool. >> Can you make it an error to specify both IMPORTED_LOCATION and >> IMPORTED_LINK_ITEM on the same target? > > This would amount to a separate task of making IMPORTED_LOCATION > an error on an INTERFACE library. It could be done as a separate > topic that would likely need a policy. Actually this is already an error case because IMPORTED_LOCATION is not whitelisted, so nothing more to do. >> Given the 'temporaryness' of this feature, it makes sense to narrow the >> design and use a more-specific name like IMPORTED_SDK_LIBNAME instead. > > I chose "IMPORTED_LIBNAME". Sounds good to me. >> The property should not support an item with a prefix of '-' (ie, CMake >> should unconditionally add the '-l'). Also, it should not be allowed to >> be a full path (should not be allowed to contain a path separator). > [snip] >> So, I don't think IMPORTED_LINK_ITEM should consider this case at all, >> and should be designed with a narrower use case accepting only library >> names and a property name which reflects that. > > Yes. Done. Great, thanks. This looks mostly good to me. + " property value\n " + value + "\nmay not start in '-'."); should be "may not start with ..." I find this strange: An interface library has no library file so linking to it normally adds its usage requirements but does not link to an actual library. The ``IMPORTED_LIBNAME`` property specifies a single library name to be placed on the link line in place of the interface library. It seems that the meaning of INTERFACE libraries is being overloaded with the opposite of their meaning. How about allowing the IMPORTED_LIBNAME for only 'UNKNOWN IMPORTED' libraries instead. That way the meaning of INTERFACE libraries would not be overloaded, and actually calling it an "unknown" library is more accurate than "interface", and it seems more-analogous to IMPORTED_LOCATION. add_library(opengl32 UNKNOWN IMPORTED) set_property(TARGET opengl32 PROPERTY IMPORTED_LIBNAME opengl32) In this interface, I think a special case would have to be made for specifing both properties on one target etc. Also the changes regarding MAP_IMPORTED_CONFIG for INTERFACE libraries can be implemented separately if they make sense (I don't know what's motivating them) instead having them as a side effect of the branch as it is currently. Thanks, Steve. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
