Review Request 112448: FindSamba add PkgConfig fallback

[Alexander Neundorf]

On Sunday 08 September 2013, Mark Gaiser wrote:
> > On Sept. 7, 2013, 8:48 p.m., Christophe Giboudeaux wrote:
> > > cmake/modules/FindSamba.cmake, lines 24-30
> > > <http://git.reviewboard.kde.org/r/112448/diff/1/?file=186323#file186323
> > > line24>
> > > 
> > >     I recommend using PC_SAMBA_INCLUDEDIR and PC_SAMBA_LIBDIR as hints
> > >     for the find_path / find_library calls instead
> > >     
> > >     eg:
> > >     if(NOT WIN32)
> > >     
> > >       find_package(PkgConfig)
> > >       if(PKG_CONFIG_FOUND)
> > >       
> > >         pkg_check_modules(PC_SAMBA smbclient)
> > >       
> > >       endif()
> > >     
> > >     endif()
> > >     
> > >     find_path(SAMBA_INCLUDE_DIR NAMES libsmbclient.h HINTS
> > >     ${PC_SAMBA_INCLUDEDIR})
> > >     
> > >     find_library(SAMBA_LIBRARIES NAMES smbclient HINTS
> > >     ${PC_SAMBA_LIBDIR})
> > 
> > Mark Gaiser wrote:
> >     While it certainly looks cleaner, isn't it a bit strange to do it
> >     like that? I mean, it sounds so strange to let pkg-config search for
> >     samba and then fall back to the other search stuff to again find
> >     samba..
> > 
> > Alexander Neundorf wrote:
> >     The if(NOT WIN32) is not needed. If pkgconfig is not found,
> >     pkg_check_modules() simply does nothing.
> >     
> >     Interesting point.
> >     But I agree with Christophe. That way, i.e. use pkg-config to give
> >     hints to the actual find-calls, the result is in the same form in
> >     all cases, whether it has been found with or without pkg-config.
> >     I.e. the SAMBA_INCLUDE_DIR and SAMBA_LIBRARIES are cached cmake
> >     variables and can be edited by the user. When using pkg-config
> >     results directly, this is different.
> 
> It makes no sense to me.. The way i made it the pkg-config route is being
> taken when the current search stuff fails and either SAMBA_INCLUDE_DIR or
> SAMBA_LIBRARIES is empty. If it is, the fallback will use pkg-config and
> fill those vars if it can.
> 
> In other terms, pkg-config is a fallback! In your proposal pkg-config is
> not a fallback but is always used (when found) just to provide an extra
> hint.. That seems wrong to me.

Hmm, I can't load http://git.reviewboard.kde.org currently...

I understand what you mean.
There are other people, who insist on that if pkg-config found something, this 
should get priority. So doing it that way will simply be more consistent with 
other modules.

When doing it as you propose, which variables end up in the cache ?

Alex