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

[Robert Maynard]

I like the sounds of both Option 1, and Option 2. I feel Option 3 is really
bad.

We should remember that people understand/are taught the current CMake
behavior of local variables being preferred over cache variables.

If we move to Option 3, that rule becomes "local variables are preferred
over cache variables, and cache variables are not constructed if a local
variable exists ( even if unset? ) with the same name.". I can already
imagine people writing functions that try to set cache variables, but can't
since a local function variable is blocking them.




On Fri, Dec 11, 2015 at 2:44 PM, Ben Boeckel <ben.boeckel at kitware.com>
wrote:

> 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
> --
>
> Powered by www.kitware.com
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Kitware offers various services to support the CMake community. For more
> information on each offering, please visit:
>
> CMake Support: http://cmake.org/cmake/help/support.html
> CMake Consulting: http://cmake.org/cmake/help/consulting.html
> CMake Training Courses: http://cmake.org/cmake/help/training.html
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/cmake-developers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20151211/51869cfb/attachment.html>