This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "CMake".
The branch, next has been updated via e3db575e2e29789127fa40b2a7690aabd873a0b4 (commit) via 4d6715e4ed2993b6e9b1753683662e4f3d8907e8 (commit) via e0c0b1ace50f77f2a76dcc7020e3a4251bc6bf96 (commit) via 064c415d275433c332b7a38eb99df5d46aaa9f9e (commit) via 679cb9863aeebbc1072c24704348648c6e2a2710 (commit) via 61032bea1bcf23516fab87221890965263b3b317 (commit) from ab5e5a6292dbf461c3e53f57fe76bf0932dc50b1 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e3db575e2e29789127fa40b2a7690aabd873a0b4 commit e3db575e2e29789127fa40b2a7690aabd873a0b4 Merge: ab5e5a6 4d6715e Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 13:08:38 2014 -0400 Commit: CMake Topic Stage <kwro...@kitware.com> CommitDate: Fri Oct 24 13:08:38 2014 -0400 Merge topic 'revert-definition-map-lookup' into next 4d6715e4 Merge branch 'parent-scope-tests' into variable-pull-failure e0c0b1ac test: add a test for PARENT_SCOPE with multiple scopes 064c415d test: add test for PARENT_SCOPE behavior 679cb986 Revert "cmDefinitions: Don't store parent lookups" 61032bea cmDefinitions: only pull variables if they aren't local diff --cc Source/cmDefinitions.cxx index 5515f35,babf1c4..fe32dd5 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@@ -36,10 -35,11 +36,11 @@@ cmDefinitions::GetInternal(const std::s { return i->second; } - else if(cmDefinitions* up = this->Up) + if(cmDefinitions* up = this->Up) { - // Query the parent scope. - return up->GetInternal(key); + // Query the parent scope and store the result locally. + Def def = up->GetInternal(key); + return this->Map.insert(MapType::value_type(key, def)).first->second; } return this->NoDef; } http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=4d6715e4ed2993b6e9b1753683662e4f3d8907e8 commit 4d6715e4ed2993b6e9b1753683662e4f3d8907e8 Merge: 679cb98 e0c0b1a Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 13:00:28 2014 -0400 Commit: Ben Boeckel <ben.boec...@kitware.com> CommitDate: Fri Oct 24 13:00:28 2014 -0400 Merge branch 'parent-scope-tests' into variable-pull-failure * parent-scope-tests: test: add a test for PARENT_SCOPE with multiple scopes test: add test for PARENT_SCOPE behavior Conflicts: Tests/RunCMake/set/RunCMakeTest.cmake diff --cc Tests/RunCMake/set/RunCMakeTest.cmake index 1b51ea2,0b96b28..b8e8cf1 --- a/Tests/RunCMake/set/RunCMakeTest.cmake +++ b/Tests/RunCMake/set/RunCMakeTest.cmake @@@ -1,3 -1,5 +1,5 @@@ include(RunCMake) -run_cmake(PARENT_SCOPE) +run_cmake(ParentScope) + run_cmake(ParentPulling) + run_cmake(ParentPullingRecursive) http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e0c0b1ace50f77f2a76dcc7020e3a4251bc6bf96 commit e0c0b1ace50f77f2a76dcc7020e3a4251bc6bf96 Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 12:37:29 2014 -0400 Commit: Ben Boeckel <ben.boec...@kitware.com> CommitDate: Fri Oct 24 13:00:11 2014 -0400 test: add a test for PARENT_SCOPE with multiple scopes See the comment in the test for what is being tested here. diff --git a/Tests/RunCMake/set/ParentPullingRecursive-stderr.txt b/Tests/RunCMake/set/ParentPullingRecursive-stderr.txt new file mode 100644 index 0000000..f3260ae --- /dev/null +++ b/Tests/RunCMake/set/ParentPullingRecursive-stderr.txt @@ -0,0 +1,144 @@ +---------- +variable values at top before calls: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->top<-- +top_explicit_inner_unset: <undefined> +top_explicit_inner_tounset: -->top<-- +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: <undefined> +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: <undefined> +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: <undefined> +---------- +---------- +variable values at outer start: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->top<-- +top_explicit_inner_unset: <undefined> +top_explicit_inner_tounset: -->top<-- +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: <undefined> +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: <undefined> +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: <undefined> +---------- +---------- +variable values at outer before inner: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->top<-- +top_explicit_inner_unset: <undefined> +top_explicit_inner_tounset: -->top<-- +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: -->outer<-- +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: -->outer<-- +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: -->outer<-- +---------- +---------- +variable values at inner start: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->top<-- +top_explicit_inner_unset: <undefined> +top_explicit_inner_tounset: -->top<-- +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: -->outer<-- +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: -->outer<-- +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: -->outer<-- +---------- +---------- +variable values at inner end: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->top<-- +top_explicit_inner_unset: <undefined> +top_explicit_inner_tounset: -->top<-- +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: -->outer<-- +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: -->outer<-- +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: -->outer<-- +---------- +---------- +variable values at outer after inner: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->inner<-- +top_explicit_inner_unset: -->inner<-- +top_explicit_inner_tounset: <undefined> +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: -->outer<-- +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: -->inner<-- +outer_explicit_inner_unset: -->inner<-- +outer_explicit_inner_tounset: <undefined> +---------- +---------- +variable values at outer end: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->inner<-- +top_explicit_inner_unset: -->inner<-- +top_explicit_inner_tounset: <undefined> +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_set: -->top<-- +top_explicit_outer_unset: <undefined> +top_explicit_outer_tounset: -->top<-- +outer_implicit_inner_set: -->outer<-- +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: -->inner<-- +outer_explicit_inner_unset: -->inner<-- +outer_explicit_inner_tounset: <undefined> +---------- +---------- +variable values at top after calls: +top_implicit_inner_set: -->top<-- +top_implicit_inner_unset: <undefined> +top_explicit_inner_set: -->outer<-- +top_explicit_inner_unset: -->outer<-- +top_explicit_inner_tounset: <undefined> +top_implicit_outer_set: -->top<-- +top_explicit_outer_unset: -->outer<-- +top_explicit_outer_set: -->outer<-- +top_explicit_outer_unset: -->outer<-- +top_explicit_outer_tounset: <undefined> +outer_implicit_inner_set: <undefined> +outer_implicit_inner_unset: <undefined> +outer_explicit_inner_set: <undefined> +outer_explicit_inner_unset: <undefined> +outer_explicit_inner_tounset: <undefined> +---------- diff --git a/Tests/RunCMake/set/ParentPullingRecursive.cmake b/Tests/RunCMake/set/ParentPullingRecursive.cmake new file mode 100644 index 0000000..a3e29f5 --- /dev/null +++ b/Tests/RunCMake/set/ParentPullingRecursive.cmake @@ -0,0 +1,104 @@ +cmake_minimum_required(VERSION 3.0) +project(Minimal NONE) + +function(report where) + message("----------") + message("variable values at ${where}:") + foreach(var IN ITEMS + top_implicit_inner_set top_implicit_inner_unset + top_explicit_inner_set top_explicit_inner_unset top_explicit_inner_tounset + top_implicit_outer_set top_explicit_outer_unset + top_explicit_outer_set top_explicit_outer_unset top_explicit_outer_tounset + + outer_implicit_inner_set outer_implicit_inner_unset + outer_explicit_inner_set outer_explicit_inner_unset outer_explicit_inner_tounset) + if(DEFINED ${var}) + message("${var}: -->${${var}}<--") + else() + message("${var}: <undefined>") + endif() + endforeach() + message("----------") +endfunction() + +macro(set_values upscope downscope value) + # Pull the value in implicitly. + set(dummy ${${upscope}_implicit_${downscope}_set}) + set(dummy ${${upscope}_implicit_${downscope}_unset}) + # Pull it down explicitly. + set(${upscope}_explicit_${downscope}_set "${value}" PARENT_SCOPE) + set(${upscope}_explicit_${downscope}_unset "${value}" PARENT_SCOPE) + set(${upscope}_explicit_${downscope}_tounset PARENT_SCOPE) +endmacro() + +function(inner) + report("inner start") + + set_values(top inner inner) + set_values(outer inner inner) + + report("inner end") +endfunction() + +function(outer) + report("outer start") + + set_values(top outer outer) + + # Set values for inner to manipulate. + set(outer_implicit_inner_set outer) + set(outer_implicit_inner_unset) + set(outer_explicit_inner_set outer) + set(outer_explicit_inner_unset) + set(outer_explicit_inner_tounset outer) + + report("outer before inner") + + inner() + + report("outer after inner") + + # Do what inner does so that we can test the values that inner should have + # pulled through to here. + set_values(top inner outer) + + report("outer end") +endfunction() + +# variable name is: +# +# <upscope>_<pulltype>_<downscope>_<settype> +# +# where the value is the name of the scope it was set in. The scopes available +# are "top", "outer", and "inner". The pull type may either be "implicit" or +# "explicit" based on whether the pull is due to a variable dereference or a +# PARENT_SCOPE setting. The settype is "set" where both scopes set a value, +# "unset" where upscope unsets it and downscope sets it, and "tounset" where +# upscope sets it and downscope unsets it. +# +# We test the following combinations: +# +# - outer overriding top's values; +# - inner overriding top's values; +# - inner overriding outer's values; and +# - outer overriding inner's values in top after inner has run. + +# Set values for inner to manipulate. +set(top_implicit_inner_set top) +set(top_implicit_inner_unset) +set(top_explicit_inner_set top) +set(top_explicit_inner_unset) +set(top_explicit_inner_tounset top) + +# Set values for outer to manipulate. +set(top_implicit_outer_set top) +set(top_implicit_outer_unset) +set(top_explicit_outer_set top) +set(top_explicit_outer_unset) +set(top_explicit_outer_tounset top) + +report("top before calls") + +outer() + +report("top after calls") diff --git a/Tests/RunCMake/set/RunCMakeTest.cmake b/Tests/RunCMake/set/RunCMakeTest.cmake index 9caf53b..0b96b28 100644 --- a/Tests/RunCMake/set/RunCMakeTest.cmake +++ b/Tests/RunCMake/set/RunCMakeTest.cmake @@ -2,3 +2,4 @@ include(RunCMake) run_cmake(PARENT_SCOPE) run_cmake(ParentPulling) +run_cmake(ParentPullingRecursive) http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=064c415d275433c332b7a38eb99df5d46aaa9f9e commit 064c415d275433c332b7a38eb99df5d46aaa9f9e Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 10:47:36 2014 -0400 Commit: Ben Boeckel <ben.boec...@kitware.com> CommitDate: Fri Oct 24 13:00:11 2014 -0400 test: add test for PARENT_SCOPE behavior Test code courtesy of Alex Merry <alex.me...@kde.org>. diff --git a/Tests/RunCMake/set/ParentPulling-stderr.txt b/Tests/RunCMake/set/ParentPulling-stderr.txt new file mode 100644 index 0000000..768549b --- /dev/null +++ b/Tests/RunCMake/set/ParentPulling-stderr.txt @@ -0,0 +1,3 @@ +^before PARENT_SCOPE blah=value2 +after PARENT_SCOPE blah=value2 +in parent scope, blah=value2$ diff --git a/Tests/RunCMake/set/ParentPulling.cmake b/Tests/RunCMake/set/ParentPulling.cmake new file mode 100644 index 0000000..2614533 --- /dev/null +++ b/Tests/RunCMake/set/ParentPulling.cmake @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.0) +project(Minimal NONE) + +function(test_set) + set(blah "value2") + message("before PARENT_SCOPE blah=${blah}") + set(blah ${blah} PARENT_SCOPE) + message("after PARENT_SCOPE blah=${blah}") +endfunction() + +set(blah value1) +test_set() +message("in parent scope, blah=${blah}") diff --git a/Tests/RunCMake/set/RunCMakeTest.cmake b/Tests/RunCMake/set/RunCMakeTest.cmake index 5d036e3..9caf53b 100644 --- a/Tests/RunCMake/set/RunCMakeTest.cmake +++ b/Tests/RunCMake/set/RunCMakeTest.cmake @@ -1,3 +1,4 @@ include(RunCMake) run_cmake(PARENT_SCOPE) +run_cmake(ParentPulling) http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=679cb9863aeebbc1072c24704348648c6e2a2710 commit 679cb9863aeebbc1072c24704348648c6e2a2710 Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 12:58:12 2014 -0400 Commit: Ben Boeckel <ben.boec...@kitware.com> CommitDate: Fri Oct 24 12:58:47 2014 -0400 Revert "cmDefinitions: Don't store parent lookups" This reverts commit 5abfde6cb8a1ae0b2825797eab6c2e9842eb7c49. The behaviors associated with implicit pulldown on variable lookup seriously conflict with the optimizations made in these commits. Basically, since values were copied upon variable lookup, not just on PARENT_SCOPE, coupled with PARENT_SCOPE's behavior based on whether the variable is in the current scope or not causes serious problems with not storing a value for every variable at every scope. Reported-By: Ben Cooksley <bcooks...@kde.org> diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 7aca697..babf1c4 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -28,7 +28,7 @@ void cmDefinitions::Reset(cmDefinitions* parent) //---------------------------------------------------------------------------- cmDefinitions::Def const& -cmDefinitions::GetInternal(const std::string& key) const +cmDefinitions::GetInternal(const std::string& key) { MapType::const_iterator i = this->Map.find(key); if(i != this->Map.end()) @@ -37,8 +37,9 @@ cmDefinitions::GetInternal(const std::string& key) const } else if(cmDefinitions* up = this->Up) { - // Query the parent scope. - return up->GetInternal(key); + // Query the parent scope and store the result locally. + Def def = up->GetInternal(key); + return this->Map.insert(MapType::value_type(key, def)).first->second; } return this->NoDef; } @@ -70,26 +71,13 @@ cmDefinitions::SetInternal(const std::string& key, Def const& def) } //---------------------------------------------------------------------------- -const char* cmDefinitions::Get(const std::string& key) const +const char* cmDefinitions::Get(const std::string& key) { Def const& def = this->GetInternal(key); return def.Exists? def.c_str() : 0; } //---------------------------------------------------------------------------- -void cmDefinitions::Pull(const std::string& key) -{ - if (this->Up && this->Map.find(key) == this->Map.end()) - { - Def const& def = this->Up->GetInternal(key); - if (def.Exists) - { - this->SetInternal(key, def); - } - } -} - -//---------------------------------------------------------------------------- const char* cmDefinitions::Set(const std::string& key, const char* value) { Def const& def = this->SetInternal(key, Def(value)); diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index ebe6fa5..d615fb0 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -33,11 +33,9 @@ public: /** Returns the parent scope, if any. */ cmDefinitions* GetParent() const { return this->Up; } - /** Get the value associated with a key; null if none. */ - const char* Get(const std::string& key) const; - - /** Pull a variable from the parent. */ - void Pull(const std::string& key); + /** Get the value associated with a key; null if none. + Store the result locally if it came from a parent. */ + const char* Get(const std::string& key); /** Set (or unset if null) a value associated with a key. */ const char* Set(const std::string& key, const char* value); @@ -75,7 +73,7 @@ private: MapType Map; // Internal query and update methods. - Def const& GetInternal(const std::string& key) const; + Def const& GetInternal(const std::string& key); Def const& SetInternal(const std::string& key, Def const& def); // Implementation of Closure() method. diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 412c998..630957f 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4422,7 +4422,7 @@ void cmMakefile::RaiseScope(const std::string& var, const char *varDef) if(cmDefinitions* up = cur.GetParent()) { // First localize the definition in the current scope. - cur.Pull(var); + cur.Get(var); // Now update the definition in the parent scope. up->Set(var, varDef); http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=61032bea1bcf23516fab87221890965263b3b317 commit 61032bea1bcf23516fab87221890965263b3b317 Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Fri Oct 24 10:48:21 2014 -0400 Commit: Ben Boeckel <ben.boec...@kitware.com> CommitDate: Fri Oct 24 12:45:57 2014 -0400 cmDefinitions: only pull variables if they aren't local Prior to the commit which changed behavior here, PARENT_SCOPE triggered a ->GetInternal() call which would internally pull down the parent's value into the local scope. When converted over to an explicit ->Pull() call, the logic in ->GetInternal() which ignored the parent if it is already in scope was missed and instead unconditionally pulled down. Reported-By: Ben Cooksley <bcooks...@kde.org> diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 98becf8..7aca697 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -79,7 +79,7 @@ const char* cmDefinitions::Get(const std::string& key) const //---------------------------------------------------------------------------- void cmDefinitions::Pull(const std::string& key) { - if (this->Up) + if (this->Up && this->Map.find(key) == this->Map.end()) { Def const& def = this->Up->GetInternal(key); if (def.Exists) ----------------------------------------------------------------------- Summary of changes: Source/cmDefinitions.cxx | 22 +-- Source/cmDefinitions.h | 10 +- Source/cmMakefile.cxx | 2 +- Tests/RunCMake/set/ParentPulling-stderr.txt | 3 + Tests/RunCMake/set/ParentPulling.cmake | 13 ++ .../RunCMake/set/ParentPullingRecursive-stderr.txt | 144 ++++++++++++++++++++ Tests/RunCMake/set/ParentPullingRecursive.cmake | 104 ++++++++++++++ Tests/RunCMake/set/RunCMakeTest.cmake | 2 + 8 files changed, 276 insertions(+), 24 deletions(-) create mode 100644 Tests/RunCMake/set/ParentPulling-stderr.txt create mode 100644 Tests/RunCMake/set/ParentPulling.cmake create mode 100644 Tests/RunCMake/set/ParentPullingRecursive-stderr.txt create mode 100644 Tests/RunCMake/set/ParentPullingRecursive.cmake hooks/post-receive -- CMake _______________________________________________ Cmake-commits mailing list Cmake-commits@cmake.org http://public.kitware.com/mailman/listinfo/cmake-commits