KDE/kdevplatform

[Aleix Pol]

On Tue, Aug 4, 2009 at 2:05 PM, Andreas Pakulat <apaku at gmx.de> wrote:

> On 04.08.09 14:02:46, Aleix Pol wrote:
> > On Tue, Aug 4, 2009 at 1:39 PM, Andreas Pakulat <apaku at gmx.de> wrote:
> > > On 04.08.09 12:57:42, Aleix Pol wrote:
> > > > On Tue, Aug 4, 2009 at 9:18 AM, Andreas Pakulat <apaku at gmx.de>
> wrote:
> > > > > > > > +configure_file(
> > > > > ${CMAKE_CURRENT_SOURCE_DIR}/config-kdevplatform.h.cmake
> > > > > > > > +
> > >  ${CMAKE_CURRENT_BINARY_DIR}/config-kdevplatform.h )
> > > > > > > > +
> > > > > > >
> > > > > > > Even more code to execute.
> > > > > >
> > > > > > I wonder what you mean. Every single cmake project use configure
> file
> > > for
> > > > > > conditional compilation inside files. I don't think is that much
> > > overhead
> > > > > > anyway. If cmake parsing (either cmake's or our's) is not fast
> enough
> > > is
> > > > > not
> > > > > > because of this (actually we don't do anything in
> configure_file).
> > > > > > I don't think it's superfluous anyway.
> > > > >
> > > > > Using a configure-file is good if you have lots of HAVE_XXX in
> various
> > > > > places/find-modules. But we only have 1 in there, its really,
> really
> > > > > easy to just have add_definitions( -DHAVE_KOMPARE ) inside the
> > > > > language/CMakeLists.txt and the configure_file doesn't add any
> value to
> > > > > this - hence its superfluous.
> > > >
> > > > Then let's use some of the other files we're already configuring.
> > >
> > > Which other files? The only thing I see is kdevplatformversion.h.cmake
> and
> > > that one cannot contain such a define because its installed.
> > >
> > > A config.h file is ok, if the define's set in it are used in multiple
> > > places (i.e. add_definitions() would have to be scattered around), but
> else
> > > its just adding yet another indirection without any added value. With
> an
> > > add_definition() its instantly clear what happens, with a
> configure_file I
> > > have to look into the original .cmake file to find out what it
> configures.
> >
> > Well, as I said, the idea behind that change is that having the code
> > statically inside kdevplatform/language/CMakeLists.txt makes it
> impossible
> > to reuse that finding on other places,
>
> Hmm, can't see that statement in the quoted text and can't recall you
> saying that, but I might have just missed that.
>
> If you actually do have code (uncomitted) for using kompare in other
> places, then sure we need a configure_file() inside the top-level
> CMakeLists (and the find-kompare should also be there).
>
> Andreas
>
> --
> You will be run over by a bus.
>
> _______________________________________________
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>

I'll un-revert then.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20090804/4c83616d/attachment.html>