Using target names directly vs. variables

[David Faure]

On Sunday 14 July 2013 21:44:38 Alexander Neundorf wrote:
> 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.

I didn't say KDE anywhere, above.

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

Well we're discussing it over and over again, it seems :-)
So let's discuss and finally decide.

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

Again, we can provide ${Solid_LIBRARIES} too, for consistency.

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

But for Qt5 and KF5 we can document that they do.

Killing innovation in the name of consistency is a bit strange -- consistency 
is great, so it's being kept (with _INCLUDES and _LIBRARIES variables) but at 
the same time innovation is made available, for the case where one finds out 
in the documentation or in other examples that it's available.

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

Would have worked for me too.

> 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").

This can lead to confusing error messages indeed. But clearly we're discussing 
this in the wrong forum.

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

We don't need to support both. At some point the frameworks will be splitted 
out, and then this will be the way to use them...

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

= more magic :)

> > 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} ).

This only works if you want to use all libraries from the "package".
Surely this isn't what you want when the "package" provides multiple libs, 
like Qt does with Qt5Core, Qt5Gui, etc., or Boost and its many libs, or 
ffmpeg, or any other package with "COMPONENTS".
At least when building more than one executable, you might want to link some 
things to some libs, and other things to other libs.

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

Stating the obvious? :-)
This argument is behind everyone's view of it.

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

Ah so it's always <package>_<component>_LIBRARY?
 
> > 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.

Any change you make now is also an effort. So this is not an argument for 
either way, that was my point.

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

You're again advocating against innovation in the name of "as we always did 
before". (With such thinking we'd still be using autotools.)

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

To make it clear why you don't need include_directories(). Otherwise it will 
look like a bug to anyone reading the CMakeLists.txt ("Man, you forgot the 
include_directories statements, this is going to break when the package is in 
a separate install dir!").

Different feature -> different syntax -> no ambiguity.

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

They didn't know a better solution would be available one day.

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

You assume very clear symbol names. They aren't always so clear.
Undefined reference to ucal_open_49 -- which lib should you link to?

> 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").

That's really a direct match, isn't?
Maybe we can name the package name KF5Solid? Like Qt names it Qt5Core? 
(right?)

In the end, you and Stephen can decide, I'm "just a user" in the respect of 
cmake files, you guys are the maintainers. I'm just trying to help moving the 
discussion along. But a consensus would be good soon, because Benjamin is 
already asking which form to use and standardize upon in his cleanups.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5