Config files for frameworks (Was: KArchive for Qt4)

[Stephen Kelly]

Alexander Neundorf wrote:
> I still think it is not a good idea that
> 
> target_link_libraries(foo KF5::widgetsaddons)
> 
> also sets include dirs for the target foo.

> Should I discuss that again on cmake-devel ?

It would seem more appropriate than here, but I think it's already decided.

> 
>> 4) Also on the topic of naming things correctly, kwidgetsaddons_LIBRARY
>> should be kwidgetsaddons_LIBRARIES,
> 
> Yes, I am aware of that, and wanted to change that for all at once.

You mean after committing it as *_LIBRARY everywhere, you want to change it 
all to *_LIBRARIES later? Why not just start with *_LIBRARIES so it doesn't 
have to be change later, or am I missing something?

> 
>> 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.
> 
> I'm not sold on this.
> When reading a CMakeLists.txt, I see
> 
> find_package(Foo)
> 
> and then I expect to be able to use
> include_directories(${FOO_INCLUDE_DIRS})
> ...
> target_link_libraries(hello ${${FOO_LIBRARIES})

Note that you're not using exact-case in those variables, which I think is 
ill advised anyway.

> Retraining all our users that using include_directories() is wrong doesn't
> sound good to me.

I think re-training is the cost of simplicity, and it's worth it.

You only expect to have to use include_directories() because you have always 
had to. Soon you won't have to. In the future, people using KF5 will never 
have had to.

>> Additionally, you're setting that
>> variable to KDE4__kwidgetsaddons, which we'll only have to rename by hand
>> later.
> 
> I am aware of that. This was to keep one of those libs compatible with
> FindKDE4Internal.cmake, I think threadweaver.
> This is something which can be easily patched with a script.
> 
> 
>> 5) I also see no need for the kwidgetsaddons_INSTALL_PREFIX variable.
> 
> In some cases it is necessary, if the user wants/needs to access
> additional files provided by the package.

If there are additional files provided by the package then we need to 
provide a variable for each of those files, eg

 set(Foo_ExtraMacros "${CMAKE_CURRENT_LIST_DIR}/FooExtraMacros.cmake")

and document that variable. We shouldn't expect users to dig into our 
internals like that, nor should we encourage it. 

If a file is not documented and has no associated variable, then it is fully 
internal and likely to be changed in non-backward-compatibile ways or 
removed entirely.

Besides, cmake already sets Foo_DIR to the location that the Config file was 
found in.

> It also shouldn't hurt.

It hurts because it is more for us to maintain in 20+ files which is not 
needed or useful.

Let's not add a variable like that to all Config files please. For one thing 
you are adding it to places which certainly don't need it (as they have no 
extra files), and for another it is bad API.

>> 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.
> 
> The libraries may have to provide information about supported optional
> features, but these are typically simple variables, not references to
> files etc.

Let's deal with situations like that as they come up. Do you know of any 
specific examples?

Thanks,

Steve.