[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: only install CMakeParseArguments.cmake for cmake
From:       Alexander Neundorf <neundorf () kde ! org>
Date:       2011-10-16 11:01:41
Message-ID: 201110161301.41366.neundorf () kde ! org
[Download RAW message or body]

On Sunday 16 October 2011, Ralf Jung wrote:
> > On Oct. 16, 2011, 9:43 a.m., Alexander Neundorf wrote:
> > > Please don't commit this.
> > > This is on purpose.
> > > We install a file which includes CMakeParseArguments.cmake, so we
> > > *must* also install CMakeParseArguments.cmake itself, otherwise we may
> > > get in trouble if the version in cmake changes.
> > > 
> > > Alex
> 
> The problem is that this results in a warning about CMake files being
> overridden when running cmake for kde-workspace.

Can you post the exact warning message here, please ?

> Plus, the files is not
> even exactly the same as the CMakeParseArgument.cmake that is shipped with
> my cmake version - it is missing the double-include guard.

Thanks for pointing that out, the double-include guard should probably be 
removed from the cmake one.
 
> If you do not want to use the system version of the file even if it exists,
> then why not rename this file to KDEParseAguments and fix the cmake files
> using it?

This file is there just temporary, and will go away again (once we depend on 
cmake > 2.8.3).
I'd prefer to change them as little as possible.

> After all you obviously don't even want the behaviour of the
> cmake default version, but specifically the behaviour of the version
> shipped in kdelibs. So there is no need to make the filename collide with
> the "official" version.
> 
> I could write up such a patch - is FindPackageHandleStandardArgs.cmake the
> only user?

I don't think that's a good idea.
Instead we'll require cmake 2.8.6 (or 2.8.7) in the not too far future.
Then the file can be removed and the warning goes away.

Alex

[prev in list] [next in list] [prev in thread] [next in thread]