[CMake] [PATCH] slightly modify cache variable behaviour, Was: Re: option() behavior

[Philip Lowman]

On Sun, May 10, 2009 at 5:08 PM, Alexander Neundorf <a.neundorf-work at gmx.net
> wrote:

> The attached patch (against current cvs HEAD) changes the behaviour of set(
> CACHE) and option() slightly.
>
> Until now it behaves like this:
> if a variable FOO is set to a value, and then set FOO to a value in the
> CACHE,
> and FOO was not yet in the cache, then afterwards the visible value of FOO
> is
> the one specified in the set( CACHE) command.
>
> Now assume the same, but FOO is already in the cache. In this case after
> the
> set() and the set(CACHE) the value in the cache won't have changed (which
> is
> good), but also the value from the cache will not be visible, but instead
> FOO
> will have the value from the set(), i.e. the value in the cache doesn't
> matter at all, even although the set(CACHE) was after the set().
>
>
> You can test this with the following CMakeLists.txt:
>
> set(FOO foo)
> message(STATUS "FOO 1: ${FOO}")
> set(FOO bar CACHE STRING "fofofofof")
> message(STATUS "FOO 2: ${FOO}")
>
> set(OPT abc)
> message(STATUS "OPT 1: ${OPT}")
> option(OPT "daf df df sdf " ON)
> message(STATUS "OPT 2: ${OPT}")
>
> Current cvs HEAD gives:
>
> $ cmake .
> -- FOO 1: foo
> -- FOO 2: bar
> -- OPT 1: abc
> -- OPT 2: ON
> $ cmake .
> -- FOO 1: foo
> -- FOO 2: foo
> -- OPT 1: abc
> -- OPT 2: abc
>
> With the patch you get:
> $ cmake .
> -- FOO 1: foo
> -- FOO 2: bar
> -- OPT 1: abc
> -- OPT 2: ON
> $ cmake .
> -- FOO 1: foo
> -- FOO 2: bar
> -- OPT 1: abc
> -- OPT 2: ON
>

I would have expected CMake to behave like this:

$ cmake .
-- FOO 1: foo
-- FOO 2: foo
-- OPT 1: abc
-- OPT 2: abc
$ cmake .
-- FOO 1: foo
-- FOO 2: foo
-- OPT 1: abc
-- OPT 2: abc

1. In CMake right now is "local overrides cache, unless a cache variable is
being created during a particular configure"
2. Alex's patch looks like "cache overrides local once set(..CACHE) or
option() is encountered"
3. My expectation of what CMake should have done is based on "local
overrides cache, always"

Basically what it boils down to is do we:
1. Remove local variables only when set(.. CACHE) or option() is run the
first time (while creating a cache var)
2. Remove local variables once set(..CACHE) or option() is encountered
3. Never remove local variables

The patch for the 3rd behavior is below.  Personally, I prefer it to the 2nd
(which could be slightly confusing depending on when the user evaluates the
variable) although either is preferable in my mind to the 1st (which changes
behavior depending on how many times you configure the build!)

Index: Source/cmMakefile.cxx
===================================================================
RCS file: /cvsroot/CMake/CMake/Source/cmMakefile.cxx,v
retrieving revision 1.508
diff -u -b -B -u -r1.508 cmMakefile.cxx
--- Source/cmMakefile.cxx    24 Apr 2009 15:18:05 -0000    1.508
+++ Source/cmMakefile.cxx    11 May 2009 02:59:42 -0000
@@ -1702,8 +1702,6 @@

     }
   this->GetCacheManager()->AddCacheEntry(name, val, doc, type);
-  // if there was a definition then remove it
-  this->DefinitionStack.back().erase( DefinitionMap::key_type(name));
 }


@@ -1747,7 +1745,6 @@
     val = it.GetValueAsBool();
     }
   this->GetCacheManager()->AddCacheEntry(name, val, doc);
-  this->AddDefinition(name, val);
 }

 void cmMakefile::RemoveDefinition(const char* name)


-- 
Philip Lowman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.cmake.org/pipermail/cmake/attachments/20090510/a3a474f4/attachment.htm>