Stephen Kelly wrote:
> I'd like to defer the details of the
> porcelain API for now and focus instead on the plumbing API and
> implementation.
I have several quirks with the new command proposal, The new command can be
emulated with my branch and this macro:
include(CMakeParseArguments)
macro(target_use_interfaces target)
cmake_parse_arguments(TUI "" "" "INTERFACE;PUBLIC;PRIVATE" ${ARGN})
if(TUI_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "Unknown keywords given to target_use_interfaces():
\"${TUI_UNPARSED_ARGUMENTS}\"")
endif()
foreach(lib ${TUI_INTERFACE} ${TUI_PRIVATE} ${TUI_PUBLIC})
if (NOT TARGET ${lib})
message(FATAL_ERROR "This macro only accepts targets")
endif()
endforeach()
if (TUI_INTERFACE)
target_link_libraries(${target}
INTERFACE_LINK_LIBRARIES ${TUI_INTERFACE})
endif()
if (TUI_PRIVATE)
target_link_libraries(${target} LINK_PRIVATE ${TUI_PRIVATE})
endif()
if (TUI_PUBLIC)
target_link_libraries(${target} LINK_PUBLIC ${TUI_PUBLIC})
endif()
endmacro()
The quirks:
1) Restrictiveness
Only targets are allowed, which means you likely can't use ${Foo_LIBRARIES}
at all anymore, even for porting, unless you know exactly what it contains.
Even if you do know exactly what it contains, that doesn't necessarily help.
Consider that Bar maybe doesn't provide 'rich' imported targets yet. Then
Foo will have this:
set(Foo_LIBRARIES Foo::importedlib ${Bar_LIBRARIES})
Or worse, FooConfig.cmake will create IMPORTED targets for Bar (Bad idea,
see 5 below)
This is solvable by letting target_use_interfaces also accept library names
and library paths, but then the new command is less 'exact' and more
redundant.
2) Inconvenience
Assuming target_use_interfaces only allows targets, you'll have a lot of
this:
tll(foo lib1)
target_use_interfaces(foo lib2)
tll(foo lib3)
instead of just
tll(foo lib1 lib2 lib3)
3) Incompleteness
link-libraries, INTERFACE_POSITION_INDEPENDENT_CODE,
INTERFACE_WIN32_EXECUTABLE, INTERFACE_STD_CXX_11,
INTERFACE_CXX_RUNTIME_LIBCXX, INTERFACE_CXX_RUNTIME_LIBSTDCXX
will all handled by tll in both proposals (as they depend on the
LINK_LIBRARIES, which is populated by tll), but defines and includes alone
will be the INTERFACE properties which do not get populated with tll, and
need the new command instead.
4) Redundancy into the future
Some Bar packages will not create IMPORTED targets with appropriate
INTERFACE properties.
tll will not 'go away'. The extent to which a project can achieve a 'fully
ported' state depends on its upstreams. The new command may drive some (but
not most) people to read the help entry for it for a couple of releases.
Depending on how fast their cmake dependency moves, how fast they move their
upstream dependency versions and how fast their upstream dependencies move
their cmake version, the new command could be very old news by the time they
get to use it.
And after that, there will always be two commands for very similar things.
Not every good idea is adopted (particularly not in a reasonable time), no
matter how good the idea is. Consider the amount and rate of adoption of
Config files to date.
5) Greater incentive to make future changes accidentally harder.
People will create their own IMPORTED targets to 'port fully' to the new
command and push themselves over the line (both in their own CMakeLists
files, and in their own Config files).
The same thing could happen even if tll does includes and defines too, but a
greater incentive is there if there's a new command and a 'port fully to the
new command' goal.
That means that if upstream introduces IMPORTED targets where they didn't
before there will be breakage.
6) API non-argument
One of the suggested benefits of a new command is a more restrictive
signature to be more typo safe.
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5526/focus=5569
If that is really such a problem for tll that it needs a solution (I don't
believe it is) we can just deprecate
tll(foo lib1 lib2)
with a policy and require people to use
tll(foo LINK_INTERFACE_LIBRARIES lib1 lib2)
tll(foo LINK_PRIVATE lib1 lib2)
tll(foo LINK_PUBLIC lib1 lib2)
instead if the policy is NEW. (and also
tll(foo INTERFACE_LINK_LIBRARIES lib1 lib2)
later, when that exists of course - The deprecation policy can be
implemented immediately for the next CMake release).
7) Complexity non-argument
The implementation complexity argument leveled against using tll to set
includes and defines is just not true. The patch to add it is very simple
('Add includes and compile definitions with target_link_libraries') where it
touches the tll command implementation.
Implementing a new command would probably be more complex as tll may have to
be refactored for code sharing.
8) Non-discoverability
No one will use/benefit from/discover the new feature (with regard to
includes and defines) without first explicitly using the new command. No one
will discover it accidentally. Maybe we can add a note about it to the tll
documentation, but if people are going to read that, then it is just as easy
for us to write 'tll also populates includes and defines from its
dependencies'.
9) No benefits, only burden
The stated benefits of the new command have been easier backward
compatibility (I disagree), more readable (not true, see 2), having a goal
of a 'fully ported' state (not achieveable, see 4), less complexity (not
true, see 7), easier debugging (not true, as previously shown -
CMAKE_DEBUG_TARGET_PROPERTIES is the answer instead), discoverability (I
don't think so, see 4, 8).
The only thing a new command provides is a porting and maintenance burden,
command-similarity-confusion and less discoverability.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
