Review Request 129339: Allow building kwalletd against gpgme++ from gpgme 1.7

[Andreas Sturmlechner]

> On Nov. 5, 2016, 10:39 a.m., Andreas Sturmlechner wrote:
> > kwalletd/CMakeLists.txt, line 8
> > <https://git.reviewboard.kde.org/r/129339/diff/1/?file=484099#file484099line8>
> >
> >     Please do this in reverse logic, same as in the kwallet.git commit you are referring to - that means searching for Gpgmepp first (which is unique to GpgME++) and only if NOT Gpgmepp_FOUND do you search for Gpgme and QGpgme (the latter is both known to kdepimlibs-4 and GpgME++) to make sure there is no bogus linking and to avoid confusion in FEATURE_SUMMARY output.
> 
> Antonio Rojas wrote:
>     That would make it use gmgpe 1.7 by default if it is installed. Is such a behavior change acceptable in a package that is essentially in maintenance mode? Also, cmake would throw "gpgme not found" errors even if the kdepimlibs version is installed, which could be confusing.
>     Note that QGPGME_FOUND is not satisfied by qgpgme from gpgme (at least in 1.7.1), so I see no chance of confusion.
> 
> Andreas Sturmlechner wrote:
>     Whether that's acceptible is for someone else to decide, in my view this is a nice fix to stay in line with KDE Applications 16.12 neighbourhood, since the otherwise required kdepimlibs-4 conflicts with the new GpgME++ bindings... anyway, pretty FEATURE_SUMMARY output is probably secondary.
> 
> Sandro Knauß wrote:
>     That QGpgme is not found with gpgme 1.7.X directly via (find_package) is a bug, that will be closed with 1.7.2. so don't rely at this behaviour. but the name is QGpgme_FOUND should be definded after find_package(Gpgmepp). So you can test if (QGpgme_FOUND or QGPGME_FOUND) to test for QGpgme support.
> 
> Antonio Rojas wrote:
>     Well, kwalletd doesn't need QGpgme, so it seems wrong to search for it (for kdepimlibs there is no choice, since there is no separate gpgmepp cmake file). 
>     So, if I understand correctly, by using QGPGME_FOUND (all caps) there will be no risk that the gpgme version will be picked by mistake instead of the kdepimlibs one?
> 
> Sandro Knauß wrote:
>     Just to sort fings out: kdepimlibs only exists in KDE 4 times (technically it exists longer, but most parts were split out). The repository is called kde/pim/gpgmepp installs both Gpgmepp and QGpgme (for KDE Applications <=16.08).
>     
>     For Gpgmepp (exists in Applications >=15.04 and <=16.08) in kde/pim/gpgmepp you can search for KF5::Gpgmepp and KF5::QGpgme, both should be searchable via find_package.
>     
>     But if you don't use QGpgme, so you should IMO only search for Gpgmepp and rename the HAVE_QGPGME -> HAVE_GPGMEPP. For finding current gpgme (with c++/qt lang bindings). I would write the CMakeLists.txt like following:
>     
>     find_package(Gpgmepp) # for uptodate gpgmepp withing gpgme
>     if( not Gpgmepp_FOUND)
>       find_package(KF5::Gpgmepp) # for version form within kdepim (KDE Applications 15.04 - 16.08)
>       set(HAVE_GPGMEPP)
>     else()
>       set(HAVE_GPGMEPP)
>     endif()
>     
>     
>     if (HAVE_GPGMEPP)
>       add_defintions(-DHAVE_GPGMEPP)  #renamed from HAVE_QGPGME
>       include_directories(${GPGME_INCLUDES} ${QGPGME_INCLUDE_DIR})
>       set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${KDE4_ENABLE_EXCEPTIONS}")
>     endif()
> 
> Antonio Rojas wrote:
>     I think there is some misunderstanding here - this RR is for kde-runtime, ie. KDE4. KF5 kwallet has already been ported. So KF5::Gpgmepp is not relevant here, the options are either use gpgme++ from kdepimlibs (the KDE4 version) or gpgme++ from gpgme 1.7+
> 
> Andreas Sturmlechner wrote:
>     Since we now know that QGpgme is going to be found with GpgME-1.7.2 it would really be much cleaner to move those behind the 'NOT Gpgmepp_FOUND' fence, as said in my initial reply - provided this is OK with kde-runtime maintainers. It is safe to assume that kdepimlibs' QGpgme will then be used, since GpgME/QGpgme requires GpgME/Gpgmepp.
>     
>     That *gpgme* stuff sends heads spinning... ;)

Also builds with current GpgME git master, btw.


- Andreas


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


On Nov. 5, 2016, 9:35 a.m., Antonio Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129339/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2016, 9:35 a.m.)
> 
> 
> Review request for KDE Runtime and Andreas Sturmlechner.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> -------
> 
> This patch, based on the one from the kwallet repo, allows building kwalletd against the gpgme++ library provided by gpgme 1.7 instead of the kdepimlibs one.
> 
> 
> Diffs
> -----
> 
>   kwalletd/CMakeLists.txt 73aec82 
>   kwalletd/backend/CMakeLists.txt 4db348f 
>   kwalletd/backend/backendpersisthandler.cpp b7f63f8 
>   kwalletd/kwalletwizard.cpp 78de78d 
> 
> Diff: https://git.reviewboard.kde.org/r/129339/diff/
> 
> 
> Testing
> -------
> 
> Built:
> 1) with kdepimlibs
> 2) with no kdepimlibs and gpgme 1.7
> 3) with no kdepimlibs and no gpgme
> 
> Builds correctly in all cases, links to the expected version of libgpgme++ in the first two cases. In the first case there is no change in behaviour with respect to the previous situation.
> 
> 
> Thanks,
> 
> Antonio Rojas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20161110/3e15cc69/attachment.htm>