FindPlasma.cmake obsolete ?

[Alexander Neundorf]

On Sunday 04 January 2009, Modestas Vainius wrote:
> Hello,
>
> sekmadienis 04 Sausis 2009, Alexander Neundorf rašė:
> > in kdelibs/cmake/modules/ there is a FindPlasma.cmake.
> >
> > But plasma has moved to kdelibs and is now found automatically by
> > find_package(KDE4)
> > which sets the appropriate variables like KDE4_PLASMA_LIBS etc.
>
> Why do you want to make kdelibs monolithic so badly? 
> Historically plasma was not part of kdelibs and that didn't hurt anybody.

Historically there was no plasma which was keeping any compatibility.
Now it is part of kdelibs and the necessary variables (except the opengl one) 
are already set anyway by doing find_package(KDE4), which is still required 
when using find_package(Plasma).

> Now that it is supposed to maintain BC, that does not change anything. 

Yes, it matters. We have to keep compatibility to keep source packages which 
were working with KDE 4.1 working with KDE 4.2 without having to change the 
sources. Now that plasma didn't guarantee compatibility before, source 
compatibility in this regard has been broken anyway. A source package which 
was working with plasma from 4.1 will not be working with plasma from 4.2 
independent from FindPlasma.cmake. So, it doesn't matter, compatibility has 
been broken (which is ok, since it was guaranteed). And I'm sure we don't 
guarantee compatibility for unreleased versions, do we ?

That's the file in question:

------------8<----------------8<--------------------8<-------------

# PLASMA_INCLUDE_DIR
# PLASMA_LIBS


set(PLASMA_LIBS ${KDE4_PLASMA_LIBS} )
set(PLASMA_INCLUDE_DIR ${KDE4_INCLUDE_DIR})
set(PLASMA_FOUND true)

find_file(PLASMA_OPENGL_FOUND plasma/glapplet.h
          PATHS ${PLASMA_INCLUDE_DIR}
          NO_DEFAULT_PATH)

mark_as_advanced(PLASMA_INCLUDE_DIR PLASMA_LIBS)

------------8<----------------8<--------------------8<-------------

We are late in the release cycle, I'm doing a quick check whether everything 
is in a releasable state.
The file as it is, is really bad.
It requires that find_package(KDE4) has been executed before without checking 
for that or stating it in the documentation. It doesn't report success or 
failure (hardcoding FOUND to TRUE doesn't count). The mark_as_advanced() is 
wrong (the two variables are no cache variables). It doesn't make anything 
more modular. In order for it to work you need to have a successfully working 
find_package(KDE4) anyway.
The only thing it adds is the check for glapplet. I'd happily add this to 
FindKDE4Internal.cmake, since this is where the plasma stuff is handled now.

> It can still be
> perfectly packaged separately and I think most packagers will prefer it to
> be done that way. Please do not artificially remove this *existing*
> possibility. 
> Read the thread from Gentoo people again about the reasoning behind it.

I understand the reasoning, but the file as it is, is worse than not having 
it. And I don't feel like spending time on improving this for no purpose.
I don't see where the file as it is helps with modularizing.

> What is more, I'm starting to dislike that you are considering doing this
> kind and similar changes so late in the release cycle. Don't you think that
> some sort of freeze should apply to the build system too? 

Yes, I know it's late. I'm doing this in my spare time, and sometimes I have 
more time and sometimes I have less time for KDE. I can't change that. I 
think it's better to do it late then to ship something broken.
I know about the freezes, but, obviously noone complained (except you now), 
and, more important, a lot of these big late changes were simply not possible 
before we required CMake 2.6.2. This was released relatively late in the 4.2 
release cycle. So there were not many viable options: fix the stuff late 
(what I did), or, ship broken stuff ? Have distros maintain huge patches ? 
Have a pile of ugly conditions in our cmake files ? 

> There are numerous packages which do find_package(Plasma) and will need to
> be fixed.

"removing" would actually mean to turn it into:
message(FATAL_ERROR "FindPlasma.cmake is obsolete, use the variables provided 
by KDE4 instead [more details]")

Alex