[cmake-developers] set(CACHE) and the local scope

[Ben Boeckel]

On Thu, Dec 10, 2015 at 08:50:10 -0500, Brad King wrote:
>  BUG: change in how set cache overrides local definitions. Should mainly be a NOP change for most cases
>  https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f52d37c2

So Brad and I discussed today the reasons behind this change. Here is
the thread behind the cause for the change:

    http://public.kitware.com/pipermail/cmake/2007-March/013198.html

The problem was this pattern:

    function (set_cache var value)
        set("${var}" "${value}" CACHE INTERNAL "docs")
    endfunction ()

    set(some_var value CACHE INTERNAL "docs")
    set_cache(some_var other_value)
    message("${some_var}")

Before this change, "value" would be output because the first set(CACHE)
would set `some_var` to `value` in the local scope, but afterwards,
because the variable is unset, the cache would be queried directly and
"other_value" would be output.

#######

For work related to any changes to the interactions between the local
scope and the cache, here is the baseline that must be met:

    The first configure must not differ from subsequent configures.

set(CACHE) using either FORCE or the INTERNAL type do not have this
problem currently since they always touch the local scope as well. I
don't think anyone is going to disagree that the existing behavior is a
problem in this regard, so the question is which behavior to prefer
(whether set(CACHE) has other optional arguments to select a specific
behavior or not is a separate question):

Option 1:

    set(CACHE) always pulls from the cache into the local scope (first
    configure behavior is canonical)

Option 2:

    set(CACHE) never touches the local scope (subsequent configure
    behavior is canonical)

The main benefit of the first option is that reading the code is more
"logical" in that set(CACHE) always does something to that variable
(makes the cache's value available as ${var}). The value is
unpredictable since the user can always set the cache to some bogus
value (the STRINGS property or type hints will not save you because -D
exists).

The main benefit of the second option is that projects embedding
external projects could override cache variables inside of that project
without set(CACHE INTERNAL) (which doesn't help in the case of FORCE or
INTERNAL variables that inner project uses anyways).

A third option that Brad and I brainstormed after tracking down various
bits of history and thinking about the behaviors is:

Option 3:

    set(CACHE) (and any other cache-touching behavior) does *nothing*
    with the cache if a local variable by that name is defined

This has the benefits of the second in that superset projects can set
projects *and* hide cache values with a single set() command and also
caches become less cluttered (e.g., if you set PYTHON_EXECUTABLE
explicitly, the cache entry for FindPythonInterp doesn't appear and
since the project is presumably forcing the value using a local
variable, that is what is wanted anyways, so don't let the user mess
that up).

This policy would become an invasive change (there are 40 (+6 if you
count cmCPluginAPI) call sites of cmMakefile::AddCacheDefinition that
need audited for behavior changes due to whatever policy behavior is
chosen.

Thoughts?

--Ben