[PATCH] PHONON_INCLUDE_DIR in FindPhonon.cmake ignores $KDEDIR

[Friedrich W. H. Kossebau]

Samedi, le 10 avril 2010, à 21:03, vous avez écrit:
> 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 ?

Ah, was distracted by the fact that there is a log for Qt and a lot of other 
stuff on subsequent runs, still.

Is the decision which shows always a log random and historical or is there a 
reason for the different behaviour?

> > 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

Hm, increasing a needed version number is also a source incompatible change ;)

Alright, I would have gone for cleaned up complexity, but you surely have your 
reasons for a greater focus on backward compatibility, so consider this 
patches rejected :)

Cheers
Friedrich
-- 
KDE Okteta - a simple hex editor - http://utils.kde.org/projects/okteta