Alexander Neundorf wrote: > The code for creating a Config.cmake file is not trivial, but IMO also not > boilerplate, and Stephen agreed in Berlin that this will have to be done > individually be every project. This is the added > threadweaverConfig.cmake.in and the call to > configure_package_config_file() in the CMakeLists.txt.
I think it's premature to be creating and checking in Config files for frameworks. As they are not generated, doing that now will mean that ~20 files will need to be changed if something is wrong. There are already several problems with the files you're checking in now. 1) The inclusion of the Targets.cmake file is unguarded: See http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/5410 2) You are setting things that do not need to be set: * Eg: kwidgetsaddons_INCLUDE_DIR, kwidgetsaddons_LIBRARY_DIR. The kwidgetsaddons_LIBRARY_DIR is not needed at all. That's the kind of thing that you only need (and put in the cache) if you're writing a Find file so that the user can 'help' and update the cache to the actual directory if needed http://osdir.com/ml/programming.tools.cmake.user/2006-10/msg00152.html As we're providing Config files, the user is not going to know better than the config file "by design", so we don't need to set the _DIR variable. We also don't need to set the kwidgetsaddons_LIBRARY_DIRS variable because there is no need for it or way to make use of it. Additionally, when we depend on CMake 2.8.11, we won't need to set the kwidgetsaddons_INCLUDE_DIRS variable, because the contents of that will be encoded into the IMPORTED target, and the user will simply do target_link_libraries(foo KF5::widgetsaddons) or simlilar without needing to specify the include dirs manually. If you add those things now, we'll just have to remove them when we update the CMake dependency. http://community.kde.org/Frameworks/Epics/CMake_target_usage_requirements 4) Also on the topic of naming things correctly, kwidgetsaddons_LIBRARY should be kwidgetsaddons_LIBRARIES, but actually it shouldn't exist at all because users should use the imported target directly instead as I wrote above, instead of using the variable. Additionally, you're setting that variable to KDE4__kwidgetsaddons, which we'll only have to rename by hand later. 5) I also see no need for the kwidgetsaddons_INSTALL_PREFIX variable. 6) You've already made some copypastos: +set(kwidgetsaddons_VERSION_MAJOR "@KWIDGETSADDONS_VERSION_MAJOR@") +set(kwidgetsaddons_VERSION_MINOR "@KWIDGETSADDONS_VERSION_MINOR@") +set(kwindowsystem_VERSION_PATCH "@KWIDGETSADDONS_VERSION_PATCH@") Given that we'll need to maintain Config files by hand, let's please: a) minimise what we put into them (no need to set variables which are not useful, and which we run the risk of typo-ing), b) Make sure that what we write by hand is as close to the final state as possible. We don't want to have to manually update all KDE4__* to KF5::* etc. The tier1 Config files (after CMake 2.8.11) should only contain this: set(kwidgetsaddons_VERSION_MAJOR "@KWIDGETSADDONS_VERSION_MAJOR@") set(kwidgetsaddons_VERSION_MINOR "@KWIDGETSADDONS_VERSION_MINOR@") set(kwidgetsaddons_VERSION_PATCH "@KWIDGETSADDONS_VERSION_PATCH@") if (NOT TARGET KF5::widgetsaddons) # See item 1. include("${CMAKE_CURRENT_LIST_DIR}/kwidgetsaddonsTargets.cmake") endif() include(FooMacros.cmake) # As required. The tier2 and higher config files need also to find_package their dependencies. If you think anything else is needed (apart from for compatibility variables, which we can provide separately - there is no need for compatibility variables for kwidgetaddons as it has not existed before), then please let me know and we can make sure that it can be done with IMPORTED targets instead. Thanks, Steve. _______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
