On Thursday 02 January 2014 07:45:12 Kevin Ottens wrote:
> On Thursday 02 January 2014 00:31:05 David Faure wrote:
> > On Tuesday 31 December 2013 14:48:13 Aleix Pol wrote:
> > > > -- Up-to-date:
> > > > /d/kde/inst/kde_frameworks/include/KF5/kcoreaddons/kaboutdata.h
> > > > -- Up-to-date:
> > > > /d/kde/inst/kde_frameworks/include/KF5/KCoreAddons/KAboutData
> > > >
> > > > The email thread "RFC Rules for installation of header files" does say
> > > >
> > > > >>> If the header files of a framework are not prefixed, then they
> > > > >>> should
> > > > >>> be installed in include/{lowercaseframework} and convenience
> > > > >>> headers
> > > > >>> should be installed in include/KDE/{CamelCaseFramework}.
> >
> > Actually, what's the reason for lowercaseframework there?
> >
> > I used that in all the modules I converted (except KIO, I just realized),
> > and I'm wondering what the point is.
> >
> > Example:
> > include/KF5/KIOCore/KFileItem
> > include/KF5/KIOCore/kfileitem.h
> > works fine with KIOCore in the include path, apps just include <KFileItem>
> > or <kfileitem.h>
> >
> > If I fix it to
> > include/KF5/KIOCore/KFileItem
> > include/KF5/kiocore/kfileitem.h [all lowercase]
> > it will of course work too, with both dirs in the include path.
> >
> > It's all transparent for the apps either way, since the cmake magic
> > encapsulates it for the them.
> >
> > I'm just wondering what's the point in using two different dirs for
> > kfileitem.h and KFileItem?
>
> This wasn't intended I guess. When the discussion about include files
> occured the consensus was to have everything in a single directory (the
> camel cased one).
OK I initially thought it was so the implementation of ecm_generate_headers is
simpler (lowercase everything), but in fact that's not it, it lowercases
individual parts explicitely.
In fact if we only use a camelcase header, the CMakeLists.txt code is even
simpler because the INSTALL(DIRECTORY) line will install both kinds of
headers. Which removes the need for the KCoreAddons_HEADERS variable
altogether.
See attached patch.
Aleix, any objections?
I'll script this over all modules, if not.
I think the reasoning might have been: so that people can include
<kcoreaddons/krandom.h>. But I'm not sure we even want to allow that.
It breaks the whole idea that "the install dir is transparent to the apps",
the idea of compile-time errors (header not found) when not linking to the
appropriate lib, and of course it will break when we move stuff around for
kf6.
Well, we can't physically prevent people from writing #include
<KCoreAddons/krandom.h> or <KCoreAddons/KRandom>, but the fact that the first
one looks weird will at least reduce the chances of people doing that :)
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5
diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt
index 4496ea9..5947515 100644
--- a/src/lib/CMakeLists.txt
+++ b/src/lib/CMakeLists.txt
@@ -96,7 +96,7 @@ else()
target_link_libraries(KF5CoreAddons PRIVATE netapi32)
endif()
-target_include_directories(KF5CoreAddons INTERFACE "$<INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}/kcoreaddons;${INCLUDE_INSTALL_DIR}/KCoreAddons>" )
+target_include_directories(KF5CoreAddons INTERFACE "$<INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}/KCoreAddons>" )
target_compile_definitions(KF5CoreAddons INTERFACE "$<INSTALL_INTERFACE:KCOREADDONS_LIB>")
@@ -108,13 +108,11 @@ set_target_properties(KF5CoreAddons PROPERTIES VERSION ${KCOREADDONS_VERSION_S
ecm_generate_headers(
KAboutData
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KSharedDataCache
RELATIVE caching
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KAutoSaveFile
@@ -125,7 +123,6 @@ ecm_generate_headers(
KUrlMimeData
RELATIVE io
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KCompositeJob
@@ -134,21 +131,18 @@ ecm_generate_headers(
KJobUiDelegate
RELATIVE jobs
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KRandom
KRandomSequence
RELATIVE randomness
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KMacroExpander
KStringHandler
RELATIVE text
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
ecm_generate_headers(
KFormat
@@ -156,15 +150,13 @@ ecm_generate_headers(
KShell
RELATIVE util
- REQUIRED_HEADERS KCoreAddons_HEADERS
)
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/KCoreAddons DESTINATION ${INCLUDE_INSTALL_DIR} COMPONENT Devel )
install(TARGETS KF5CoreAddons EXPORT KF5CoreAddonsTargets ${INSTALL_TARGETS_DEFAULT_ARGS})
install(FILES
- ${KCoreAddons_HEADERS}
io/kfilesystemtype_p.h #Needed for building kio, KFileSystemType
${CMAKE_CURRENT_BINARY_DIR}/kcoreaddons_export.h
- DESTINATION ${INCLUDE_INSTALL_DIR}/kcoreaddons COMPONENT Devel
+ DESTINATION ${INCLUDE_INSTALL_DIR}/KCoreAddons COMPONENT Devel
)
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
