diff --git a/Source/CTest/cmCTestBuildCommand.cxx b/Source/CTest/cmCTestBuildCommand.cxx index 3aae03a6b8..d2460c9bcf 100644 --- a/Source/CTest/cmCTestBuildCommand.cxx +++ b/Source/CTest/cmCTestBuildCommand.cxx @@ -43,9 +43,9 @@ bool cmCTestBuildCommand::InitialPass(std::vector const& args, .Bind("PROJECT_NAME"_s, &BuildArguments::ProjectName) .Bind("PARALLEL_LEVEL"_s, &BuildArguments::ParallelLevel); - std::vector unparsedArguments; - BuildArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](BuildArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } std::unique_ptr cmCTestBuildCommand::InitializeHandler( diff --git a/Source/CTest/cmCTestConfigureCommand.cxx b/Source/CTest/cmCTestConfigureCommand.cxx index d1c84fbed6..42edf89782 100644 --- a/Source/CTest/cmCTestConfigureCommand.cxx +++ b/Source/CTest/cmCTestConfigureCommand.cxx @@ -170,7 +170,7 @@ bool cmCTestConfigureCommand::InitialPass(std::vector const& args, cmArgumentParser{ MakeHandlerParser() } // .Bind("OPTIONS"_s, &ConfigureArguments::Options); - std::vector unparsedArguments; - ConfigureArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](ConfigureArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Source/CTest/cmCTestCoverageCommand.cxx b/Source/CTest/cmCTestCoverageCommand.cxx index 0fa6501f68..93544c3d64 100644 --- a/Source/CTest/cmCTestCoverageCommand.cxx +++ b/Source/CTest/cmCTestCoverageCommand.cxx @@ -50,7 +50,7 @@ bool cmCTestCoverageCommand::InitialPass(std::vector const& args, cmArgumentParser{ MakeHandlerParser() } // .Bind("LABELS"_s, &CoverageArguments::Labels); - std::vector unparsedArguments; - CoverageArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](CoverageArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx index 67eade9758..16871f9b14 100644 --- a/Source/CTest/cmCTestHandlerCommand.cxx +++ b/Source/CTest/cmCTestHandlerCommand.cxx @@ -12,7 +12,6 @@ #include "cmCTestGenericHandler.h" #include "cmExecutionStatus.h" #include "cmMakefile.h" -#include "cmMessageType.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmValue.h" @@ -70,52 +69,55 @@ private: }; } -bool cmCTestHandlerCommand::ExecuteHandlerCommand( - HandlerArguments& args, std::vector const& unparsedArguments, - cmExecutionStatus& status) +bool cmCTestHandlerCommand::InvokeImpl( + BasicArguments& args, std::vector const& unparsed, + cmExecutionStatus& status, std::function handler) { // save error state and restore it if needed SaveRestoreErrorState errorState; - - // Process input arguments. - this->CheckArguments(args); - - std::sort(args.ParsedKeywords.begin(), args.ParsedKeywords.end()); - auto it = - std::adjacent_find(args.ParsedKeywords.begin(), args.ParsedKeywords.end()); - if (it != args.ParsedKeywords.end()) { - this->Makefile->IssueMessage( - MessageType::FATAL_ERROR, - cmStrCat("Called with more than one value for ", *it)); - } - - bool const foundBadArgument = !unparsedArguments.empty(); - if (foundBadArgument) { - this->SetError(cmStrCat("called with unknown argument \"", - unparsedArguments.front(), "\".")); - } - bool const captureCMakeError = !args.CaptureCMakeError.empty(); - // now that arguments are parsed check to see if there is a - // CAPTURE_CMAKE_ERROR specified let the errorState object know. - if (captureCMakeError) { + if (!args.CaptureCMakeError.empty()) { errorState.CaptureCMakeError(); } - // if we found a bad argument then exit before running command - if (foundBadArgument) { - // store the cmake error - if (captureCMakeError) { - this->Makefile->AddDefinition(args.CaptureCMakeError, "-1"); - std::string const err = this->GetName() + " " + status.GetError(); - if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { - cmCTestLog(this->CTest, ERROR_MESSAGE, err << " error from command\n"); - } - // return success because failure is recorded in CAPTURE_CMAKE_ERROR - return true; + + bool success = [&]() -> bool { + std::sort(args.ParsedKeywords.begin(), args.ParsedKeywords.end()); + auto const it = std::adjacent_find(args.ParsedKeywords.begin(), + args.ParsedKeywords.end()); + if (it != args.ParsedKeywords.end()) { + status.SetError(cmStrCat("called with more than one value for ", *it)); + return false; } - // return failure because of bad argument - return false; + + if (!unparsed.empty()) { + status.SetError( + cmStrCat("called with unknown argument \"", unparsed.front(), "\".")); + return false; + } + + return handler(); + }(); + + if (args.CaptureCMakeError.empty()) { + return success; } + if (!success) { + cmCTestLog(this->CTest, ERROR_MESSAGE, + this->GetName() << ' ' << status.GetError() << '\n'); + } + + cmMakefile& mf = status.GetMakefile(); + success = success && !cmSystemTools::GetErrorOccurredFlag(); + mf.AddDefinition(args.CaptureCMakeError, success ? "0" : "-1"); + return true; +} + +bool cmCTestHandlerCommand::ExecuteHandlerCommand( + HandlerArguments& args, cmExecutionStatus& /*status*/) +{ + // Process input arguments. + this->CheckArguments(args); + // Set the config type of this ctest to the current value of the // CTEST_CONFIGURATION_TYPE script variable if it is defined. // The current script value trumps the -C argument on the command @@ -165,14 +167,6 @@ bool cmCTestHandlerCommand::ExecuteHandlerCommand( cmCTestLog(this->CTest, ERROR_MESSAGE, "Cannot instantiate test handler " << this->GetName() << std::endl); - if (captureCMakeError) { - this->Makefile->AddDefinition(args.CaptureCMakeError, "-1"); - std::string const& err = status.GetError(); - if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { - cmCTestLog(this->CTest, ERROR_MESSAGE, err << " error from command\n"); - } - return true; - } return false; } @@ -186,13 +180,6 @@ bool cmCTestHandlerCommand::ExecuteHandlerCommand( this->CTest->GetCTestConfiguration("BuildDirectory")); if (workdir.Failed()) { this->SetError(workdir.GetError()); - if (captureCMakeError) { - this->Makefile->AddDefinition(args.CaptureCMakeError, "-1"); - cmCTestLog(this->CTest, ERROR_MESSAGE, - this->GetName() << " " << status.GetError() << "\n"); - // return success because failure is recorded in CAPTURE_CMAKE_ERROR - return true; - } return false; } @@ -205,21 +192,6 @@ bool cmCTestHandlerCommand::ExecuteHandlerCommand( this->Makefile->AddDefinition(args.ReturnValue, std::to_string(res)); } this->ProcessAdditionalValues(handler.get(), args); - // log the error message if there was an error - if (captureCMakeError) { - const char* returnString = "0"; - if (cmSystemTools::GetErrorOccurredFlag()) { - returnString = "-1"; - std::string const& err = status.GetError(); - // print out the error if it is not "unknown error" which means - // there was no message - if (!cmSystemTools::FindLastString(err.c_str(), "unknown error.")) { - cmCTestLog(this->CTest, ERROR_MESSAGE, err); - } - } - // store the captured cmake error state 0 or -1 - this->Makefile->AddDefinition(args.CaptureCMakeError, returnString); - } return true; } diff --git a/Source/CTest/cmCTestHandlerCommand.h b/Source/CTest/cmCTestHandlerCommand.h index c5534b91e0..1cc30ba432 100644 --- a/Source/CTest/cmCTestHandlerCommand.h +++ b/Source/CTest/cmCTestHandlerCommand.h @@ -4,8 +4,10 @@ #include "cmConfigure.h" // IWYU pragma: keep +#include #include #include +#include #include #include @@ -23,12 +25,25 @@ public: using cmCTestCommand::cmCTestCommand; protected: - struct HandlerArguments + struct BasicArguments { + std::string CaptureCMakeError; std::vector ParsedKeywords; + }; + + template + static auto MakeBasicParser() -> cmArgumentParser + { + static_assert(std::is_base_of::value, ""); + return cmArgumentParser{} + .Bind("CAPTURE_CMAKE_ERROR"_s, &BasicArguments::CaptureCMakeError) + .BindParsedKeywords(&BasicArguments::ParsedKeywords); + } + + struct HandlerArguments : BasicArguments + { bool Append = false; bool Quiet = false; - std::string CaptureCMakeError; std::string ReturnValue; std::string Build; std::string Source; @@ -38,11 +53,10 @@ protected: template static auto MakeHandlerParser() -> cmArgumentParser { - return cmArgumentParser{} - .BindParsedKeywords(&HandlerArguments::ParsedKeywords) + static_assert(std::is_base_of::value, ""); + return cmArgumentParser{ MakeBasicParser() } .Bind("APPEND"_s, &HandlerArguments::Append) .Bind("QUIET"_s, &HandlerArguments::Quiet) - .Bind("CAPTURE_CMAKE_ERROR"_s, &HandlerArguments::CaptureCMakeError) .Bind("RETURN_VALUE"_s, &HandlerArguments::ReturnValue) .Bind("SOURCE"_s, &HandlerArguments::Source) .Bind("BUILD"_s, &HandlerArguments::Build) @@ -50,11 +64,25 @@ protected: } protected: + template + bool Invoke(cmArgumentParser const& parser, + std::vector const& arguments, + cmExecutionStatus& status, Handler handler) + { + std::vector unparsed; + Args args = parser.Parse(arguments, &unparsed); + return this->InvokeImpl(args, unparsed, status, + [&]() -> bool { return handler(args); }); + }; + bool ExecuteHandlerCommand(HandlerArguments& args, - std::vector const& unparsedArguments, cmExecutionStatus& status); private: + bool InvokeImpl(BasicArguments& args, + std::vector const& unparsed, + cmExecutionStatus& status, std::function handler); + virtual std::string GetName() const = 0; virtual void CheckArguments(HandlerArguments& arguments); diff --git a/Source/CTest/cmCTestMemCheckCommand.cxx b/Source/CTest/cmCTestMemCheckCommand.cxx index f5ce12a2b2..2cb2549593 100644 --- a/Source/CTest/cmCTestMemCheckCommand.cxx +++ b/Source/CTest/cmCTestMemCheckCommand.cxx @@ -64,7 +64,7 @@ bool cmCTestMemCheckCommand::InitialPass(std::vector const& args, cmArgumentParser{ MakeTestParser() } .Bind("DEFECT_COUNT"_s, &MemCheckArguments::DefectCount); - std::vector unparsedArguments; - MemCheckArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](MemCheckArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Source/CTest/cmCTestSubmitCommand.cxx b/Source/CTest/cmCTestSubmitCommand.cxx index 4c1dc57fd0..6b1973176a 100644 --- a/Source/CTest/cmCTestSubmitCommand.cxx +++ b/Source/CTest/cmCTestSubmitCommand.cxx @@ -207,10 +207,10 @@ bool cmCTestSubmitCommand::InitialPass(std::vector const& args, bool const cdashUpload = !args.empty() && args[0] == "CDASH_UPLOAD"; auto const& parser = cdashUpload ? uploadParser : partsParser; - std::vector unparsedArguments; - SubmitArguments arguments = parser.Parse(args, &unparsedArguments); - arguments.CDashUpload = cdashUpload; - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](SubmitArguments& a) -> bool { + a.CDashUpload = cdashUpload; + return this->ExecuteHandlerCommand(a, status); + }); } void cmCTestSubmitCommand::CheckArguments(HandlerArguments& arguments) diff --git a/Source/CTest/cmCTestTestCommand.cxx b/Source/CTest/cmCTestTestCommand.cxx index fbbd8921dc..60621a7929 100644 --- a/Source/CTest/cmCTestTestCommand.cxx +++ b/Source/CTest/cmCTestTestCommand.cxx @@ -158,7 +158,7 @@ bool cmCTestTestCommand::InitialPass(std::vector const& args, { static auto const parser = MakeTestParser(); - std::vector unparsedArguments; - TestArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](TestArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Source/CTest/cmCTestUpdateCommand.cxx b/Source/CTest/cmCTestUpdateCommand.cxx index bc57e67242..e42027a73d 100644 --- a/Source/CTest/cmCTestUpdateCommand.cxx +++ b/Source/CTest/cmCTestUpdateCommand.cxx @@ -6,7 +6,6 @@ #include -#include "cmArgumentParser.h" #include "cmCTest.h" #include "cmCTestGenericHandler.h" #include "cmCTestUpdateHandler.h" @@ -103,7 +102,7 @@ bool cmCTestUpdateCommand::InitialPass(std::vector const& args, { static auto const parser = MakeHandlerParser(); - std::vector unparsedArguments; - HandlerArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](HandlerArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Source/CTest/cmCTestUploadCommand.cxx b/Source/CTest/cmCTestUploadCommand.cxx index bf7b0832c4..1e4c02e022 100644 --- a/Source/CTest/cmCTestUploadCommand.cxx +++ b/Source/CTest/cmCTestUploadCommand.cxx @@ -61,7 +61,7 @@ bool cmCTestUploadCommand::InitialPass(std::vector const& args, .Bind("FILES"_s, &UploadArguments::Files) .Bind("QUIET"_s, &UploadArguments::Quiet); - std::vector unparsedArguments; - UploadArguments arguments = parser.Parse(args, &unparsedArguments); - return this->ExecuteHandlerCommand(arguments, unparsedArguments, status); + return this->Invoke(parser, args, status, [&](UploadArguments& a) { + return this->ExecuteHandlerCommand(a, status); + }); } diff --git a/Tests/RunCMake/ctest_submit/RepeatRETURN_VALUE-stderr.txt b/Tests/RunCMake/ctest_submit/RepeatRETURN_VALUE-stderr.txt index 6e17c759ee..3951524ace 100644 --- a/Tests/RunCMake/ctest_submit/RepeatRETURN_VALUE-stderr.txt +++ b/Tests/RunCMake/ctest_submit/RepeatRETURN_VALUE-stderr.txt @@ -1,2 +1,2 @@ CMake Error at .*/Tests/RunCMake/ctest_submit/RepeatRETURN_VALUE/test.cmake:[0-9]+ \(ctest_submit\): - Called with more than one value for RETURN_VALUE + ctest_submit called with more than one value for RETURN_VALUE