Templates for frameworks CMake files

[Aurélien Gâteau]

On Tuesday 24 September 2013 16:58:36 Stephen Kelly wrote:
> Aurélien Gâteau wrote:
> > Hi,
> > 
> > I have been playing around with itemviews CMake files and put together
> > some templates for the top level CMakeLists.txt and *Config.cmake.in. You
> > can find them attached there. Any one against me adding those to the
> > repository?
> > 
> > Aurélien
> > CMakeLists.txt
> > 
> >   cmake_minimum_required(VERSION 2.8.11)
> 
> This template will be out of date whenever we require a new cmake.
> 
> >  project(FooBar)
> >  
> >  find_package(ECM 0.0.8 REQUIRED NO_MODULE)
> 
> The latest ECM is 0.0.9

Just bumped the version number. Interestingly no existing framework requires 
it yet.

> >  set(CMAKE_MODULE_PATH ${ECM_MODULE_PATH})
> >  
> >  set(REQUIRED_QT_VERSION "5.2.0")
> 
> ECM and Qt version bumps are not as much an issue as the cmake one. The ECM
> and Qt version deps are not as likely to change.

I don't understand why you think specifying the cmake version is an issue: it 
should be easy to update the template when this happens, isn't it?

> >  # Required Qt5 components to build this framework
> >  # For example:
> >  # find_package(Qt5 ${REQUIRED_QT_VERSION} REQUIRED NO_MODULE COMPONENTS
> >  # Widgets)
> 
> Simplify by putting NO_MODULE before REQUIRED and removing COMPONENTS:
> 
>  find_package(Qt5 ${REQUIRED_QT_VERSION} NO_MODULE REQUIRED Widgets)

Done.
> 
> >  if(NOT kdelibs_SOURCE_DIR)
> >  find_package(KF5 5.0.0 REQUIRED MODULE COMPONENTS CMake Compiler
> >  InstallDirs) # Required KF5 frameworks to build this framework
> 
> I still think this stuff is odd. We're building KF5, yet we need to find KF5
> to do so.

This line is only used when building the framework outside of kdelibs, in this 
case we are not building KF5.
> 
> <snip>
> 
> >   FooBarConfigVersion.cmake.in
> >   @PACKAGE_INIT@
> >  
> >  # Required components to use this framework
> >  # For example:
> >  # find_dependency(Qt5Widgets @REQUIRED_QT_VERSION@)
> >  # find_dependency(KCoreAddons ${PACKAGE_VERSION})
> 
> We might want to change this before you template it. Currently,
> 
>  find_package(KAnyTier2Framework)
> 
> will result in finding its dependency with
> 
>  find_package(KDependentTier1Framework)
> 
> ie, the dependency is not found by version unless a version is specified
> when finding KAnyTier2Framework. ie:
> 
>  find_package(KAnyTier2Framework 5.3)
> 
> will call
> 
>  find_package(KDependentTier1Framework 5.3)
> 
> However, it might make sense for even
> 
>  find_package(KAnyTier2Framework)
> 
> to require the same or later version as KAnyTier2Framework.
> 
> That would be another change that would affect your template, and which
> would make
> 
>  find_dependency(KCoreAddons ${PACKAGE_VERSION})
> 
> 'wrong'.
> 
> >  set(FooBar_INSTALL_PREFIX "@PACKAGE_CMAKE_INSTALL_PREFIX@")
> 
> IMO this should be removed from all config files and not added to new ones:
> 
>  http://thread.gmane.org/gmane.comp.kde.devel.frameworks/1194/focus=1318
> 
> It is unused and not useful or needed. ie
> 
>  git grep -l PACKAGE_CMAKE_INSTALL_PREFIX | xargs \
>    sed -i '/PACKAGE_CMAKE_INSTALL_PREFIX/d'
> 
> and remove the CMAKE_INSTALL_PREFIX from the configure_package_config_file
> invocations.
> 
> >  set_and_check(FooBar_INCLUDE_DIR "@PACKAGE_INCLUDE_INSTALL_DIR@")
> >  set(FooBar_INCLUDE_DIRS ${FooBar_INCLUDE_DIR} )
> 
> As the include dirs are encoded into the imported targets, this is
> superfluous. No one needs to use it, and it might contain different/out of
> date information compared to what is encoded in the target.
> 
> If it is decided to remove them, then the INCLUDE_INSTALL_DIR should be
> removed from the configure_package_config_file too.

I removed those from the template as well.

I just filed a review request for the templates, which should make it easier 
to discuss, you can find it here:

https://git.reviewboard.kde.org/r/112928/

Aurélien