Using target names directly vs. variables

[Alexander Neundorf]

On Sunday 14 July 2013, David Faure wrote:
> On Sunday 14 July 2013 11:42:26 Alexander Neundorf wrote:
> > A good step forward to improve the experience for the user is to follow
> > those naming conventions as good as possible, so that if you do
> > 
> > find_package(WhatEverItIs)
> > 
> > you know, that you can use ${WhateEverItIs_LIBRARIES} and
> > ${WhatEverItIs_INCLUDE_DIRS} later on without looking at the
> > documentation for this specific module.
> 
> Yes, this is "the old way" :-)
> 
> With Qt5 and KF5, however, there's no need for the _INCLUDE_DIRS variable;
> linking to the target name brings the include dirs automatically. It works
> differently, so it's not crazy to use a different syntax for it.
> Different feature -> different syntax.
> 
> To anyone only knowing the "old way", it would actually be quite confusing
> to see cmakelists using ${Foo_LIBRARIES} and NOT using
> ${Foo_INCLUDE_DIRS}.
> 
> We could still set the variables, of course, for the principle of less
> surprises you mention, so that app developers can use the variables the
> "old way". But this doesn't mean we have to use these variables
> internally.

Really, this is not about KDE-only.
There is, AFAIK, no official "new way".

There is, AFAIK, the one documented, standard way, and there is Stephens 
"let's use the targets directly"-way he just committed without any proposal or 
discussion about that whatsoever I can remember.

I find it cool that targets can have their include directories attached, no 
doubt about that.

I have heard so many times the complaint from users, not speaking specifically 
about KDE developers here, that it sucks that they can't rely on standardized 
variable names/results from find_package().
With using target names directly we make that even worse, instead of improving 
it.

Then we have three different ways, no standard *at all*:

find_package( Solid )  -> use "KF5::Solid"
find_package( JPEG ) -> use ${JPEG_LIBRARIES}
find_package( ALSA ) -> use ${ALSA_LIBRARY}

instead of almost standard, with just a few variations:

find_package( Solid )  -> use ${Solid_LIBRARIES}
find_package( JPEG ) -> use ${JPEG_LIBRARIES}
find_package( ALSA ) -> use ${ALSA_LIBRARY}

Also, there is no guarantee that an imported target actually has its include 
dirs attached. This also depends on the package in question. So using the 
target name directly is also no indication that include_directories() is not 
necessary for this package.


From what I heard from developers in the last years, this is something which 
would make many of them much more happy with cmake.
In my books this would be a good thing.


You say "new feature -> new syntax". I would have liked that a lot. David Cole 
(ex-Kitware) too. Our concerns were dismissed.
I would have much prefered if it would have to be explicit that the thing you 
link against is a target and that you expect it to have its include 
directories attached. This would have made it obvious to the reader and it 
would have enabled cmake to generate proper error/warning messages if 
something goes wrong, something like

target_use_target(hello KF5::Solid  KF5::KArchive)

Then it would be obvious that those two items are targets, no guessing 
involved. CMake could also have warned/errored out if those targets would not 
have include directories attached. It could have errored out if no such 
targets exist.
But it was decided to stay with target_link_libraries(), where any of the 
arguments can be either a linker flag ("-fPIC"), or a library flag ("-lfoo"), 
or a target ("Solid") or if no such target exists it's used as the basename of 
a library (-> "-lSolid").

 
> > OTOH it has a couple of disadvantages:
> > * the developer first has to check the documentation of the specific
> > module whether it provides the standard variables or only the imported
> > targets
> 
> Right. Let's provide both.

Of course. The targets are there anyway.

Using the target names directly also means just that. E.g. until last week in 
plasma-frameworks the kde4support library was referred to as 
"KDE4__kde4support". This doesn't look to me like a clean way to use the 
imported target. The "KDE4__" prefix is actually just an implementation 
detail, nobody should have to know about. I changed those places to use 
${KDE4Support_LIBRARIES}. Now plasma-frameworks is done.
Once kde4support can be built separately, no change will be required in 
plasma-frameworks. It will be possible to build plasma-frameworks both with 
the all-in-one kdelibs, and also with the separately built frameworks.
So these variables also add am abstraction layer which protects the users of a 
package from name changes of the targets in the used project.


