KDE/kdevplatform

[Andreas Pakulat]

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.

Andreas

-- 
You will wish you hadn't.