[PATCH] Port from UsePkgConfig to PkgConfig in kdelibs/cmake/modules

[Friedrich W. H. Kossebau]

Jeudi, le 15 avril 2010, à 22:33, Alexander Neundorf a écrit:
> On Thursday 15 April 2010, Friedrich W. H. Kossebau wrote:
> > Jeudi, le 15 avril 2010, à 21:41, Alexander Neundorf a écrit:
> > > On Thursday 15 April 2010, Friedrich W. H. Kossebau wrote:
> > > > Hi,
> > > > 
> > > > messing around with the buildsystem I stumbled upon the message to
> > > > port away from UsePkgConfig. Trying to gain more experience with
> > > > cmake I produced the attached patches, they seem to work for me.
> > > 
> > > Very cool, thanks a lot :-)
> > > 
> > > > The ones for (K)Exiv2, KDCRaw and Kipi were more complex, did I match
> > > > the logic?
> > > > 
> > > > Okay to commit, or to be improved before?
> > > 
> > > If the pkg-config executable hasn't been found, pkg_check_modules()
> > > does nothing, right ?
> > 
> > No idea, others like FindLCMS use it without checking if it is really
> > found, too. Only FindStrigi seemed to check for that. But I chose the
> > easy template ;)

So, after having a look both practically and theoretically, 
pkg_check_modules() does nothing, because the internal function is guarded 
with "if(PKG_CONFIG_EXECUTABLE)".

Still I decided to no rely on this undocumented behaviour and added a "if 
(PKG_CONFIG_FOUND)" guard around the call of "pkg_check_modules()" now, 
thinking it makes thing more transparent.

> > So that should be approached in another step, if needed.
> > 
> > > FindExiv2.cmake:
> > > set(EXIV2_MIN_VERSION "0.12")
> > > is only in the NOT WIN32 branch, right ? I think at least the setting
> > > of this variable shouldn't be limited to non-Windows.
> > 
> > Was that way before, will move it out of the non-Windows for consistency
> > then.
> > 
> > > FindKIPI.cmake: is KIPI_FOUND set here directly after the results from
> > > pkg-config, i.e. not by checking the results of the find_path/library()
> > > calls ? This should be changed. Can you please also have another look
> > > at the other files whether this is the case there ?
> > 
> > The trick, as found in the old code and kept, is with
> > 
> >       set(KIPI_FOUND PC_KIPI_FOUND)
> >       set(KIPI_VERSION_GOOD_FOUND PC_KIPI_FOUND)
> > 
> > that only on PC_KIPI_FOUND=true the results of find_path/library() are
> > checked and KIPI_FOUND is updated to these results. Means it relies on
> > the negative result of pkg_check_modules already. If this is acceptable
> > the logic seems fine to me.
> 
> This should be changed.
> If pkg-config is there and brings a good result, this good result should be
> preferred.
> If pkg-config does not exist or pkg-config doesn't find what we are looking
> for, we should still try to find it outselves.

Preferred as in "used in HINTS" or chosen? I decided for the later, because if 
we use pkg-config we should also trust it's results, no?
There is just the problem with the(/our?) cmake library handling that we use 
find_library to get the full path to the library, which pkg-config does not 
seem to offer, just data about the library suited as flags for compiler and 
linker.
Any idea how to improve the library stuff to directly take up what pkg-config 
delivers?

So the general pattern (looks like a candidate for a macro) is this, by the 
example of glib-2.0:
--- 8< ---
if (NOT WIN32)
   find_package(PkgConfig)
   if (PKG_CONFIG_FOUND)
     pkg_check_modules(PC_LibGLIB2 QUIET glib-2.0)
   endif (PKG_CONFIG_FOUND)
endif(NOT WIN32)

if (PC_LibGLIB2_INCLUDEDIR)
    set(GLIB2_MAIN_INCLUDE_DIR ${PC_LibGLIB2_INCLUDEDIR})
else (PC_LibGLIB2_INCLUDEDIR)
    find_path(GLIB2_MAIN_INCLUDE_DIR  NAMES glib.h PATH_SUFFIXES glib-2.0)
endif (PC_LibGLIB2_INCLUDEDIR)

find_library(GLIB2_LIBRARIES 
             NAMES glib-2.0 
             HINTS ${PC_LibGLIB2_LIBDIR}
)
--- 8< ---

Other than that now commitable, if only as an intermediate solution?

Cheers
Friedrich
-- 
KDE Okteta - a simple hex editor - http://utils.kde.org/projects/okteta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindGLIB2.patch
Type: text/x-patch
Size: 1553 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0006.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindExiv2.patch
Type: text/x-patch
Size: 2943 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0007.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindKdcraw.patch
Type: text/x-patch
Size: 3072 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0008.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindKexiv2.patch
Type: text/x-patch
Size: 3066 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0009.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindKipi.patch
Type: text/x-patch
Size: 2974 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0010.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindPopplerQt4.patch
Type: text/x-patch
Size: 1285 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20100419/e2538579/attachment-0011.patch