> > * if a package provides only the imported targets, there is no naming
> > standard for targets, so the developer has to read the documentation for
> > each package to find out what the name of the imported target is. This is
> > a step backward, not foward. Stephen agrees that he does not see a
> > standard way to name targets in relation to their cmake package names.
> 
> Well, one needs to find the name of the package too, in the first place.
> Should I find_package(attica) or find_package(Attica) or
> find_package(LibAttica)?
> At some point, someone needs to read some documentation.
> In fact, I'm confused. If there's multiple libs in the package (e.g.
> kiocore and kiowidgets), with a single find_package(KIO COMPONENTS core
> widgets) call, then with variables the same problem happens, surely a
> KIO_LIBRARIES isn't good enough.

In KDE we are in a somewhat special situation, especially right now.
People working on KDE frameworks now the framework libs and adapt the using 
modules accordingly.
Typically, a user of a library is not a developer of the library, and does not 
know what the names of the targets are. Typically, he wants to do 
find_package(Foo) and then target_link_libraries(... ${Foo_LIBRARIES} ).
I am not making this up, I have heard that many many times, on mailing lists 
and in real life.
More or less nobody cares whether the build system is cool, for more or less 
everybody it should just work and get out of the way.

For KIO, there will be ${KIO_LIBRARIES}, and ${KIO_Core_LIBRARY} etc., as 
recommended in cmake's readme.txt.


> > * if we use target names now directly in kdelibs, there will be breakage
> > when moving them to separate builds, and a big switch day. Within
> > kdelibs, the target name is e.g. KService. When exporting them as cmake
> > targets, they get a "namespace" prepended, to give a strong hint that
> > the thing you see is an imported target. So the name of the imported
> > target is "KF5::KService". When moving a framework out of kdelibs, all
> > places where this target is used have to be changed accordingly. We
> > avoid this work (basically thanks to the standard naming of variables)
> > if we right now start to use
> > ${KService_LIBRARIES}. This can be set to "KService" right now in the
> > toplevel CMakeLists.txt in kdelibs. Inside the framework, there will be
> > find_package(KF5 .... COMPONENTS KService)
> > or
> > find_package(KService)
> > which will set, as every find_package() should do, the
> > ${KService_LIBRARIES} variable, and there will be no additional work and
> > no breakage and no switch day when finally separating the frameworks out
> > of kdelibs. This is done now already for all frameworks which are
> > already in tier1/ and tier2/, they cab right now be built as part of
> > kdelibs or as a standalone project.
> 
> I can write a perl script to replace KService with KF5::KService (and same
> for the other targets) in target_link_libraries lines in 15 minutes tops.
> Changing A to B now so that we don't have to change A to C later doesn't
> save anything (same script...).

It doesn't matter whether it takes 15 minutes or more.
It is still an effort which is simply not necessary.

> > * I don't see an advantage in having to teach our developers that for KDE
> > packages they have to use imported target names (including Qt5), while
> > for all other packages they can follow what they know from cmake for
> > years, and continue to use ${Foo_LIBRARIES}.
>
> See above - it's different because it takes care of the include dirs too.

If we continue to use the variables, they don't have to learn anything new. If 
somebody wants to, he can find out that he doesn't need the 
include_directories() calls anymore. If he doesn't care, everything still 
works as before. Why change this just for the sake of changing.
 
> > * I have never heard a complaint from a cmake user that he can't use
> > imported target names directly but has to use variables for external
> > packages. So I don't see any motivation to change this.
> 
> The complaint is: if I use a variable that is not set, it is expanded to
> nothing, and all I get is undefined symbols. If I use a target name that is
> unknown, I get an error about that very target name, much easier to debug.

I basically asked for a complaint, so you found something, no surprise. Nobody 
ever was annoyed by this enough (or at all) to complain about that. Really.

If you get undefined references to solid_something, and the 
target_link_libraries() call contains ${Solid_LIBRARIES}, it seems quite 
obvious to me where to start looking.

I would have a different opinion if there was a direct match between package 
name and target names. There isn't, not even in KF5 ("Solid" -> "KF5::Solid").

Alex