Do not force CMAKE_COLOR_MAKEFILE=ON

[Alexander Neundorf]

Hi Maciej,

On Tuesday 29 June 2010, Maciej Mrozowski wrote:
> On Tuesday 29 of June 2010 02:29:15 Allen Winter wrote:
> > On Sunday 27 June 2010 7:33:15 pm Maciej Mrozowski wrote:
> > > Let me reverse the question: why do you need to explicitly enable
> > > colours? :)
> >
> > Mainly because the developers like it.
> > The old buildsystem (with unsermake) enabled colors and we got attached
> > to that feature. colors were set on by default iirc
> >
> > So this is just making things behave like they have for many years and
> > because the developers like it that way.
> >
> > Not saying that we can't change for a good reason.  But this sounds more
> > like a bug in CMake or something you could easily fix in your build
> > script.
>
> No, it's not bug in CMake (like I said, single stream redirection is
> detected by CMake so that it doesn't output Esc characters) nor it's a bug
> in build script, as apps not using find_package(KDE4) work as expected -
> they respect CMAKE_COLOR_MAKEFILE variable.
>
> I think I wasn't clear enough.
>
> Why do you need to hard-enable colours in CMake while it's already enabled
> by default (so there's no need to force it - I've noted this in first post)
> - leaving no means to disable them for those who for some reason don't like
> or want them.
>
> set(CMAKE_COLOR_MAKEFILE ON)
>
> This is not enabling colours by default, this is forcing them.
> It's equally 'lethal' as hardcoding CMAKE_BUILD_TYPE, CMAKE_INSTALL_DIR
> (you wouldn't want that, would you?) and similar. Leaving you no choice but
> to edit CMakeLists.txt (which against its purpose as a build system - being
> flexible and adopting to build environment).
> As for hardcoding CMAKE_BUILD_TYPE, CMAKE_INSTALL_DIR, I've seen many
> broken packages with buildsystem along the lines "we know better".
> People do.
>
> Anyway, forget it, I'll just patch it locally for Gentoo as getting things
> upstream is in general resource wasteful (I think this is why most dirtros
> prefer saving resources - we still need to get smarter).
>
> P.S.
> Seriously, I didn't expect proposing removal of hardcoded, unnecessary
> CMAKE_COLOR_MAKEFILE=ON to force me to waste so many posts explaining those
> obvious things.

That's wrong.
When I put these lines there, it was for a reason. Probably (... it's 4 years 
ago...) at that time the color had to be enabled explicitely.
This has changed since then.
Still, if there is a patch which removes something which is or at least was 
there for a reason, and where it is also not completely obvious why this is 
necessary (I expected the autodetection to cover all cases), 
So, an explanation even for such a small patch is necessary.

Now that everything is clear, I'd say go ahead and commit.

Alex

P.S. I really appreciate _a lot_ that you as a packager contribute directly 
upstream, please keep it going :-)