From b10c5cbb878af541f4ffa35a9e287c31743c6e97 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 24 Apr 2012 16:24:17 -0400 Subject: [PATCH] CTest: Simplify environment save/restore Replace use of AppendEnv/RestoreEnv pairs with instances of SaveRestoreEnvironment. Simplify the signature of AppendEnv and use it in place of similar loops elsewhere. Move the RestoreEnv implementation inside the SaveRestoreEnvironment destructor which is the only place left that calls it. --- Source/CTest/cmCTestRunTest.cxx | 2 +- Source/CTest/cmCTestScriptHandler.cxx | 6 +-- Source/cmCTest.cxx | 23 ++++------ Source/cmSystemTools.cxx | 65 +++++++++------------------ Source/cmSystemTools.h | 12 +---- 5 files changed, 34 insertions(+), 74 deletions(-) diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 81f18b05c..c3de5dcf6 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -671,7 +671,7 @@ bool cmCTestRunTest::ForkProcess(double testTimeOut, bool explicitTimeout, if (environment && environment->size()>0) { - cmSystemTools::AppendEnv(environment); + cmSystemTools::AppendEnv(*environment); } return this->TestProcess->StartProcess(); diff --git a/Source/CTest/cmCTestScriptHandler.cxx b/Source/CTest/cmCTestScriptHandler.cxx index 5841b8d42..d3ab2efda 100644 --- a/Source/CTest/cmCTestScriptHandler.cxx +++ b/Source/CTest/cmCTestScriptHandler.cxx @@ -643,11 +643,7 @@ int cmCTestScriptHandler::RunCurrentScript() { std::vector envArgs; cmSystemTools::ExpandListArgument(this->CTestEnv.c_str(),envArgs); - // for each variable/argument do a putenv - for (unsigned i = 0; i < envArgs.size(); ++i) - { - cmSystemTools::PutEnv(envArgs[i].c_str()); - } + cmSystemTools::AppendEnv(envArgs); } // now that we have done most of the error checking finally run the diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index 3f7fdc7a2..4aff64bd8 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -48,7 +48,7 @@ #include #include -#include // auto_ptr +#include #include #include @@ -509,7 +509,7 @@ int cmCTest::Initialize(const char* binary_dir, cmCTestStartCommand* command) cmake cm; cmGlobalGenerator gg; gg.SetCMakeInstance(&cm); - std::auto_ptr lg(gg.CreateLocalGenerator()); + cmsys::auto_ptr lg(gg.CreateLocalGenerator()); cmMakefile *mf = lg->GetMakefile(); if ( !this->ReadCustomConfigurationFileTree(this->BinaryDir.c_str(), mf) ) { @@ -1277,7 +1277,6 @@ int cmCTest::RunTest(std::vector argv, std::ostream* log, double testTimeOut, std::vector* environment) { - std::vector origEnv; bool modifyEnv = (environment && environment->size()>0); // determine how much time we have @@ -1334,9 +1333,11 @@ int cmCTest::RunTest(std::vector argv, } std::string oldpath = cmSystemTools::GetCurrentWorkingDirectory(); + cmsys::auto_ptr saveEnv; if (modifyEnv) { - origEnv = cmSystemTools::AppendEnv(environment); + saveEnv.reset(new cmSystemTools::SaveRestoreEnvironment); + cmSystemTools::AppendEnv(*environment); } *retVal = inst.Run(args, output); @@ -1351,11 +1352,6 @@ int cmCTest::RunTest(std::vector argv, "Internal cmCTest object used to run test." << std::endl << *output << std::endl); - if (modifyEnv) - { - cmSystemTools::RestoreEnv(origEnv); - } - return cmsysProcess_State_Exited; } std::vector tempOutput; @@ -1364,9 +1360,11 @@ int cmCTest::RunTest(std::vector argv, *output = ""; } + cmsys::auto_ptr saveEnv; if (modifyEnv) { - origEnv = cmSystemTools::AppendEnv(environment); + saveEnv.reset(new cmSystemTools::SaveRestoreEnvironment); + cmSystemTools::AppendEnv(*environment); } cmsysProcess* cp = cmsysProcess_New(); @@ -1436,11 +1434,6 @@ int cmCTest::RunTest(std::vector argv, } cmsysProcess_Delete(cp); - if (modifyEnv) - { - cmSystemTools::RestoreEnv(origEnv); - } - return result; } diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 1376a485b..548fa7458 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1630,50 +1630,12 @@ std::vector cmSystemTools::GetEnvironmentVariables() } //---------------------------------------------------------------------- -std::vector cmSystemTools::AppendEnv( - std::vector* env) +void cmSystemTools::AppendEnv(std::vector const& env) { - std::vector origEnv = GetEnvironmentVariables(); - - if (env && env->size()>0) + for(std::vector::const_iterator eit = env.begin(); + eit != env.end(); ++eit) { - std::vector::const_iterator eit; - - for (eit = env->begin(); eit!= env->end(); ++eit) - { - PutEnv(eit->c_str()); - } - } - - return origEnv; -} - -//---------------------------------------------------------------------- -void cmSystemTools::RestoreEnv(const std::vector& env) -{ - std::vector::const_iterator eit; - - // First clear everything in the current environment: - // - std::vector currentEnv = GetEnvironmentVariables(); - for (eit = currentEnv.begin(); eit!= currentEnv.end(); ++eit) - { - std::string var(*eit); - - std::string::size_type pos = var.find("="); - if (pos != std::string::npos) - { - var = var.substr(0, pos); - } - - UnsetEnv(var.c_str()); - } - - // Then put back each entry from the original environment: - // - for (eit = env.begin(); eit!= env.end(); ++eit) - { - PutEnv(eit->c_str()); + cmSystemTools::PutEnv(eit->c_str()); } } @@ -1686,7 +1648,24 @@ cmSystemTools::SaveRestoreEnvironment::SaveRestoreEnvironment() //---------------------------------------------------------------------- cmSystemTools::SaveRestoreEnvironment::~SaveRestoreEnvironment() { - cmSystemTools::RestoreEnv(this->Env); + // First clear everything in the current environment: + std::vector currentEnv = GetEnvironmentVariables(); + for(std::vector::const_iterator + eit = currentEnv.begin(); eit != currentEnv.end(); ++eit) + { + std::string var(*eit); + + std::string::size_type pos = var.find("="); + if (pos != std::string::npos) + { + var = var.substr(0, pos); + } + + cmSystemTools::UnsetEnv(var.c_str()); + } + + // Then put back each entry from the original environment: + cmSystemTools::AppendEnv(this->Env); } #endif diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 5f21de2d2..69673c938 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -371,16 +371,8 @@ public: /** Get the list of all environment variables */ static std::vector GetEnvironmentVariables(); - /** Append multiple variables to the current environment. - Return the original environment, as it was before the - append. */ - static std::vector AppendEnv( - std::vector* env); - - /** Restore the full environment to "env" - use after - AppendEnv to put the environment back to the way it - was. */ - static void RestoreEnv(const std::vector& env); + /** Append multiple variables to the current environment. */ + static void AppendEnv(std::vector const& env); /** Helper class to save and restore the environment. Instantiate this class as an automatic variable on