Brad King wrote: > We > cannot lift the requirement that IMPORTED_LOCATION be a full path > because that is fundamental to the design of imported targets and > is needed for things like $<TARGET_FILE:...> to work correctly.
Could you add a unit test showing what happens when using that genex with a target which has this property? Come to think of it, it should probably be an error to use TARGET_FILE on an INTERFACE target. That doesn't seem to currently be the case. Can you make it an error to specify both IMPORTED_LOCATION and IMPORTED_LINK_ITEM on the same target? > Instead I propose a new IMPORTED_LINK_ITEM[_<CONFIG>] property to take > the place of IMPORTED_LOCATION[_<CONFIG>] for an imported INTERFACE > library. It will allow one to specify exactly one raw item to be > placed on the link line for the interface library since it has no > location. For the above case this item would be just 'opengl32'. > P.S. My original attempt at allowing link item specification was > much more general. It was an INTERFACE_LINK_ITEMS property on > INTERFACE libraries that could be specified on in-project targets > and exported during installation: > > Allow INTERFACE libraries to specify link items > http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e4adad1e > > I reverted it from 'next' and never merged it to 'master' due to > overlap with possible LINK_OPTIONS/ARCHIVE_OPTIONS work discussed > previously. Instead I propose the more narrow IMPORTED_LINK_ITEM > approach to resolve the problem for FindOpenGL without tackling the > larger design goals yet. Basically IMPORTED_LINK_ITEM completes > the design of IMPORTED_LOCATION as might have been done if interface > libraries existed when imported targets were first created. I discussed this feature off-list with Brad over the last few weeks. I think a remaining problem with this is that is disincentivises acheiving the larger goals. That is probably a small problem though, as there may be other incentives for adding INTERFACE_{LINK,ARCHIVE}_OPTIONS. The other incentives appear to be not-as-strong though. Given the 'temporaryness' of this feature, it makes sense to narrow the design and use a more-specific name like IMPORTED_SDK_LIBNAME instead. 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). > This could also be used for cases like FindThreads and '-pthread'. I looked into this a bit and I don't think that's a good example usecase for this feature. The '-pthread' flag is used at both compile time and link time. On my system it causes '-D_REENTRANT' to be added to the compile line and '-lpthread' to be added to the link line. On other platforms it might add '-D_MT' instead apparently https://groups.google.com/forum/#!msg/comp.programming.threads/NCEpG0EOCCY/gzfXXG6VBv0J (That's an old post though - needs fact checking) On FreeBSD, -pthread may cause use of a different libc instead of linking to the pthread library https://groups.google.com/d/msg/comp.programming.threads/NCEpG0EOCCY/MDqdY3nB9DMJ (That's an old post though - needs fact checking) SolarisStudio does not support -pthread but it does support -mt, which also has the effect of adding -D_REENTRANT and -lpthreads, with version 12.4 at least. XL seems to have a _r invocation mode: http://www.ibm.com/developerworks/library/l-portsolaris/ So, 1) The -pthread option does not necessarily link to the pthreads library. It causes the use of multithreading in toolchain/platform libraries, which may or may not result in linking to the pthreads library. User code wishing to use the pthreads library and its symbols explicitly should still link to it explicitly in the normal way. The 'normal way' might be to use a target provided by FindThreads which has IMPORTED_SDK_LIBNAME property content set to 'pthread' (so that '-lpthread' is added, not '-pthread'). 2) The order of the '-pthread'-like option does not seem to matter. One of the design goals of IMPORTED_LINK_ITEM is to target cases where the order does matter. 3) If the '-pthread'-like option is required for linking, then it is also required for compiling. 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. == Designing something for -pthread and similar requirements == I think this is out of scope for what you are currently designing, but this is relevant because your design is at least related to the potential future INTERFACE_LINK_ITEMS and INTERFACE_{LINK,ARCHIVE}_OPTIONS. Perhaps the way forward for -pthread is {INTERFACE_,}TOOLCHAIN_OPTIONS properties which contain flags to add to the compile line and the link line. Other flags which would potentially be used with such a feature include -std=c++11 (many compilers) -fno-exceptions (several compilers) -tbb (Intel) -mkl (Intel) -ipp (Intel) -xlibmopt (SolarisStudio) and probably others. The '-std=c++11' option can make sense as a compile option alone (if not using the std lib and using Qt instead, or just not using the std lib in the interface of a shared library as Qt does), so it's ok that we currently have a design allows that. Using '-std=c++11' for linking but not compiling does not make sense though, so such a {INTERFACE_,}TOOLCHAIN_OPTIONS would ensure it is used for both compiling and linking: # These targets provided by CMake via the project(CXX) command: add_library(CMake::CXX11 INTERFACE IMPORTED) # We should ensure that this works well with the existing features # so that duplicates or different flags of the same 'group' are not added # to command lines: # Or a higher level abstraction like INTERFACE_CXX_STANDARD: set_property(TARGET CMake::CXX11 PROPERTY INTERFACE_COMPILE_OPTIONS -std=c++11) # Or a higher level abstraction set_property(TARGET CMake::CXX11 PROPERTY INTERFACE_LINK_OPTIONS -std=c++11) add_library(CMake::CXX14 INTERFACE IMPORTED) set_property(TARGET CMake::CXX14 PROPERTY INTERFACE_COMPILE_OPTIONS -std=c++14) set_property(TARGET CMake::CXX14 PROPERTY INTERFACE_LINK_OPTIONS -std=c++14) # TODO: Add COMPATIBLE_INTERFACE_ property to make sure only the 'most # recent' dialect specified in the dependency closure is used. # compile and link with -std=c++14 add_library(foo foo.cpp) target_link_libraries(foo CMake::CXX14) add_library(bar bar.cpp) # Requires -std=c++14: target_compile_features(bar PRIVATE cxx_relaxed_constexpr) # Link with -std=c++14 because of above target_link_libraries(bar CMake::CXX11) add_library(bang bang.cpp) # Requires -std=c++11: target_compile_features(bang PRIVATE cxx_auto_type) # Compile with -std=c++14 because of below. target_link_libraries(bang CMake::CXX14) So, I think even a future INTERFACE_LINK_ITEMS would not be useful for a CMake feature for link flags like -pthread and others like it, and we need something different anyway. == In-placement of link items == There is still a potential gap in CMake for in-placement of link items. It may be the case that for this: target_link_libraries(foo PRIVATE bar bat bang) we want to link to bat in a special way, such as -Wl,--whole-archive /path/to/bat -Wl,--no-whole-archive Specifying that content in an INTERFACE library with INTERFACE_LINK_OPTIONS would not be correct because the content would be added after 'bang' on the link line. We need that content in-placed. Using a genex to in-place the content, such as target_link_libraries(foo PRIVATE bar $<TARGET_PROPERTY:bat,INTERFACE_LINK_OPTIONS> bang) would also not work because tll() does not allow genexes by design, and because it would in-place all interface link options even if it did work, and it is probably not wanted to in-place all of them. So, a solution for that could be something like: add_library(bat LINK_ITEMS -Wl,--whole-archive $<TARGET_FILE:bat> -Wl,--no-whole-archive STATIC bat.c ) where the link items would support (some?) genexes, be exported/imported etc. Importantly, the LINK_ITEMS would not be a 'usage requirement' with everything that means for relevant features in CMake today. The set_property command would not work with it, and it would not get a target_link_items() porcelain command either. It's more analogous to a C++ class constructor parameter. The command get_property(result TARGET bat PROPERTY LINK_ITEMS) would populate result with * The contents of LINK_ITEMS if present, or * The IMPORTED_LINK_ITEM if present, or * The IMPORTED_LOCATION if present, or * The LOCATION That is, it would give the content (possibly containing genexes) that will be used in the link line. A $<TARGET_LINK_ITEMS:tgt> genex could be added to compute the actual content in generation time scopes. There would be no INTERFACE_LINK_ITEMS at all. This also means that LINK_ITEMS would co-exist with INTERFACE_{LINK,ARCHIVE}_OPTIONS without the features overlapping in scope: add_library(bat LINK_ITEMS -Wl,--whole-archive $<TARGET_FILE:bat> -Wl,--no-whole-archive STATIC bat.c ) # https://sourceware.org/binutils/docs-2.25/ld/Options.html target_link_options(bat INTERFACE # bat provides a malloc wrapper that must be used -Wl,--wrap malloc ) The LINK_ITEMS are in-placed, and the INTERFACE_LINK_OPTIONS are appended like other usage requirements. There is more design work to do on a feature like LINK_ITEMS. I raise it now only because I think this means that the name and scope of the IMPORTED_LINK_ITEM/IMPORTED_SDK_LIBNAME feature should be made more narrow so that it may only contain a library name, and not a full path. 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
