Wrong check for ktorrent in kdenetwork, Was: Re: overriding CMAKE_MODULE_PATH

[Alexander Neundorf]

Hi all, Lukas,

On Monday 16 August 2010, Yury G. Kudryashov wrote:
> Andreas Pakulat wrote:
> > On 16.08.10 14:52:58, Yury G. Kudryashov wrote:
> >> Andreas Pakulat wrote:
> >> > On 16.08.10 12:16:41, Yury G. Kudryashov wrote:
> >> >> Why you don't like the current situation? libktorrent installs
> >> >> FindKTorrent into <prefix>/share/apps/cmake/modules. If it is
> >> >> installed, kdenetwork should find it (if we add CMAKE_PREFIX_PATH ->
> >> >> CMAKE_MODULE_PATH map), else it doesn't find. What's wrong?
> >> >
> >> > Whats wrong is that if ktorrent is not installed there's no way to
> >> > give the user a useful error message. All you get is a 'no
> >> > xxxConfig.cmake found' error from cmake, becuase no FindKTorrent.cmake
> >> > is found (and no KTorrentConfig.cmake either). The point of doing a
> >> > find_package(KTorrent) is to find out wether its installed or not, if
> >> > its not installed you're supposed to give your users a useful message,
> >> > preferably telling them where to get it. When the FindKTorrent.cmake
> >> > is only installed if ktorrent is installed you're making that
> >> > impossible.
> >>
> >> macro_display_feature_log() displays all this information.
> >
> > But only if someone calls it. But that doesn't happen if no FindXXX.cmake
> > is found by cmake. Because doing a find_package(XXX) and having no
> > FindXXX.cmake anywhere is an error for cmake and it stops processing of
> > the cmake-files at that point.

Andreas is completely right, it doesn't make any sense if package <FOO> 
installs a FindFOO.cmake.
It kind of makes sense that we install a whole bunch of these modules from 
kdelibs, since kdelibs provides the basis for all KDE development, so we 
extend the cmake functionality a bit.
Installing FindKTorrent from ktorrent, which is in extragear, really does not 
make sense. The installed file is useless. Even if it is installed into a 
directory where other modules are installed to.
You need your own copy of FindKTorrent.cmake in the software which uses 
ktorrent.

Installing such a file also comes with a non-negligible cost: it should stay 
source compatible, otherwise a new release may break the builds of people.

> kdenetwork calls macro_optionally_find_package(KTorrent), and it doesn't
> stop processing if it fails to find FindKTorrent.cmake.

This is clearly wrong.
http://websvn.kde.org/trunk/KDE/kdenetwork/CMakeLists.txt?view=log
Uh, so we have that in the 4.5 branch ?

Just put the text below in an otherwise empty CMakeLists.txt and put 
MacroOptionalFindPackage.cmake in the same directory:

cmake_minimum_required(VERSION 2.6)
include(MacroOptionalFindPackage.cmake)
macro_optional_find_package(Blub)


This is what I get:

ammer:~/src/tests/optionallyfindpackage/b$ /opt/cmake-2.6.4-Linux-i386/bin/cmake ..
-- The C compiler identification is GNU
-- The CXX compiler identification is GNU
-- Check for working C compiler: /usr/bin/gcc
-- Check for working C compiler: /usr/bin/gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
CMake Warning at MacroOptionalFindPackage.cmake:32 (find_package):
  Could not find module FindBlub.cmake or a configuration file for package
  Blub.

  Adjust CMAKE_MODULE_PATH to find FindBlub.cmake or set Blub_DIR to the
  directory containing a CMake configuration file for Blub.  The file will
  have one of the following names:

    BlubConfig.cmake
    blub-config.cmake

Call Stack (most recent call first):
  CMakeLists.txt:5 (macro_optional_find_package)


-- Configuring done
-- Generating done
-- Build files have been written 
to: /home/alex/src/tests/optionallyfindpackage/b
hammer:~/src/tests/optionallyfindpackage/b$

Since REQUIRED was not used, it is just a warning and not an error.

So, a proper FindKTorrent.cmake has to be added to kdenetwork, commit 1135222 
is wrong, both in trunk and in the 4.5 branch.

Alex