Updating the minimal required version number for kdelibs, inside kdebase

Andreas Pakulat apaku at gmx.de
Sun Mar 29 22:11:51 CEST 2009


On 29.03.09 21:13:30, Shlomi Fish wrote:
> The attached patch adds a minimal required version number of kdelibs

This is not quite the right list for patches, you should send such a patch to
kde-core-devel next time (AFAIK). I'm cc'ing them.

> to the kdebase module in trunk. At the moment, the cmake process will
> complete successfully with any kdelibs, including that of kde-4.2.x,
> only to result in obscure compilation errors later.
> 
> However, the version numbers in the patch needs to be updated whenever
> there's a new development version of KDE in the trunk. So someone has
> to do it.

That would be the job of the module coordinator for kdebase.

Regarding the actual patch:


> Index: runtime/kioslave/fish/tests/CMakeLists.txt
> ===================================================================
> --- runtime/kioslave/fish/tests/CMakeLists.txt	(revision 946568)
> +++ runtime/kioslave/fish/tests/CMakeLists.txt	(working copy)
> @@ -1,5 +1,5 @@
>  PROJECT( copytester )
> -FIND_PACKAGE( KDE4 REQUIRED )
> +FIND_PACKAGE( KDE4 4.2.68 REQUIRED )

This looks wrong (seems the whole dir was copied from some place where it
was built standalone). There's no need for the find_package here as its
already being searched in runtime.

> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt	(revision 946568)
> +++ CMakeLists.txt	(working copy)
> @@ -13,7 +13,7 @@
>  #
>  
>  # search packages used by KDE
> -find_package(KDE4 REQUIRED)
> +find_package(KDE4 4.2.68 REQUIRED)

I guess the reason for the find_packages in runtime/workspace/apps is for
the split-built thats supported. If that is the case I don't see why a
find_package is needed in the top-level file at all. And the following
calls should be moved down to the three subdirs. (I know thats unrelated to
your actual patch).

> Index: workspace/solid/solid-actions-kcm/CMakeLists.txt
> ===================================================================
> --- workspace/solid/solid-actions-kcm/CMakeLists.txt	(revision 946568)
> +++ workspace/solid/solid-actions-kcm/CMakeLists.txt	(working copy)
> @@ -1,6 +1,6 @@
>  PROJECT (solid-actions)
>  
> -find_package(KDE4 REQUIRED)
> +find_package(KDE4 4.2.68 REQUIRED)

Same as above for the unit-test, there's already a call in workspace so
this one is superflous.

> Index: workspace/plasma/applets/quicklaunch/CMakeLists.txt
> ===================================================================
> --- workspace/plasma/applets/quicklaunch/CMakeLists.txt	(revision 946568)
> +++ workspace/plasma/applets/quicklaunch/CMakeLists.txt	(working copy)
> @@ -2,7 +2,7 @@
>  
>  # building separately or as part of kdebase ?
>  if(NOT KDE4_FOUND)
> -   find_package(KDE4 REQUIRED)
> +   find_package(KDE4 4.2.68 REQUIRED)

Superflous as well.

Apart from that, the change you did will only work for KDE 4.2.0 and later,
KDE 4.1.x will still be found as it didn't support the version argument of
find_package. To make your change work properly for kdelibs 4.1.x or older
also set KDE_MIN_VERSION to an apropriate string.

Andreas

-- 
You will attract cultured and artistic people to your home.


More information about the release-team mailing list