[cmake-developers] Setting include directories via target_link_libraries() ?

[Stephen Kelly]

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.