<div dir="ltr">I like the sounds of both Option 1, and Option 2. I feel Option 3 is really bad.<div><br></div><div>We should remember that people understand/are taught the current CMake behavior of local variables being preferred over cache variables. </div><div><br></div><div>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.</div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 11, 2015 at 2:44 PM, Ben Boeckel <span dir="ltr"><<a href="mailto:ben.boeckel@kitware.com" target="_blank">ben.boeckel@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Dec 10, 2015 at 08:50:10 -0500, Brad King wrote:<br>
</span><span class="">> BUG: change in how set cache overrides local definitions. Should mainly be a NOP change for most cases<br>
> <a href="https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f52d37c2" rel="noreferrer" target="_blank">https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f52d37c2</a><br>
<br>
</span>So Brad and I discussed today the reasons behind this change. Here is<br>
the thread behind the cause for the change:<br>
<br>
<a href="http://public.kitware.com/pipermail/cmake/2007-March/013198.html" rel="noreferrer" target="_blank">http://public.kitware.com/pipermail/cmake/2007-March/013198.html</a><br>
<br>
The problem was this pattern:<br>
<br>
function (set_cache var value)<br>
set("${var}" "${value}" CACHE INTERNAL "docs")<br>
endfunction ()<br>
<br>
set(some_var value CACHE INTERNAL "docs")<br>
set_cache(some_var other_value)<br>
message("${some_var}")<br>
<br>
Before this change, "value" would be output because the first set(CACHE)<br>
would set `some_var` to `value` in the local scope, but afterwards,<br>
because the variable is unset, the cache would be queried directly and<br>
"other_value" would be output.<br>
<br>
#######<br>
<br>
For work related to any changes to the interactions between the local<br>
scope and the cache, here is the baseline that must be met:<br>
<br>
The first configure must not differ from subsequent configures.<br>
<br>
set(CACHE) using either FORCE or the INTERNAL type do not have this<br>
problem currently since they always touch the local scope as well. I<br>
don't think anyone is going to disagree that the existing behavior is a<br>
problem in this regard, so the question is which behavior to prefer<br>
(whether set(CACHE) has other optional arguments to select a specific<br>
behavior or not is a separate question):<br>
<br>
Option 1:<br>
<br>
set(CACHE) always pulls from the cache into the local scope (first<br>
configure behavior is canonical)<br>
<br>
Option 2:<br>
<br>
set(CACHE) never touches the local scope (subsequent configure<br>
behavior is canonical)<br>
<br>
The main benefit of the first option is that reading the code is more<br>
"logical" in that set(CACHE) always does something to that variable<br>
(makes the cache's value available as ${var}). The value is<br>
unpredictable since the user can always set the cache to some bogus<br>
value (the STRINGS property or type hints will not save you because -D<br>
exists).<br>
<br>
The main benefit of the second option is that projects embedding<br>
external projects could override cache variables inside of that project<br>
without set(CACHE INTERNAL) (which doesn't help in the case of FORCE or<br>
INTERNAL variables that inner project uses anyways).<br>
<br>
A third option that Brad and I brainstormed after tracking down various<br>
bits of history and thinking about the behaviors is:<br>
<br>
Option 3:<br>
<br>
set(CACHE) (and any other cache-touching behavior) does *nothing*<br>
with the cache if a local variable by that name is defined<br>
<br>
This has the benefits of the second in that superset projects can set<br>
projects *and* hide cache values with a single set() command and also<br>
caches become less cluttered (e.g., if you set PYTHON_EXECUTABLE<br>
explicitly, the cache entry for FindPythonInterp doesn't appear and<br>
since the project is presumably forcing the value using a local<br>
variable, that is what is wanted anyways, so don't let the user mess<br>
that up).<br>
<br>
This policy would become an invasive change (there are 40 (+6 if you<br>
count cmCPluginAPI) call sites of cmMakefile::AddCacheDefinition that<br>
need audited for behavior changes due to whatever policy behavior is<br>
chosen.<br>
<br>
Thoughts?<br>
<span class="HOEnZb"><font color="#888888"><br>
--Ben<br>
</font></span><div class="HOEnZb"><div class="h5">--<br>
<br>
Powered by <a href="http://www.kitware.com" rel="noreferrer" target="_blank">www.kitware.com</a><br>
<br>
Please keep messages on-topic and check the CMake FAQ at: <a href="http://www.cmake.org/Wiki/CMake_FAQ" rel="noreferrer" target="_blank">http://www.cmake.org/Wiki/CMake_FAQ</a><br>
<br>
Kitware offers various services to support the CMake community. For more information on each offering, please visit:<br>
<br>
CMake Support: <a href="http://cmake.org/cmake/help/support.html" rel="noreferrer" target="_blank">http://cmake.org/cmake/help/support.html</a><br>
CMake Consulting: <a href="http://cmake.org/cmake/help/consulting.html" rel="noreferrer" target="_blank">http://cmake.org/cmake/help/consulting.html</a><br>
CMake Training Courses: <a href="http://cmake.org/cmake/help/training.html" rel="noreferrer" target="_blank">http://cmake.org/cmake/help/training.html</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" rel="noreferrer" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/mailman/listinfo/cmake-developers" rel="noreferrer" target="_blank">http://public.kitware.com/mailman/listinfo/cmake-developers</a><br>
</div></div></blockquote></div><br></div>