> 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()
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+ - Antonio ----------------------------------------------------------- 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 > >