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 6568c4a37ec615e5a3dc4908377ad5dfd6661bde (commit) via e59d61d58a645c0ede270115d0b10286271881be (commit) via 8e93fee04e56cf8368340ba5170368fc478c412c (commit) via 854e9bac14dab38261ad85a0291fe03aae449fe4 (commit) via 15da32a5a55cd219a97e8b88742a2ea5145bfbd8 (commit) via 8d3a6c02ee0f88a8c70cde9f3b52c062b6968f43 (commit) via 06d6fc42053663c2bec34618f9132b7ac0402e30 (commit) from d67cc53a2836d98bfc7941bfc6b27472377d135e (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=6568c4a37ec615e5a3dc4908377ad5dfd6661bde commit 6568c4a37ec615e5a3dc4908377ad5dfd6661bde Merge: d67cc53 e59d61d Author: Ben Boeckel <ben.boec...@kitware.com> AuthorDate: Tue Aug 6 14:28:36 2013 -0400 Commit: CMake Topic Stage <kwro...@kitware.com> CommitDate: Tue Aug 6 14:28:36 2013 -0400 Merge topic 'dev/fix-variable-watch-crash' into next e59d61d Remove variable_watch watches on destruction 8e93fee Check newValue for NULL 854e9ba Don't share memory for variable_watch callbacks 15da32a Allow specifying the data to match in RemoveWatch 8d3a6c0 Match client_data as well for finding duplicates 06d6fc4 Add test for watching a variable multiple times http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=e59d61d58a645c0ede270115d0b10286271881be commit e59d61d58a645c0ede270115d0b10286271881be Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:18:58 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:55 2013 -0400 Remove variable_watch watches on destruction This cleans out old watches. Now that watches are completely separate, if we don't remove all of the old watches, the following happens: variable_watch(watched call1) variable_watch(watched call2) set(bar "${watched}") (in ccmake): configure configure makes the output: <configure> call1 call2 <configure> call1 call2 call1 call2 since the old watches are still valid. diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index 25bf450..bd161f9 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -101,6 +101,18 @@ cmVariableWatchCommand::cmVariableWatchCommand() } //---------------------------------------------------------------------------- +cmVariableWatchCommand::~cmVariableWatchCommand() +{ + std::set<std::string>::const_iterator it; + for ( it = this->WatchedVariables.begin(); it != this->WatchedVariables.end(); + ++it ) + { + this->Makefile->GetCMakeInstance()->GetVariableWatch()->RemoveWatch( + *it, cmVariableWatchCommandVariableAccessed); + } +} + +//---------------------------------------------------------------------------- bool cmVariableWatchCommand ::InitialPass(std::vector<std::string> const& args, cmExecutionStatus &) { @@ -128,6 +140,7 @@ bool cmVariableWatchCommand data->InCallback = false; data->Command = command; + this->WatchedVariables.insert(variable); if ( !this->Makefile->GetCMakeInstance()->GetVariableWatch()->AddWatch( variable, cmVariableWatchCommandVariableAccessed, data, deleteVariableWatchCallbackData) ) diff --git a/Source/cmVariableWatchCommand.h b/Source/cmVariableWatchCommand.h index 295a64a..545535c 100644 --- a/Source/cmVariableWatchCommand.h +++ b/Source/cmVariableWatchCommand.h @@ -32,6 +32,9 @@ public: //! Default constructor cmVariableWatchCommand(); + //! Destructor. + ~cmVariableWatchCommand(); + /** * This is called when the command is first encountered in * the CMakeLists.txt file. @@ -75,6 +78,9 @@ public: } cmTypeMacro(cmVariableWatchCommand, cmCommand); + +protected: + std::set<std::string> WatchedVariables; }; http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=8e93fee04e56cf8368340ba5170368fc478c412c commit 8e93fee04e56cf8368340ba5170368fc478c412c Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:32:19 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:19 2013 -0400 Check newValue for NULL On read access, newValue can be NULL since there is no new value, so use the empty string instead. diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index 63069d0..25bf450 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -80,7 +80,7 @@ static void cmVariableWatchCommandVariableAccessed( { cmOStringStream msg; msg << "Variable \"" << variable.c_str() << "\" was accessed using " - << accessString << " with value \"" << newValue << "\"."; + << accessString << " with value \"" << (newValue?newValue:"") << "\"."; makefile->IssueMessage(cmake::LOG, msg.str()); } http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=854e9bac14dab38261ad85a0291fe03aae449fe4 commit 854e9bac14dab38261ad85a0291fe03aae449fe4 Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:16:23 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:12 2013 -0400 Don't share memory for variable_watch callbacks Each callback is now a separate entity completely and doesn't rely on the command which spawned it at all. diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index 38ce25b..63069d0 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -14,72 +14,27 @@ #include "cmVariableWatch.h" //---------------------------------------------------------------------------- -static void cmVariableWatchCommandVariableAccessed( - const std::string& variable, int access_type, void* client_data, - const char* newValue, const cmMakefile* mf) -{ - cmVariableWatchCommand* command - = static_cast<cmVariableWatchCommand*>(client_data); - command->VariableAccessed(variable, access_type, newValue, mf); -} - -//---------------------------------------------------------------------------- -static void deleteVariableWatchCommand(void* client_data) -{ - cmVariableWatchCommand* command - = static_cast<cmVariableWatchCommand*>(client_data); - delete command; -} - -//---------------------------------------------------------------------------- -cmVariableWatchCommand::cmVariableWatchCommand() +struct cmVariableWatchCallbackData { - this->InCallback = false; -} + bool InCallback; + std::string Command; +}; //---------------------------------------------------------------------------- -bool cmVariableWatchCommand -::InitialPass(std::vector<std::string> const& args, cmExecutionStatus &) +static void cmVariableWatchCommandVariableAccessed( + const std::string& variable, int access_type, void* client_data, + const char* newValue, const cmMakefile* mf) { - if ( args.size() < 1 ) - { - this->SetError("must be called with at least one argument."); - return false; - } - std::string variable = args[0]; - if ( args.size() > 1 ) - { - std::string command = args[1]; - this->Handlers[variable].Commands.push_back(args[1]); - } - if ( variable == "CMAKE_CURRENT_LIST_FILE" ) - { - cmOStringStream ostr; - ostr << "cannot be set on the variable: " << variable.c_str(); - this->SetError(ostr.str().c_str()); - return false; - } - - this->Makefile->GetCMakeInstance()->GetVariableWatch()->AddWatch( - variable, cmVariableWatchCommandVariableAccessed, - this->Clone(), deleteVariableWatchCommand); + cmVariableWatchCallbackData* data + = static_cast<cmVariableWatchCallbackData*>(client_data); - return true; -} - -//---------------------------------------------------------------------------- -void cmVariableWatchCommand::VariableAccessed(const std::string& variable, - int access_type, const char* newValue, const cmMakefile* mf) -{ - if ( this->InCallback ) + if ( data->InCallback ) { return; } - this->InCallback = true; + data->InCallback = true; cmListFileFunction newLFF; - cmVariableWatchCommandHandler *handler = &this->Handlers[variable]; - cmVariableWatchCommandHandler::VectorOfCommands::iterator it; cmListFileArgument arg; bool processed = false; const char* accessString = cmVariableWatch::GetAccessAsString(access_type); @@ -89,10 +44,8 @@ void cmVariableWatchCommand::VariableAccessed(const std::string& variable, cmMakefile* makefile = const_cast<cmMakefile*>(mf); std::string stack = makefile->GetProperty("LISTFILE_STACK"); - for ( it = handler->Commands.begin(); it != handler->Commands.end(); - ++ it ) + if ( !data->Command.empty() ) { - std::string command = *it; newLFF.Arguments.clear(); newLFF.Arguments.push_back( cmListFileArgument(variable, true, "unknown", 9999)); @@ -104,7 +57,7 @@ void cmVariableWatchCommand::VariableAccessed(const std::string& variable, cmListFileArgument(currentListFile, true, "unknown", 9999)); newLFF.Arguments.push_back( cmListFileArgument(stack, true, "unknown", 9999)); - newLFF.Name = command; + newLFF.Name = data->Command; newLFF.FilePath = "Some weird path"; newLFF.Line = 9999; cmExecutionStatus status; @@ -116,9 +69,9 @@ void cmVariableWatchCommand::VariableAccessed(const std::string& variable, error << "Error in cmake code at\n" << arg.FilePath << ":" << arg.Line << ":\n" << "A command failed during the invocation of callback \"" - << command << "\"."; + << data->Command << "\"."; cmSystemTools::Error(error.str().c_str()); - this->InCallback = false; + data->InCallback = false; return; } processed = true; @@ -130,5 +83,58 @@ void cmVariableWatchCommand::VariableAccessed(const std::string& variable, << accessString << " with value \"" << newValue << "\"."; makefile->IssueMessage(cmake::LOG, msg.str()); } - this->InCallback = false; + + data->InCallback = false; +} + +//---------------------------------------------------------------------------- +static void deleteVariableWatchCallbackData(void* client_data) +{ + cmVariableWatchCallbackData* data + = static_cast<cmVariableWatchCallbackData*>(client_data); + delete data; +} + +//---------------------------------------------------------------------------- +cmVariableWatchCommand::cmVariableWatchCommand() +{ +} + +//---------------------------------------------------------------------------- +bool cmVariableWatchCommand +::InitialPass(std::vector<std::string> const& args, cmExecutionStatus &) +{ + if ( args.size() < 1 ) + { + this->SetError("must be called with at least one argument."); + return false; + } + std::string variable = args[0]; + std::string command; + if ( args.size() > 1 ) + { + command = args[1]; + } + if ( variable == "CMAKE_CURRENT_LIST_FILE" ) + { + cmOStringStream ostr; + ostr << "cannot be set on the variable: " << variable.c_str(); + this->SetError(ostr.str().c_str()); + return false; + } + + cmVariableWatchCallbackData* data = new cmVariableWatchCallbackData; + + data->InCallback = false; + data->Command = command; + + if ( !this->Makefile->GetCMakeInstance()->GetVariableWatch()->AddWatch( + variable, cmVariableWatchCommandVariableAccessed, + data, deleteVariableWatchCallbackData) ) + { + deleteVariableWatchCallbackData(data); + return false; + } + + return true; } diff --git a/Source/cmVariableWatchCommand.h b/Source/cmVariableWatchCommand.h index 0fca80f..295a64a 100644 --- a/Source/cmVariableWatchCommand.h +++ b/Source/cmVariableWatchCommand.h @@ -14,13 +14,6 @@ #include "cmCommand.h" -class cmVariableWatchCommandHandler -{ -public: - typedef std::vector<std::string> VectorOfCommands; - VectorOfCommands Commands; -}; - /** \class cmVariableWatchCommand * \brief Watch when the variable changes and invoke command * @@ -33,9 +26,7 @@ public: */ virtual cmCommand* Clone() { - cmVariableWatchCommand* cmd = new cmVariableWatchCommand; - cmd->Handlers = this->Handlers; - return cmd; + return new cmVariableWatchCommand; } //! Default constructor @@ -84,14 +75,6 @@ public: } cmTypeMacro(cmVariableWatchCommand, cmCommand); - - void VariableAccessed(const std::string& variable, int access_type, - const char* newValue, const cmMakefile* mf); - -protected: - std::map<std::string, cmVariableWatchCommandHandler> Handlers; - - bool InCallback; }; http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=15da32a5a55cd219a97e8b88742a2ea5145bfbd8 commit 15da32a5a55cd219a97e8b88742a2ea5145bfbd8 Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:12:54 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:00 2013 -0400 Allow specifying the data to match in RemoveWatch Now that watches are dependent on their client_data when adding, it also makes sense to allow matching the data for removal. diff --git a/Source/cmVariableWatch.cxx b/Source/cmVariableWatch.cxx index 9481c76..57eabe6 100644 --- a/Source/cmVariableWatch.cxx +++ b/Source/cmVariableWatch.cxx @@ -52,7 +52,7 @@ cmVariableWatch::~cmVariableWatch() } } -void cmVariableWatch::AddWatch(const std::string& variable, +bool cmVariableWatch::AddWatch(const std::string& variable, WatchMethod method, void* client_data /*=0*/, DeleteData delete_data /*=0*/) { @@ -69,14 +69,16 @@ void cmVariableWatch::AddWatch(const std::string& variable, client_data && client_data == pair->ClientData) { // Callback already exists - return; + return false; } } vp->push_back(p); + return true; } void cmVariableWatch::RemoveWatch(const std::string& variable, - WatchMethod method) + WatchMethod method, + void* client_data /*=0*/) { if ( !this->WatchMap.count(variable) ) { @@ -86,7 +88,10 @@ void cmVariableWatch::RemoveWatch(const std::string& variable, cmVariableWatch::VectorOfPairs::iterator it; for ( it = vp->begin(); it != vp->end(); ++it ) { - if ( (*it)->Method == method ) + if ( (*it)->Method == method && + // If client_data is NULL, we want to disconnect all watches against + // the given method; otherwise match ClientData as well. + (!client_data || (client_data == (*it)->ClientData))) { vp->erase(it); return; diff --git a/Source/cmVariableWatch.h b/Source/cmVariableWatch.h index d666815..790c75a 100644 --- a/Source/cmVariableWatch.h +++ b/Source/cmVariableWatch.h @@ -34,9 +34,10 @@ public: /** * Add watch to the variable */ - void AddWatch(const std::string& variable, WatchMethod method, + bool AddWatch(const std::string& variable, WatchMethod method, void* client_data=0, DeleteData delete_data=0); - void RemoveWatch(const std::string& variable, WatchMethod method); + void RemoveWatch(const std::string& variable, WatchMethod method, + void* client_data=0); /** * This method is called when variable is accessed http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=8d3a6c02ee0f88a8c70cde9f3b52c062b6968f43 commit 8d3a6c02ee0f88a8c70cde9f3b52c062b6968f43 Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:08:57 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:00 2013 -0400 Match client_data as well for finding duplicates If a callback has the same data as another call, we don't want to delete the old callback. This is because if the client_data is the same, it might get deleted causing the new client_data to be bogus. Now, AddWatch will return true if it will use the watch, false otherwise. diff --git a/Source/cmVariableWatch.cxx b/Source/cmVariableWatch.cxx index db36599..9481c76 100644 --- a/Source/cmVariableWatch.cxx +++ b/Source/cmVariableWatch.cxx @@ -65,10 +65,10 @@ void cmVariableWatch::AddWatch(const std::string& variable, for ( cc = 0; cc < vp->size(); cc ++ ) { cmVariableWatch::Pair* pair = (*vp)[cc]; - if ( pair->Method == method ) + if ( pair->Method == method && + client_data && client_data == pair->ClientData) { - delete pair; - (*vp)[cc] = p; + // Callback already exists return; } } http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=06d6fc42053663c2bec34618f9132b7ac0402e30 commit 06d6fc42053663c2bec34618f9132b7ac0402e30 Author: Ben Boeckel <maths...@gmail.com> AuthorDate: Tue Aug 6 14:31:28 2013 -0400 Commit: Ben Boeckel <maths...@gmail.com> CommitDate: Tue Aug 6 14:32:00 2013 -0400 Add test for watching a variable multiple times diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 6d1bca2..367c2b9 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -117,3 +117,4 @@ endif() add_RunCMake_test(File_Generate) add_RunCMake_test(ExportWithoutLanguage) add_RunCMake_test(target_link_libraries) +add_RunCMake_test(WatchTwice) diff --git a/Tests/RunCMake/WatchTwice/CMakeLists.txt b/Tests/RunCMake/WatchTwice/CMakeLists.txt new file mode 100644 index 0000000..e8db6b0 --- /dev/null +++ b/Tests/RunCMake/WatchTwice/CMakeLists.txt @@ -0,0 +1,3 @@ +cmake_minimum_required(VERSION 2.8) +project(${RunCMake_TEST} NONE) +include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/WatchTwice/RunCMakeTest.cmake b/Tests/RunCMake/WatchTwice/RunCMakeTest.cmake new file mode 100644 index 0000000..19205b2 --- /dev/null +++ b/Tests/RunCMake/WatchTwice/RunCMakeTest.cmake @@ -0,0 +1,3 @@ +include(RunCMake) + +run_cmake(WatchTwice) diff --git a/Tests/RunCMake/WatchTwice/WatchTwice-stderr.txt b/Tests/RunCMake/WatchTwice/WatchTwice-stderr.txt new file mode 100644 index 0000000..fa7d347 --- /dev/null +++ b/Tests/RunCMake/WatchTwice/WatchTwice-stderr.txt @@ -0,0 +1,2 @@ +From watch1 +From watch2 diff --git a/Tests/RunCMake/WatchTwice/WatchTwice.cmake b/Tests/RunCMake/WatchTwice/WatchTwice.cmake new file mode 100644 index 0000000..540cfc1 --- /dev/null +++ b/Tests/RunCMake/WatchTwice/WatchTwice.cmake @@ -0,0 +1,11 @@ +function (watch1) + message("From watch1") +endfunction () + +function (watch2) + message("From watch2") +endfunction () + +variable_watch(watched watch1) +variable_watch(watched watch2) +set(access "${watched}") ----------------------------------------------------------------------- Summary of changes: Source/cmVariableWatch.cxx | 19 ++- Source/cmVariableWatch.h | 5 +- Source/cmVariableWatchCommand.cxx | 147 +++++++++++--------- Source/cmVariableWatchCommand.h | 21 +-- Tests/RunCMake/CMakeLists.txt | 1 + .../{CMP0004 => WatchTwice}/CMakeLists.txt | 0 Tests/RunCMake/WatchTwice/RunCMakeTest.cmake | 3 + Tests/RunCMake/WatchTwice/WatchTwice-stderr.txt | 2 + Tests/RunCMake/WatchTwice/WatchTwice.cmake | 11 ++ 9 files changed, 120 insertions(+), 89 deletions(-) copy Tests/RunCMake/{CMP0004 => WatchTwice}/CMakeLists.txt (100%) create mode 100644 Tests/RunCMake/WatchTwice/RunCMakeTest.cmake create mode 100644 Tests/RunCMake/WatchTwice/WatchTwice-stderr.txt create mode 100644 Tests/RunCMake/WatchTwice/WatchTwice.cmake hooks/post-receive -- CMake _______________________________________________ Cmake-commits mailing list Cmake-commits@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-commits