Modified build system for itemmodels

[Stephen Kelly]

Alexander Neundorf wrote:
> URL and DESCRIPTION belong to the used project, so they are candidates for
> being set in the Config.cmake file.
> But there are two downsides of setting them in the Config file:
> - the information where to get the package is not present when the package
> is not present

Right. 

This IMO makes it not useful to use set_package_properties in Config files.

> - the Config file has to, as you mentioned, include()
> FeatureSummary.cmake, which drags in compatibility issues.
> 
> I think having trivial FindFoo.cmake files even for projects which install
> Config files is a good thing.
> They can set such information

Or such information could be set in the using project, which isn't much 
worse.

> , they can do stuff, like search dependencies, 

Dependencies should be found in the Config file so that the Find module 
isn't strictly necessary.

> they can provide documentation 

That could be done in the config file too of course. We discussed that 
before, and one of the issues with putting that stuff in the Find module is 
that it is not distributed with the used project, so the documentation could 
be out of date. If the docs are distributed with the Config file, that is 
not a problem.

> and they make it easy for
> users to check how a package is searched.

I'm not sure that's useful. 

Even if there's a Config file and a Find file, I still have to use git grep 
in the calling project to see what arguments it uses to find_package (which 
are sent to the Find module).

Besides, that just makes the Find module another place to look when faced 
with a build error, which might be outside of the repo I'm trying to build 
(eg if we put such Find modules in ecm). Doesn't sound obvious.

> So, if possible, we should not include more external files than strictly
> necessary into Config files.

Yes. We need to figure out what's necessary and why then. ParseArguments is 
necessary because it is useful for macros. 

Including something for conveniently setting the common stuff is the part we 
disagree on the necessity of.

> Won't propagating the libraries and include dirs to use be done via your
> work on target_use_package() ?

That's not necessarily everything, so it doesn't solve all of the problem.

For example, it doesn't include() the dependent macro files, it doesn't 
include the calls to set(Foo_HAS_Blub True), and other things. 

Most importantly though, if we don't find_package(KArchive) inside 
KioConfig.cmake, the KF5::Archive target won't exist.

So calling find_package() for the dependencies both makes sense and is 
necessary.

>  
>> Then there's optional dependencies too, which would have to be found, and
>> which might have an effect on how the variables are populated... You
>> can't stuff everything into a macro like that. It becomes very unwieldy
>> and I don't think it's flexible enough.
> 
> It's name contains "basic", I think it is good enough for "basic"
> Config.cmake files.
> Also, with the feature to be able to include() additional files into the
> Config.cmake file you can more or less do whatever you want.

Then we're back to the problem of whether include()ing in the Config file is 
ok.

If it is, then I'd resubmit my proposal to have:

include(ItemModelsConfigCommon.cmake)

inside ItemModelsConfig.cmake, and generate it in the CMakeLists.txt

It is shipped with KItemModels, so it doesn't have the problem of including 
something that is not shipped with it, as we want to avoid.

> 
> As I said, you can put a lot of stuff into Config.cmake files, so the
> developer has to understand them and actually write them.
> 
> So your main point is to avoid repeating the trivial parts ?

It seems that with your proposal, either 

* The basic macro is enough, or 
* The basic macro can be used, but you have a non-trivial 
EXTRA_CONFIG_STRING, or
* You don't use the basic macro and you have a lot of repetition.

I think that being able to avoid the repetition would be a good thing, yes. 

More importantly though I want to be able to extend what is common. I'll 
mention again that the _export.h files we had in kdelibs were slightly 
divergent because they were copy+pasted at different times, and so contain 
different features.

> We have them now because we decided to split kdelibs into multiple
> repositories.
> 
> Also we decided to try to avoid using stuff which is not in cmake itself.
> Now adding trivial stuff to e-c-m with the only purpose to make the life
> of the KDE buildsystem maintainers easier, defeats this.
> 
> Please post a proposal for what you want to do with the version stuff.
> I really want to get rid of this ASAP:
> 
> macro(ecm_version _major _minor _patch)
>   set(ECM_VERSION_MAJOR ${_major})
>   set(ECM_VERSION_MINOR ${_minor})
>   set(ECM_VERSION_PATCH ${_patch})
>   set(ECM_SOVERSION ${_major})
>   set(ECM_VERSION_STRING
>     "${ECM_VERSION_MAJOR}.${ECM_VERSION_MINOR}.${ECM_VERSION_PATCH}")
> endmacro()
> 
> and I want to actually start working on kf5 before Akademy.

Is the version macro a blocker to you working on kf5? If so just remove it 
and copy+paste the stuff. 

I don't understand the urgency of removing it in particular (there's plenty 
of other stuff to do), so we can sort it out later if need be, or just leave 
the copy+paste.

Thanks,

Steve.