Please update extra-cmake-modules, kdelibs (and plasma-frameworks)

[Alexander Neundorf]

Hi,

back from the weekend...

On Friday 01 March 2013, Stephen Kelly wrote:
> Alexander Neundorf wrote:
> > On Thursday 28 February 2013, David Faure wrote:
> > ...
> > 
> >> ... which means it will also fail at link time, not at cmake time.
> >> So I don't see the point in this change. I'd must rather write
> >> KF5::Solid than ${Solid_LIBRARIES}, where I can never remember if
> >> that's LIBRARY or LIBRARIES, and Solid or SOLID, and where such
> >> variables come from, etc. They look like an unnecessary additional
> >> layer on top...
> > 
> > Yes, they are an additional layer on top.
> 
> An *unnecessary* additional layer :).
> 
> One reason I prefer the target names is that it simplifies debugging by
> removing an unnecessary layer. When I see
> 
>  target_link_libraries(user ${FOO_LIBRARIES})
> 
> and something is wrong, the first thing I have to do is
> 
>  message("FOO_LIBRARIES: ${FOO_LIBRARIES}")
> 
> to see what is hidden inside. Please let's not hide things unnecessarily.
> 
> If instead it is
> 
>  target_link_libraries(user Foo2::foowidgets Foo3::foonetwork )
> 
> I can skip the 'remove an unnecessary layer' step and go directly to step
> two.
> 
> > For all KF5-libraries, the names are <ExactCaseName>_LIBRARIES.
> 
> And the targets are all KF5::<ExactCaseName>. Following a pattern doesn't
> proove usefulness one way or the other.
> 
> I agree with David here and Brad on the CMake list that using namespaced
> imported target names is better. I wish you hadn't made the change to use
> variables like that, but now we at least have the topic in discussion.
> 
> I suggest that we have consensus (yes, it's not unanimous) that we should
> use target names and imported target names instead.
> 
> We're already using imported target names for Qt targets in some of the
> cases. Please don't replace those with variables. Let's finish the job in
> the direction of using imported targets instead.
>
> We're also using target names in many cases for in-build targets. Please
> don't wrap any more of those in variables either.

In case it's not obvious: inside kdelibs, there is, in the current state, for 
technical reasons no other option than using variables. So we do not have to 
discuss that at all.

Currently, kdelibs can be built all together as one, or, alternatively, 
kdeqtstaging and all libraries in tier1/ and tier2/ can be built also 
separately (*1).
The targets have different names when used in-project vs. when using as 
imported targets ("KCoreAddons" vs. KF5::KCoreAddons").
So by converting all uses of libraries which are already in tier1/ or tier2/ 
to use variables, those places have already been prepared for their move to 
tier[123]/.


I also would like to get a concensus on what to  use, that's why I posted this 
issue here.
I don't think it is KDE style to reach concensus by forcing the majority vote 
on the maintainer and just dismiss his arguments.

Until last week I was undecided, now that I worked more with them and thought 
more about it I see more advantages for using variables than for target names 
directly. If you can convince me that they are not valid, fine.


So here are the pro's and con's I see for imported targets:

Pro's 

* It makes the imported targets more visible, so cmake users will learn faster 
about them. This is a good thing.




Con's

* Using imported target names directly extends the scope of cmake source 
compatibility. Anything which makes more stuff significant for compatibility 
is bad, e.g. that's why we have d-pointers in C++.
By using the target names directly, the scope of cmake source compatiblity 
extends from the variables which are defined in a Find-module or a 
Config.cmake file, to also include the names of the targets in the used 
project. So that additional layer is useful.

* First sentences from the documentation of find_package():
"Finds and loads settings from an external project. ... When the package is 
found package-specific information is provided through variables documented by 
the package itself."
So the public interface of find_package() *is* the set of variables it 
defines. Simply put, by using variables, we use the interface as defined by 
find_package(), while by using target names, we ignore the defined interface 
and use string literals, independent from any find_package() call.
So that additional layer is not only useful, but actually the documented way 
how to use the results from find_package().

* By using variables, the way to use find_package() stays the same. This is 
good. Requiring users to learn something new is not good if there is no good 
reason for it.

* For the naming of variables which hold the result of a find_package()-call 
there are strong recommendations (I don't dare to say standard, but I could), 
see readme.txt, for naming targets there aren't.
So if you do 
find_package(Something)
if the Find-module or Config.cmake file follows the recommendations, the 
result of a successful find_package() call will be ${Something_INCLUDE_DIRS} 
and ${Something_LIBRARIES}. There are a few variations from that, like the 
same but ALLUPPERCASE, and using _INCLUDES or _LIBS. Those together cover 
probably over 90 percent of the existing Find-modules.
All packages coming from KF5 will follow the recommendations, many others 
follow them too or at least closely.
There are ZERO recommendations how a developer should names his targets in 
relation to the Config.cmake file he will install, and there are ZERO 
recommendations how he should call the namespace. There is not even an 
official recommendation to use a namespace at all when exporting.
So from find_package(SomeLib) you cannot even guess what the name of the 
imported target will be (looking beyond KF5 and Qt5), it may be "Some::Lib", 
or "somelib", or "some" or anything else.
So, this is not "following a pattern or not", this is more basically whether 
some kind of standard for naming those things exists, it does for variables, 
it does not for targets and namespaces.

* By using imported target names, there is the chance to add a second meaning 
to the error message "ld: cannot find -lsolid". This is not desirable.
I have talked with many people who like or dislike CMake in the last 7 years, 
those that dislike it find enough issues they complain about, I don't think it 
is necessary to add another one which they can use to ridicule cmake "CMake, 
the buildsystem that makes up library names to confuse its users". This is not 
necessary, I don't know why this should be our preferred option.

* If we stay with the convention as defined by the find_package() 
documentation, whenever a developer sees e.g. a
target_link_libraries(... ${Stream_LIBRARIES})
, he can assume that there is somewhere a find_package(Stream) call. This is 
good. If there is instead
target_link_libraries(... stream)
there is no indication whether this is an in-project target or an imported 
target, or whether it's just the name of the library and happened to work on 
the original developers machine because it was installed in a standard 
location. So I see an advantage here too for using variables.

* Since currently it must be variables inside kdelibs, it would be somewhat 
inconsistent and maybe confusing for our developers to do it differently in 
plasma-frameworks. This may be a temporary situation, but it is what we 
currently have and which will last for a few months more.



Neither option is more "elegant" or "pretty" or requires significantly more 
typing or anything like that, so that doesn't matter.

The only real strong, forcing argument in that whole list is that currently in 
kdelibs it must be variables, all others are much weaker, on both sides, I 
know that.



I respect David a lot, but in this regard I consider his opinion as the 
opinion of a KF5 developer who wants to have a buildsystem as convenient as 
possible for him when working on KF5.

This is an important POV, but not the only one, and for me not the most 
important one.
There is also the POV of a cmake expert (e.g. Brad and Stephen), the POV of 
somebody who just wants to build some package he downloaded, and the POV of a 
developer using cmake with basic cmake knowledge (most developers, including 
KDE developers). All of them are important (maybe the cmake expert not so 
much, because I assume that a cmake expert will figure it out anyway).
I try to take all those into account when coming to conclusions. E.g. 
redefining the meaning of a well-known error message is definitely not a good 
point IMO from the POV of a non-CMake expert, and it's good if this can be 
avoided.


I could start repeating things like "code is read much more often than 
written", or point to the "Convenience Trap" ( http://qt-project.org/wiki/API-
Design-Principles ).
My main points I want to achieve in the buildsystem are:
* it shall be as easy to read and understand as possible. This is very 
different from "it shall require very little work from the developer". It 
includes trying to make things explicit and visible, sometimes preferring a 
local copy (which an app maintainer may dare to modify) over a shared file he 
will never touch, and often involves more typing instead of less.

* it shall be usable as an example for projects outside KDE which look around 
for how cmake should be used (that's why it doesn't matter that in KF5, all 
exported targets have the same prefix and the library has the same name as the 
package. This is simply irrelevant for other projects).



Alex

--

(*1) Ok, variables are not the only existing option, but the only viable 
option. Other alternatives are e.g.

- using something like target_link_libraries(... ${KF5PREFIX}KCoreAddons )
but I don't see where this would be better
- using only the in-project targets, and have only the all-in-one build 
working until the one day where we make the big switch, and from then on 
everything can be built separately and only separately, but this is IMO not 
feasible, at least I don't want to work in such a way, stuff will be broken 
for quite some time, while being able to have both ways working in parallel 
makes for a smooth transition