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
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
