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

[Alexander Neundorf]

On Tuesday 11 December 2012, Stephen Kelly wrote:
> Brad King wrote:
> > On 12/07/2012 06:10 AM, Stephen Kelly wrote:
> >> Stephen Kelly wrote:
> >>> I haven't tried to analyse how much of the complexity is due to whether
> >>> target_use_targets or target_link_libraries is used. I think the harder
> >>> parts of this topic so far have been related to exports.
> >> 
> >> I've split out the part of the commit that changes tll(), and I think
> >> the complexity remains in the part that would be essential even with a
> >> new command.
> > 
> > Obviously there will be complexity inherent to the new capabilities.
> > I think the goal of this discussion is to re-design the interface that
> > enables the new features in order to avoid complexity related to backward
> > compatibility.
> 
> What kind of complexity?
> 
> Complexity of implementation, or complexity for users (having to understand
> that using tll() with targets means not needing include_directories() after
> the patch)?
> 
> Alex's concern is the latter only afaik. 

Yes, I was looking at this only from a users perspective so far.

> I still think that introducing a
> new command is more complex for all users.

Here we disagree.

If these are separate commands, both examples below can be valid cmake code 
for the same installation of package Foo:

find_package(Foo REQUIRED)

include_directories(${Foo_INCLUDE_DIRS})

add_executable(hello main.cpp)

target_link_libraries(hello ${Foo_LIBRARIES})

-----------------------------
as well as 

find_package(Foo REQUIRED)

add_executable(hello main.cpp)

target_use_targets(hello TARGETS Foo:FooLib)


IMO it is a good thing that this way a package can support both the old and 
the new way at the same time, and that using a package the new way is very 
obvious from just looking at the cmake code.
A similar effect can be achieved by adding a keyword to tll(), e.g.

target_link_libraries(hello ${JPEG_LIBRARIES} USE_INTERFACES Foo:FooLib)

This would have the same effect, but people could simply hide the 
USE_INTERFACES keyword in a variable:
set(Foo_LIBRARIES USE_INTERFACES Foo::FooLib)

and then it's again hidden:
target_link_libraries(hello ${JPEG_LIBRARIES} ${Foo_LIBRARIES})

...
> >> Even if we have a policy for when people use the old and new command
> >> with the same foo, people will use both tll() and the new command in
> >> the same project (with different targets), and that will be confusing
> >> for them.

Why will this be confusing ?
Those two commands will have clear and separate functionality.
tll() links, the new one links, sets include dirs and defines using target 
propertiesm and it can error/warn if something else that a target with all 
necessary properties is used there.
 
> > Another idea is to simply not allow both commands to be used on a given
> > target.
> 
> Yes, that's what I means by policy with the same foo. But I guess as it's a
> new command, no policy would be needed.
> 
> I do think that proposal makes the whole thing harder to use, and makes the
> user need to understand a lot more about what's going on under the hood.
> 
> > Since the new command does not yet exist this cannot break any
> > existing projects.  One must either specify everything by the new command
> > or everything by the old command.
> > 
> > We could also use this to switch the default link interface to be empty
> > for shared library targets.  If a newly created target doesn't link to
> > anything then of course its link interface is empty too.  Once one uses
> > the new command to link to something then since it is a new command we
> > can make the link interface empty without changing existing projects.
> 
> True.
> 
> > I like Daniel's name "target_use_interfaces" for the new command rather
> > than "target_use_targets".  The RHS might not always be targets.
> 
> Alex's proposal was that it would only accept targets, not library paths.
> What else do you think would be in the RHS apart from targets?

 
> I don't like the target_use_interfaces name because it conflicts with the
> INTERFACE_LIBRARY type. The name implies that it can only be used with
> targets of that type.
> 
> Anyway, let's discuss the actual name of any new command later.
> 
> For now, can we agree that the only reason to use a new command or not
> depends on complexity for the user?
> 
> Then can someone (preferably not me) please make a list of the types of
> users who will be affected by either choice (and in what situations) and we
> can discuss that?
> 
> Eg, I don't believe most developers of KDE applications will be affected -
> in practice they ignore the buildsystem 

I prefer obviousness over convenience.
While this may lead to more code, it makes the code easier to understand and 
easier to debug, and hopefully less likely to be considered as "magic" and 
thus be ignored. ;-)

> and it is Alex or, more likely, me
> who will port them to KDE Frameworks 5 on a buildsystem level at least. Any
> power users of CMake will learn about any new tll behavior from the release
> notes, and non-power users probably won't notice until the notice by
> accident. New users can follow the documentation or the example of KDE or
> others.

All kinds of users will notice when for whatever reason something suddenly 
doesn't build for them due to bad include dirs.
When they then see

target_link_libraries(hello ${Foo_LIBRARIES} ${Bar_LIBRARIES})

there is no hint that this does more than it always did, so why should they 
even check the documentation whether tll() now also sets include dirs ?
They will just see that their include dirs are wrong in some way, but figuring 
out that this might be caused by target properties hidden in ${Foo_LIBRARIES} 
will probably need an advanced user who knows about this new feature.

This goes away if a new command is used.

Probably it would be good if we could provide also a convenient way to print 
target properties for debugging purposes, something like

cmake_print_properties(Foo::Foo_Libraries PROPERTIES INCLUDE_DIRS BLUB ... )

Alex