[Kde-pim] Review Request 120959: [KPimUtils] Don't install headers into $include_dir/KPIMUtils/KPIMUtils
Martin Klapetek
martin.klapetek at gmail.com
Mon Nov 3 22:54:19 GMT 2014
> On Nov. 3, 2014, 8:59 p.m., Christophe Giboudeaux wrote:
> > KPIMUtils/KPIMUtils/SomeHeader is correct. This patch isn't (and the facebook resource builds fine without this patch).
> >
> > KF5PimUtilsTargets.cmake sets this:
> > set_target_properties(KF5::PimUtils PROPERTIES
> > INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/KF5/KPIMUtils;${_IMPORT_PREFIX}/include/KF5"
> > )
> >
> > so, example:
> > #include <KPIMUtils/Email> (ie (install prefix)/include/KF5/KPIMUtils/KPIMUtils/Email) won't have troubles finding kpimutils/email.h (ie (install prefix)/include/KF5/KPIMUtils/kpimutils/email.h)
>
> Martin Klapetek wrote:
> Well nothing in current Frameworks5 collection installs headers like this, so I'm not sure why kdepimlibs is/should be different. I'll add the frameworks group to weigh in.
>
> > and the facebook resource builds fine without this patch
>
> Fwiw, the facebook resource is disabled in master and it in no way builds fine, there are many other build errors, this is just one of them :)
>
> Christophe Giboudeaux wrote:
> ls /kde/inst/5/attica/include/KF5/Attica
> attica Attica
>
> ls /kde/inst/5/baloo/include/KF5/Baloo
> baloo Baloo
>
> ls /kde/inst/5/kdeclarative/include/KF5/KDeclarative
> kdeclarative KDeclarative quickaddons QuickAddons
>
> ls /kde/inst/5/kdesu/include/KF5/KDESu
> kdesu KDESu
>
> and so on...
>
> The first KPIMUtils is the module name, that's consistent with all the current frameworks
> The second one is there to give a hint that the include should be KPIMUtils/PrettyHeader and not just PrettyHeader.
>
> That's why you can't see this for all frameworks.
>
> Martin Klapetek wrote:
> But then there is
>
> $ ls /opt/kde5/include/KF5/KArchive/
> K7Zip KAr KArchiveDirectory KArchiveEntry karchive_export.h karchivefile.h kar.h kcompressiondevice.h kfilterbase.h kfilterdev.h ktar.h KZipFileEntry kzip.h
> k7zip.h KArchive karchivedirectory.h karchiveentry.h KArchiveFile karchive.h KCompressionDevice KFilterBase KFilterDev KTar KZip kzipfileentry.h
>
> $ ls /opt/kde5/include/KF5/KNotifications/
> KNotification knotification.h KNotificationPlugin knotificationplugin.h KNotificationRestrictions knotificationrestrictions.h knotifications_export.h KNotifyPlugin knotifyplugin.h KPassivePopup kpassivepopup.h KStatusNotifierItem kstatusnotifieritem.h
>
> $ ls /opt/kde5/include/KF5/KCoreAddons/
> (no subdirs and loots of files)
>
> and so on...
>
> Also, there is this: http://mail.kde.org/pipermail/kde-frameworks-devel/2013-December/009001.html (see Kevin's reply at the bottom).
>
> In fact, after looking a bit more into this, about half of frameworks installs headers into same directory while the rest uses subdirs. According to the above archive, it should use the same directory however.
>
> Christophe Giboudeaux wrote:
> probably fixed with http://commits.kde.org/kdepim-runtime/c43c404e915084b07582177928643c3f16ffc7cf
>
> Martin Klapetek wrote:
> Well, no. The issue as described above still stands, the consensus was different than the current way and I'd like us to get things less random.
>
> Christophe Giboudeaux wrote:
> There's nothing random. Let's take kcoreaddons and attica as examples:
>
> KCoreAddons/KAboutData contains #include "kaboutdata.h"
> Attica/Attica/AccountBalance contains #include "attica/accountbalance.h"
>
> The namespaced camelcase headers just follow what is done for the files they include.
> and if you need another reason:
>
> # ls **/Plugin
> kabc/include/KF5/KABC/KABC/Plugin kontactinterface/include/KF5/KontactInterface/KontactInterface/Plugin ktexteditor/include/KF5/KTextEditor/KTextEditor/Plugin kdelibs4support/include/KF5/KDELibs4Support/KDE/KParts/Plugin kparts/include/KF5/KParts/KParts/Plugin
>
> If you use these build dependencies, what should happen if you just #include <Plugin> ?
Please refer to http://mail.kde.org/pipermail/kde-frameworks-devel/2013-December/009001.html
Quoting:
The camel cased includes and the .h ones were planned to live in the same folder.
Ending quote.
Now, as you posted above:
set_target_properties(KF5::PimUtils PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/KF5/KPIMUtils;${_IMPORT_PREFIX}/include/KF5"
)
...will clearly make #include <KPIMUtils/Whatever> still work, without the need of another KPIMUtils subdirectory. Which is pretty much my whole point.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120959/#review69749
-----------------------------------------------------------
On Nov. 3, 2014, 10:21 p.m., Martin Klapetek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120959/
> -----------------------------------------------------------
>
> (Updated Nov. 3, 2014, 10:21 p.m.)
>
>
> Review request for KDE Frameworks, KDEPIM-Libraries and Laurent Montel.
>
>
> Repository: kdepimlibs
>
>
> Description
> -------
>
> The generated headers have "#include "kpimutils/linklocator.h" but since they are being installed in KPIMUtils/KPIMUtils/, the inclusion fails because those real headers are installed in KPIMUtils/kpimutils (rather than KPIMUtils/KPIMUtils/kpimutils). I also don't see a reason why to install into KPIMUtils/KPIMUtils/...?
>
>
> Diffs
> -----
>
> kpimutils/src/CMakeLists.txt 1acc88e
>
> Diff: https://git.reviewboard.kde.org/r/120959/diff/
>
>
> Testing
> -------
>
> Akonadi-facebook fails to build with "kpimutils/linklocator.h - No such file or directory", with this it no longer gives the error.
>
>
> Thanks,
>
> Martin Klapetek
>
>
_______________________________________________
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