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

[David Cole]

On Wed, Dec 12, 2012 at 4:16 PM, Alexander Neundorf <neundorf at kde.org>wrote:

> 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
> --
>
> 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
>


I strongly agree with Alex here. The union/sum of all of our end user's
perspectives is much more important to consider than our own convenience as
CMake developers.

Mysteriously changing the target_link_libraries implementation to
automatically do a bunch of stuff BY DEFAULT that it didn't used to do
violates the principle of least surprise big time. I don't think it's a
good idea.


D
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20121212/a5159ab0/attachment.html>