Phonon

[Alexander Neundorf]

Hi all,

On Thursday 10 March 2011, Ben Cooksley wrote:
> Hi all,
>
> I have found recently that I cannot build KDE Runtime. In tracing
> this, I ended up with an obscure error:
>
> CMake Error at
> /opt/trunk-kde/kde/share/apps/cmake/modules/MacroEnsureVersion.cmake:95
> (NORMALIZE_VERSION):
>   NORMALIZE_VERSION Macro invoked with incorrect arguments for macro named:
>   NORMALIZE_VERSION
> Call Stack (most recent call first):
>   phonon/CMakeLists.txt:18 (macro_ensure_version)

When calling normalize_version(), the version variable should be put in 
quotes, this way even if it is empty it at least becomes an empty string 
instead of nothing.

NORMALIZE_VERSION( "${found_version}" found_vers_num )

> This error message actually means that PHONON_VERSION is empty.
> Further diagnosing this, I noticed that Phonon recently made changes
> to where they stored the version string, yet they updated their CMake
> files.
>
> Digging further, I discovered that CMake wasn't using the new ones,
> because they are being installed to $prefix/share/phonon-buildsystem/

That's not a good idea. Don't do that. Or only do it if you are really sure 
you know what you are doing.

> instead of the standard location ( $prefix/share/apps/cmake )

Although kdelibs install both cmake find-modules as well as it's own cmake 
info-files there, it is not a standard location in any way. For KDE5 this 
will be different.


I just had a look at cmake/CMakeLists.txt in phonons git. 

Now this does not look good:

set(BUILDSYSTEM_INSTALL_DIR ${SHARE_INSTALL_PREFIX}/phonon-buildsystem/)
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/cmake_uninstall.cmake.in
              ${CMAKE_CURRENT_SOURCE_DIR}/FindAutomoc4.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/FindPackageHandleStandardArgs.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/FindPhonon.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/FindPhononInternal.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/PhononMacros.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/FindQt4.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/MacroEnsureVersion.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/MacroLogFeature.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/MacroOptionalFindPackage.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/MacroPushRequiredVars.cmake
              ${CMAKE_CURRENT_SOURCE_DIR}/PhononMacros.cmake
        DESTINATION ${BUILDSYSTEM_INSTALL_DIR})


Why do you think that Phonon needs to install that many Find-modules ?

This means you have to take care of keeping compatibility for them !
(it's not often that I use the exclamation mark).
Who is your cmake guy who does that ?

Second, it makes no sense for package Foo to install a FindFoo.cmake for 
itself.
So, don't install FindPhonon.cmake.

Sorry, FindPhonon.cmake itself looks bad, delete it and start from scratch:

if (NOT PHONON_BUILDSYSTEM_DIR)
    find_program(PC_EXECUTABLE NAMES pkg-config
    PATH_SUFFIXES bin               
    HINTS
    ${CMAKE_INSTALL_PREFIX}
    ONLY_CMAKE_FIND_ROOT_PATH
    )


There is a FindPkgConfig.cmake, why don't you use that ?

Beside, Phonon itself uses cmake, so install a cmake Config.cmake file. This 
file can contain all the information about the installed phonon you want.
There is absolutely no need for *requiring* pkg-config.
Especially not in a FindPhonon.cmake which is only found if the "searching" 
project already knows where Phonon is.


...

if (PHONON_BUILDSYSTEM_DIR)
    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PHONON_BUILDSYSTEM_DIR})

    if (Phonon_FIND_REQUIRED)
        set(_req REQUIRED)
    endif (Phonon_FIND_REQUIRED)
    if (PHONON_FIND_QUIETLY)
        set(_quiet QUIET)
    endif (PHONON_FIND_QUIETLY)

    find_package(PhononInternal ${_req} ${_quiet})
else (PHONON_BUILDSYSTEM_DIR)
    if (_data_DIR)
        if (Phonon_FIND_REQUIRED)
            message(FATAL_ERROR "ERROR: FindPhonon.cmake not found in 
${_data_DIR}")
        endif (Phonon_FIND_REQUIRED)
    else (_data_DIR)
        if (Phonon_FIND_REQUIRED)
            message(FATAL_ERROR "ERROR: Either pkg-config can not find its 
phonon config, or you are not using a recent enough Phonon version.")
        endif (Phonon_FIND_REQUIRED)
    endif (_data_DIR)
endif (PHONON_BUILDSYSTEM_DIR)

There is the macro find_package_handle_standard_args().
Use that and remove all the code above.

If you have question about how to do things properly with cmake, don't 
hesitate to ask on kde-buildsystem at kde.org or cmake at cmake.org.
This would be much better than spreading such code.

Alex