Support CMake-Version-Variables

[Alexander Neundorf]

On Thursday 07 August 2008, Andreas Pakulat wrote:
> On 07.08.08 22:11:11, Thiago Macieira wrote:
> > Andreas Pakulat wrote:
> > >On 07.08.08 21:32:32, Thiago Macieira wrote:
> > >> Andreas Pakulat wrote:
> > >> >On 07.08.08 19:59:52, Thiago Macieira wrote:
> > >> >> Andreas Pakulat wrote:
> > >> >> >Hi,
> > >> >> >
> > >> >> >since we require Cmake 2.6 now I think we should support CMake's
> > >> >> >"integrated" way of version-checking. CMake 2.6 supports this:
> > >> >> >
> > >> >> >find_package(KDE4 4.2.0)
> > >> >> >
> > >> >> >and I'm proposing the attached patch to make this work. This
> > >> >> > removes the need for setting an extra variable before calling
> > >> >> > find_package, still KDE_MIN_VERSION is given authority over the
> > >> >> > cmake-way for backwards compatibility reasons.
> > >> >> >
> > >> >> >Objections against comitting this?
> > >> >>
> > >> >> Are you sure? This reads Boost_*.
> > >> >
> > >> >Damn, sorry. Here's the right patch :)
> > >> >
> > >> >There's one drawback of this whole thing I just noticed: When
> > >> > somebody puts:
> > >> >
> > >> >find_package(KDE4 4 REQUIRED)
> > >> >
> > >> >in his CMakeLists.txt that won't work unless he runs KDE5. I'm not
> > >> > sure how to best fix this, one way would be to require at least
> > >> > MAJOR+MINOR version and else use 3.9 as default. The other would be
> > >> > to "0" as default value for minor.
> > >>
> > >> Then I don't approve of your patch :-)
> > >>
> > >> If the user didn't set anything, the minimum version is 4.0.0 (no one
> > >> should be using pre-4.0 releases anymore).
> > >
> > >Ok, I was thinking the same.
> > >
> > >> If the major is set, it has to be 4. Otherwise, error out.
> > >>
> > >> If the minor isn't set, set min version to 4.0.0. If it is set, set
> > >> minimum version to 4.x.0.
> > >>
> > >> We shouldn't need dependencies on patch-level releases.
> > >
> > >This has a problem, I can't depend on KDE 4.2.0 until its released and
> > >hence modules in trunk/ cannot have a dependency on kdelibs from trunk/
> > >
> > >For example kdevplatform now has a hard dependency on kdelibs from
> > >trunk/, so what I need is require KDE >= 4.1.60 (current version used in
> > >trunk/) not 4.2.0 as that doesn't exist yet and 4.1.0 doesn't work
> > >anymore.
> >
> > Right, sorry. I hadn't thought of pre-releases.
>
> Thanks for doing the logic-thinking for me :) Patch attached, it uses a
> negation of your pseudo-code to avoid having to create new variables.
>
> Andreas

Looks good from the technical side.
Still there is one thing:
right now the way to specify the required KDE version is to set the 
KDE_MIN_VERSION variable before calling find_package(KDE4).

This patch now adds a second way how to specify the required version.
Pro:
-this is how find_package() now supports specifying the minimum version 
number, i.e. in some way the officially recommended way

Con:
-with the patch there are now two ways how to do the same thing (and the old 
way has to stay, in order to stay source compatible)
-several other modules also support FOO_MIN_VERSION or similar variables, so 
using such a variable is currently kind of de-facto standard for specifying 
the minimum version.

Comments ?

Alex