Using target names directly vs. variables

Tue Jul 23 16:29:45 UTC 2013

On Monday 22 July 2013, David Faure wrote:
> 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.

I'm getting really tired of this discussion.

I seriously do not see how writing "KF5::Foo" instead ${Foo_LIBRARIES} is more 
"innovative".

In the end it works just exactly the same way, none of the two will go into 
some "legacy" code paths or whatever.

Our goal is to be able to build the frameworks standalone.

I don't like to say it that way, but the only one who actually worked on this 
until now, i.e. actually building the frameworks in tier1/, tier2/ and some in 
staging/ standalone, is me.

There is more work to that than just replacing these strings in the 
target_link_libraries() calls. Missing included files, missing properties, 
etc.

This work was only possible at all by using the ${Foo_LIBRARIES} variables.
Without them, we have to wait until the day everything is split up, and then 
start fixing the builds.
Don't count on me that I will have time at this specific day to do the work, 
while everybody else is waiting to get it into a buildable state again.

So in case it's not clear: all libraries in tier1/, tier2/ and some in 
staging/ could *as of today* be built, installed and packaged separately (if 
kdeqt5staging would also be built separately).

If you decide to switch to targets, you break these separate builds now, and 
I'll stop working on this until, I'm not sure, probably the day when the big 
separation happens.

I also don't like how such changes are handled here in KF5 in general.

There was no proposal sent to this list whatsoever to use the targets 
directly. We never did that before, so I would have expected a discussion 
about this. Point me to it if I missed it. And I mean, not after the fact.

There was no proposal whatsoever to depend on unreleased cmake versions. It 
was always important to me to make life for KDE developers easy, by not 
forcing them to update cmake often, and especially by not forcing them to 
build from unreleased cmake sources. Since 2006, we never ever dependend on 
cmake git/cvs, and we only depended on betas in the very beginning when 
switching to cmake.
If I missed the proposal to change this, please point me to it. I think there 
was none.

Back to the variables.

It seems the argument that actual cmake users are complaining that there is no 
standard set of variables somehow doesn't count here. For me, it does count.

For "innovation": how about introducing a ${Foo_TARGETS} (or ${Foo_TARGETS} ?) 
variable for packages which export fully featured targets ?
It could be made a convention by cmake, that if a package sets a 
${Foo_TARGETS} variable, that these targets must contain all necessary target 
properties. Maybe this could even be checked by find_package() automatically.
For the reader it would be clear that this thing contains targets.
The naming of the variable can follow a convention, without forcing developers 
to change the names of their targets within their projects.
In kdelibs it could help me, no, enable me to continue the work on building 
everything separately without causing disruptions for the developers.

On the downside, there would then be packages which set _LIBRARIES and 
packages which set _TARGETS. Or both. Not that good, but at least the name of 
the _TARGETS variable can follow a convention, and as I said, it should be 
possible to actually enforce the full-featuredness by cmake in find_package().

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

Yes. And it's also kind of too late.

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

As I said above, if we want a smooth transition, we do need to support that 
now and until the split happens. I think this will be several months more.

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

Come on.
Setting well known variables is magic ?

Keeping the magic down is one of my main goals.
To me "no magic" translates to "everything obvious".
I'm trying my best to remove magic. That's what I'm arguing for.
And this is not only my personal opinion, have a look at Sune's emails in this 
thread here back in November: http://lists.kde.org/?t=135297140600008&r=1&w=2
He echoed exactly what I expected to here from users, so I do expect that his 
views there apply for the majority of developers.

Back then it was, at least to Sune, and to me, "magic", basically due to 
Stephens "minimal" approach.
I said it before, others are saying it too: code is read much more often than 
it is written. So for me, it is more important that it is easy to understand 
and maintain, than to save a few words or lines when writing it.

I also still have the impression that I'm the one who took care of the 
kde4support issue, that I'm the one who took care of Sune's concerns, that I'm 
the one who is generally fixing trouble.
So please give my opinion some weight, to make my life easier.

What we have now, is, I hope, not magic, but explicit and obvious.
If there are magic things in it, let me know.

Alex