Modified build system for itemmodels

[Stephen Kelly]

Alexander Neundorf wrote:
> Hi,
> 
> attached is a patch for kdelibs/tier1/itemmodels/.
> 
> It removes the usage of ECMQtFramework.cmake and uses instead the (new)
> cmake features directly.
> 
> The biggest difference is that itemmodels_version.h.in,
> itemmodelsConfig.cmake.in and itemmodelsUseFile.cmake.in are now part of
> itemmodels and also configured() directly in its CMakeLists.txt.

itemmodels_version.h.in should be able to be a template in ecm (it works for 
ConfigVersion.cmake files). The problem with copies is that they get changed 
in a different way in each project, so you'll end up with 20 slightly 
different instances of this. There is also no central place to add features. 
If I want to add K_ITEMMODELS_DEPRECATED_SINCE(x, y), similar to the Qt 
macro, I want to add it once, not 20 times.

For the config file, I would also prefer not to copy everything around. I 
think it would be better to have a macro/function in ecm generate the common 
stuff in a Config-common.cmake, and then each framework maintainer will 
write a config file like this:

include(itemmodelsConfig-common.cmake)

macro(my_framework_macro)
  # ...
endmacro()

Again, this would allow the frameworks to get the new features when we add 
them to the ${project}Config-common.cmake template.

> 
> Despite of the code duplication (in trivial cases) this causes, why do I
> think this is better ?
> 
> Without the patch, all you see in the CMakeLists.txt is
> include(ECMQtFramework)
> without any following function/macro calls.

Your patch is indeed an improvement in that area. However, we can also 
equally have 

ecm_write_version_header(...) to create the itemmodels.h, and 
ecm_write_config_common(...) to create the cmake file for config stuff.

> 
> So, this basically doesn't tell the developer anything about what it does.
> As result, he ends up with a bunch of created files in his build dir,
> which also get installed.

Yes, having macro/function invokations is more clear than just the include.

This makes it obvious enough to developers where the stuff comes from.

> 
> This IMO easily qualifies as "magic" most developers will not understand,
> they will not know where these files come from, what they are good for,
> what they should contain.
> 
> By having the
> write_basic_package_version_file()
> configure_package_config_file()
> and
> configure_file()
> 
> calls, followed by their respective install() calls directly in the
> CMakeLists.txt, it becomes immediately obvious where these generated files
> come from.
> By having the .in files in the same directly, it makes it also much easier
> for the developer to have a look at them, and to understand them,
> especially since they are simpler now too (since they are less general).

I don't think they'll have a look anyway tbh. Most people in KDE avoid the 
buildsystem.

> Especially the FooConfig.cmake.in file should IMO not be provided
> somewhere, or generated automatically, since it can and should carry
> information specific to the respective project. 

Right I agree. The common stuff can be generated though.

> That's way it is IMO not a
> good idea to provide a generic one in e-c-m. Developers will start to
> simply use it, which will keep them from learning what they can do with
> such files, and which information can and should be put in them.

I think that's also the case if we generate 'common' stuff.

> The same probably applies for the Use-file.

Why do we need a use file? Qt 5 doesn't create or install them.

include(${QT_USE_FILE})

won't do anything in Qt 5 (It will be an error)

> 
> We can discuss about providing some wrapper which in some way bundles the
> calls to write_basic_package_version_file(),
> configure_package_config_file() and configure_file(), but it should be in
> a way that it stays obvious where the generated files come from, and easy
> for the developer to modify them depending on the project-specific needs.

Maybe. I'm not sure it's needed.

> 
> In short: the Config.cmake files should be visible to the developer, this
> should make understanding them easier.

I'm sure we can still get the same effect without having developers 
copy/pasting out of date stuff. 

Many of the export headers we removed earlier in the year were inconsistent 
with each other for exactly that reason. 

Thanks,

Steve.