[Kde-pim] Review Request 115318: Fix strigi analyzers not compiling even though strigi is found

Matthew John Dawson matthew at mjdsystems.ca
Mon Jan 27 16:45:58 GMT 2014



> On Jan. 27, 2014, 5:51 a.m., Christophe Giboudeaux wrote:
> > -1 from me.
> > I don't see any need for this change.
> > FindStrigi.cmake does (even in 4.11):
> > find_package(Strigi QUIET NO_MODULE ...)
> > which sets Strigi_FOUND.

I think the problem happens when that line doesn't work, and it falls back to a search and uses FIND_PACKAGE_HANDLE_STANDARD_ARGS.  FindStrigi.cmake doesn't set FOUND_VAR in the call, and the documentation says the default is to prefer an uppercase name.  For whatever reason, Plasma Active triggers this case during the build.

find_package sets both versions, so the most compatible change is to use STRIGI_FOUND.  If preferred, I can go change it in kdelibs to use Strigi_FOUND always.  But I would think that be considered breaking API, as the documented version is all caps.


- Matthew John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115318/#review48364
-----------------------------------------------------------


On Jan. 26, 2014, 1:31 a.m., Matthew John Dawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115318/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2014, 1:31 a.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Repository: kdepim
> 
> 
> Description
> -------
> 
> During Plasma Active work, I discovered the strigi analyzers were no longer compiling or installing.  At some point, the variable for whether strigi is found or not changed its name to be all uppercase.  Fix the top level CMakeLists.txt to account for this difference.
> 
> I have no idea when this change occurred, or why it wasn't noticed earlier.  Under master, strigi is no longer even searched for.  Would it be preferred to just delete the analyzers on the master branch?
> 
> This patch is originally from KDE/4.11, forwarded ported to KDE/4.12.  I'd like to commit to 4.11, 4.12, and master.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt b445e3a 
> 
> Diff: https://git.reviewboard.kde.org/r/115318/diff/
> 
> 
> Testing
> -------
> 
> Just a straight compile was tested on the KDE/4.11 branch, were the plasma active work was being done.
> 
> 
> Thanks,
> 
> Matthew John Dawson
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/



More information about the kde-pim mailing list