KDE/kdevplatform

[Andreas Pakulat]

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.