Review Request 117652: Rewrite FindLibGcrypt.cmake to not use gcrypt-config

Alex Merry alex.merry at kde.org
Sun Apr 20 10:27:08 UTC 2014


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


I think it would be good to still use libgcrypt-config if it finds it and can run it to provide hints to find_library and find_path (in much the same way pkg-config is used for other find modules).

Just to note that if you want to submit it to ECM (which I think is probably a good idea), you'll need to provide an imported target as well.


cmake/FindLibGcrypt.cmake
<https://git.reviewboard.kde.org/r/117652/#comment39140>

    Have a look at FindEGL.cmake in ECM master for what I view as a better documentation style (although that's only something I'm going to demand if you submit it to ECM).



cmake/FindLibGcrypt.cmake
<https://git.reviewboard.kde.org/r/117652/#comment39138>

    The license should be separated from the docs by a completely blank line (no #). Also, (c) is pointless in copyright statements.



cmake/FindLibGcrypt.cmake
<https://git.reviewboard.kde.org/r/117652/#comment39139>

    Please put in the full license (BSD 3-clause).



cmake/FindLibGcrypt.cmake
<https://git.reviewboard.kde.org/r/117652/#comment39137>

    This shouldn't be here (unless the find module really can't deal with older versions).


- Alex Merry


On April 20, 2014, 2:54 a.m., Nicolás Alvarez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117652/
> -----------------------------------------------------------
> 
> (Updated April 20, 2014, 2:54 a.m.)
> 
> 
> Review request for Build System and KDE Frameworks.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> Using the gcrypt-config shell script won't work on Windows.
> 
> I wrote a new find module from scratch. It now searches for the library and include directory the usual CMake way (find_library and find_path). It supports version checking by extracting the version number from the gcrypt.h header.
> 
> By the way, lxr shows there are four different copies of FindLibGcrypt.cmake in different KDE projects and they all have one flaw or another. I think none uses FindPackageHandleStandardArgs, for example. Maybe we should consider putting this one into ECM for kwallet and the others to share.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 9aad9c0 
>   cmake/FindLibGcrypt.cmake 45c0d5d 
>   src/runtime/kwalletd/backend/CMakeLists.txt 4c36767 
> 
> Diff: https://git.reviewboard.kde.org/r/117652/diff/
> 
> 
> Testing
> -------
> 
> Tested on Windows with MSVC2010, cmake 2.8.12, finding emerge-built libgcrypt. Also tested on Debian Linux, cmake 2.8.12.1, finding the distro's libgcrypt.
> 
> 
> Thanks,
> 
> Nicolás Alvarez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140420/2bc01df9/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list