Review Request 120959: [KPimUtils] Don't install headers into $include_dir/KPIMUtils/KPIMUtils

[Christophe Giboudeaux]

> On nov. 3, 2014, 7:59 après-midi, 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.

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> ?


- Christophe


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


On nov. 3, 2014, 9:21 après-midi, Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120959/
> -----------------------------------------------------------
> 
> (Updated nov. 3, 2014, 9:21 après-midi)
> 
> 
> 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
> 
>

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