[PATCH] PHONON_INCLUDE_DIR in FindPhonon.cmake ignores $KDEDIR

[Alexander Neundorf]

On Saturday 10 April 2010, Friedrich W. H. Kossebau wrote:
> Vendredi, le 9 avril 2010, à 22:52, Andreas Pakulat a écrit:
> > On 09.04.10 22:45:41, Friedrich W. H. Kossebau wrote:
> > > Vendredi, le 9 avril 2010, à 21:46, vous avez écrit:
> > > > On Friday 09 April 2010, Friedrich W. H. Kossebau wrote:
> > > > > Hi,
> > > > >
> > > > > TechBase [TB] teaches to install Phonon to $KDEDIR (!=$QTDIR). So I
> > > > > have done. But I cannot get kdelibs to compile then, it only finds
> > > > > the system one, which is an older one.
> > > > >
> > > > > [TB]
> > > > > http://techbase.kde.org/Getting_Started/Build/KDE4/Prerequisites#Ph
> > > > >on on
> > > > >
> > > > > FindPhonon.cmake uses
> > > > >
> > > > > find_path(PHONON_INCLUDE_DIR NAMES phonon/phonon_export.h
> > > > >
> > > > >   HINTS
> > > > >   ${KDE4_INCLUDE_INSTALL_DIR}
> > > > >   ${QT_INCLUDE_DIR}
> > > > >   ${INCLUDE_INSTALL_DIR}
> > > > >   ${QT_LIBRARY_DIR})
> > > > >
> > > > > But both KDE4_INCLUDE_INSTALL_DIR and INCLUDE_INSTALL_DIR are empty
> > > > > at this moment,
> > > > >
> > > > > message(STATUS
> > > > > "${INCLUDE_INSTALL_DIR}###${KDE4_INCLUDE_INSTALL_DIR}")
> > > > >
> > > > > gives only ###. What is wrong here? Why are both vars empty?
> > > >
> > > > You could set CMAKE_PREFIX_PATH, this will be prefered over
> > > > everything else.
> > >
> > > Not a prefered solution though, right?
> >
> > IMHO its the correct solution to tell cmake to search for libs in a
> > specific plugin. CMAKE_PREFIX_PATH is the official way with cmake to
> > influence how things like find_package, find_path and find_library work.
>
> Okay, and I see TechBase even tells [TB] to extend CMAKE_PREFIX_PATH with
> the $KDEDIR, like
> 	export CMAKE_PREFIX_PATH=$KDEDIR:$CMAKE_PREFIX_PATH
>
> So in the end there is no need for FindPhonon.cmake to use theKDE4_* stuff,
> not only because the inbalance between bootstrapping (have to tell where
> Phonon sits even if in KDE install dirs) and non-bootstrapping (don't have
> to tell).
>
> Attached patches clean FindPhonon.cmake, also add a message to see which
> Phonon lib is actually used, 

This should be done already by the find_package_handle_standard_args() call, 
including the version which has been found (only done if something changes, 
e.g. on the first run, on subsequent runs it's silent).
Doesn't it ?

> and move the find_package(Phonon) back to the
> other find_packages, where it belongs.

Hmm, I wouldn't commit this.
Two things:
- having these two additional search directories in FindPhonon.cmake shouldn't 
hurt, but in many cases help
- removing search paths could be considered a source incompatible change, 
since now a package which found Phonon before may not find it anymore without 
setting CMAKE_PREFIX_PATH

Alex