[CMake] IMPORTANT!! SET CACHE FORCE not wortking with UNINITIALIZED variables

[Fernando Cacciola]

Hi Bill,

> Eric NOULARD wrote:
>> Le Tue, 11 Nov 2008 11:47:20 -0200,
>> Fernando Cacciola <fernando.cacciola at gmail.com> a écrit :
>>> Hi,
>>>
>>> Since my last post about this got unnoticed I'm reposting it, this
>>> time with additional information.
>>>
> 
> Sorry, I did see your post, and I am busy...
> 
Understandable.

> So, you also mentioned this case:
> 
> SET( VAR "321" CACHE STRING "" FORCE )
> set(VAR )
> message(${VAR})   -> prints out 321
> 
Actually I said that it prints 123 in 2.6.2 but 321 in 2.4.*

> So, this was intentional.   set(var ) removes the value of var, and var 
> becomes "unset".  This then re-exposes the cache variable VAR.
> 
I know, I purposedly added the SET(VAR) line to discover the value in 
the cache so as to confirm that if its type is UNINITIALIZED then FORCE 
doesn't override it.

(IOW just to make sure I wasn't seeing the in-memory value instead)

> 
> So, I think this was intentional, sort of...
> 
> 
> The issue is that we want the command line to be used.

Me too :)

 > So, if you have:
> 
> cmake -DFOO=456
> 
> Then, FOO should be set in the cache to the value 456 and something like 
> this:
> SET( VAR "123" CACHE STRING ""  )
> 
> Should not set VAR to 123

Right, I agree with this.

 > but it should set the type to STRING.

Hmm, OK.

 From reading the source code I see that:

If I said -DFOO:BOOL=ON

 > SET( VAR "OFF" CACHE STRING ""  )

Does not even change the type.

So what you want is that *untyped* command line variables can be given a 
type via SET command, even without FORCE?

And you implement that via UNINITIALIZED, this is why there is this test 
for UNINITIALIZED in cmSetCommand.cxx:

   if(!it.IsAtEnd() && (it.GetType() != cmCacheManager::UNINITIALIZED))
     {
     // if the set is trying to CACHE the value but the value
     // is already in the cache and the type is not internal
     // then leave now without setting any definitions in the cache
     // or the makefile
     if(cache && type != cmCacheManager::INTERNAL && !force)
       {
       return true;
       }
     }

so SET doesn't bail early in all cases it sees the var already without 
FORCE and gets a chance to change the type later in AddCacheDefinition, 
which is purposedly keeping the previous value in the case of an 
UNINITIALIZED variable as it's purpose in this context is to only change 
the type.
Right?



OK, then IMO what we need is to pass the force boolean down to 
AddCacheDefinition, so it can do the right thing:

void cmMakefile::AddCacheDefinition(const char* name, const char* value,
                                     const char* doc,
                                     cmCacheManager::CacheEntryType type
                                     bool force <<<<<<<<<<<<<<<<<<<
                                     )
{
   const char* val = value;
   cmCacheManager::CacheIterator it =
     this->GetCacheManager()->GetCacheIterator(name);
   if(!it.IsAtEnd() && (it.GetType() == cmCacheManager::UNINITIALIZED) &&
      it.Initialized())
     {
     if ( ! force )  <<<<<<<<<<<<<<<<<<<<<<<<<
       val = it.GetValue();


I can generate proper diff patches if that looks OK to you, but I have 
no idea how to test it except manually.

Best

Fernando