From d06db7ebe80636876d9701064b16cec9d3e2e3cb Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 May 2013 15:30:34 -0400 Subject: [PATCH] try_compile: Refactor argument processing Process all arguments in a single loop using a simple state machine. While at it, fix some error message typos. Also allow LINK_LIBRARIES with no actual libraries to disable use of the -DLINK_LIBRARIES=... from the CMAKE_FLAGS. This was already possible in the old logic if LINK_LIBRARIES was immediately followed by another keyword argument instead of the end of the argument list, so allow it in general. Update the RunCMake.try_compile test cases accordingly. --- Source/cmCoreTryCompile.cxx | 248 ++++++++---------- .../try_compile/NoCopyFile-stderr.txt | 2 +- ...ries-result.txt => NoCopyFile2-result.txt} | 0 .../try_compile/NoCopyFile2-stderr.txt | 4 + ...oLinkLibraries.cmake => NoCopyFile2.cmake} | 2 +- .../try_compile/NoLinkLibraries-stderr.txt | 4 - .../try_compile/NoOutputVariable-stderr.txt | 2 +- .../try_compile/NoOutputVariable2-result.txt | 1 + .../try_compile/NoOutputVariable2-stderr.txt | 4 + .../try_compile/NoOutputVariable2.cmake | 2 + .../NonSourceCompileDefinitions-stderr.txt | 2 +- Tests/RunCMake/try_compile/RunCMakeTest.cmake | 3 +- 12 files changed, 126 insertions(+), 148 deletions(-) rename Tests/RunCMake/try_compile/{NoLinkLibraries-result.txt => NoCopyFile2-result.txt} (100%) create mode 100644 Tests/RunCMake/try_compile/NoCopyFile2-stderr.txt rename Tests/RunCMake/try_compile/{NoLinkLibraries.cmake => NoCopyFile2.cmake} (72%) delete mode 100644 Tests/RunCMake/try_compile/NoLinkLibraries-stderr.txt create mode 100644 Tests/RunCMake/try_compile/NoOutputVariable2-result.txt create mode 100644 Tests/RunCMake/try_compile/NoOutputVariable2-stderr.txt create mode 100644 Tests/RunCMake/try_compile/NoOutputVariable2.cmake diff --git a/Source/cmCoreTryCompile.cxx b/Source/cmCoreTryCompile.cxx index 85e49a928..bf28428e9 100644 --- a/Source/cmCoreTryCompile.cxx +++ b/Source/cmCoreTryCompile.cxx @@ -23,150 +23,130 @@ int cmCoreTryCompile::TryCompileCode(std::vector const& argv) this->BinaryDirectory = argv[1].c_str(); this->OutputFile = ""; // which signature were we called with ? - this->SrcFileSignature = false; - unsigned int i; + this->SrcFileSignature = true; const char* sourceDirectory = argv[2].c_str(); const char* projectName = 0; const char* targetName = 0; - char targetNameBuf[64]; - int extraArgs = 0; - - // look for CMAKE_FLAGS and store them std::vector cmakeFlags; - for (i = 3; i < argv.size(); ++i) - { - if (argv[i] == "CMAKE_FLAGS") - { - // CMAKE_FLAGS is the first argument because we need an argv[0] that - // is not used, so it matches regular command line parsing which has - // the program name as arg 0 - for (; i < argv.size() && argv[i] != "COMPILE_DEFINITIONS" && - argv[i] != "OUTPUT_VARIABLE" && - argv[i] != "LINK_LIBRARIES"; - ++i) - { - extraArgs++; - cmakeFlags.push_back(argv[i]); - } - break; - } - } - - // look for OUTPUT_VARIABLE and store them + std::vector compileDefs; std::string outputVariable; - for (i = 3; i < argv.size(); ++i) - { - if (argv[i] == "OUTPUT_VARIABLE") - { - if ( argv.size() <= (i+1) ) - { - this->Makefile->IssueMessage(cmake::FATAL_ERROR, - "OUTPUT_VARIABLE specified but there is no variable"); - return -1; - } - extraArgs += 2; - outputVariable = argv[i+1]; - break; - } - } - - // look for COMPILE_DEFINITIONS and store them - std::vector compileFlags; - for (i = 3; i < argv.size(); ++i) - { - if (argv[i] == "COMPILE_DEFINITIONS") - { - extraArgs++; - for (i = i + 1; i < argv.size() && argv[i] != "CMAKE_FLAGS" && - argv[i] != "OUTPUT_VARIABLE" && - argv[i] != "LINK_LIBRARIES"; - ++i) - { - extraArgs++; - compileFlags.push_back(argv[i]); - } - break; - } - } - + std::string copyFile; std::vector targets; std::string libsToLink = " "; bool useOldLinkLibs = true; - for (i = 3; i < argv.size(); ++i) + char targetNameBuf[64]; + bool didOutputVariable = false; + bool didCopyFile = false; + + enum Doing { DoingNone, DoingCMakeFlags, DoingCompileDefinitions, + DoingLinkLibraries, DoingOutputVariable, DoingCopyFile }; + Doing doing = DoingNone; + for(size_t i=3; i < argv.size(); ++i) { - if (argv[i] == "LINK_LIBRARIES") + if(argv[i] == "CMAKE_FLAGS") { - if ( argv.size() <= (i+1) ) - { - this->Makefile->IssueMessage(cmake::FATAL_ERROR, - "LINK_LIBRARIES specified but there is no content"); - return -1; - } - extraArgs++; - ++i; + doing = DoingCMakeFlags; + // CMAKE_FLAGS is the first argument because we need an argv[0] that + // is not used, so it matches regular command line parsing which has + // the program name as arg 0 + cmakeFlags.push_back(argv[i]); + } + else if(argv[i] == "COMPILE_DEFINITIONS") + { + doing = DoingCompileDefinitions; + } + else if(argv[i] == "LINK_LIBRARIES") + { + doing = DoingLinkLibraries; useOldLinkLibs = false; - for ( ; i < argv.size() && argv[i] != "CMAKE_FLAGS" - && argv[i] != "COMPILE_DEFINITIONS" && argv[i] != "OUTPUT_VARIABLE"; - ++i) - { - extraArgs++; - libsToLink += "\"" + cmSystemTools::TrimWhitespace(argv[i]) + "\" "; - cmTarget *tgt = this->Makefile->FindTargetToUse(argv[i].c_str()); - if (!tgt) - { - continue; - } - switch(tgt->GetType()) - { - case cmTarget::SHARED_LIBRARY: - case cmTarget::STATIC_LIBRARY: - case cmTarget::UNKNOWN_LIBRARY: - break; - case cmTarget::EXECUTABLE: - if (tgt->IsExecutableWithExports()) - { - break; - } - default: - this->Makefile->IssueMessage(cmake::FATAL_ERROR, - "Only libraries may be used as try_compile IMPORTED " - "LINK_LIBRARIES. Got " + std::string(tgt->GetName()) + " of " - "type " + tgt->GetTargetTypeName(tgt->GetType()) + "."); - return -1; - } - if (!tgt->IsImported()) - { - continue; - } - targets.push_back(tgt); - } - break; } - } - - // look for COPY_FILE - std::string copyFile; - for (i = 3; i < argv.size(); ++i) - { - if (argv[i] == "COPY_FILE") + else if(argv[i] == "OUTPUT_VARIABLE") { - if ( argv.size() <= (i+1) ) + doing = DoingOutputVariable; + didOutputVariable = true; + } + else if(argv[i] == "COPY_FILE") + { + doing = DoingCopyFile; + didCopyFile = true; + } + else if(doing == DoingCMakeFlags) + { + cmakeFlags.push_back(argv[i]); + } + else if(doing == DoingCompileDefinitions) + { + compileDefs.push_back(argv[i]); + } + else if(doing == DoingLinkLibraries) + { + libsToLink += "\"" + cmSystemTools::TrimWhitespace(argv[i]) + "\" "; + if(cmTarget *tgt = this->Makefile->FindTargetToUse(argv[i].c_str())) { - this->Makefile->IssueMessage(cmake::FATAL_ERROR, - "COPY_FILE specified but there is no variable"); - return -1; + switch(tgt->GetType()) + { + case cmTarget::SHARED_LIBRARY: + case cmTarget::STATIC_LIBRARY: + case cmTarget::UNKNOWN_LIBRARY: + break; + case cmTarget::EXECUTABLE: + if (tgt->IsExecutableWithExports()) + { + break; + } + default: + this->Makefile->IssueMessage(cmake::FATAL_ERROR, + "Only libraries may be used as try_compile IMPORTED " + "LINK_LIBRARIES. Got " + std::string(tgt->GetName()) + " of " + "type " + tgt->GetTargetTypeName(tgt->GetType()) + "."); + return -1; + } + if (tgt->IsImported()) + { + targets.push_back(tgt); + } } - extraArgs += 2; - copyFile = argv[i+1]; - break; + } + else if(doing == DoingOutputVariable) + { + outputVariable = argv[i].c_str(); + doing = DoingNone; + } + else if(doing == DoingCopyFile) + { + copyFile = argv[i].c_str(); + doing = DoingNone; + } + else if(i == 3) + { + this->SrcFileSignature = false; + projectName = argv[i].c_str(); + } + else if(i == 4 && !this->SrcFileSignature) + { + targetName = argv[i].c_str(); + } + else + { + cmOStringStream m; + m << "try_compile given unknown argument \"" << argv[i] << "\"."; + this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, m.str()); } } - // do we have a srcfile signature - if (argv.size() - extraArgs == 3) + if(didCopyFile && copyFile.empty()) { - this->SrcFileSignature = true; + this->Makefile->IssueMessage(cmake::FATAL_ERROR, + "COPY_FILE must be followed by a file path"); + return -1; + } + + if(didOutputVariable && outputVariable.empty()) + { + this->Makefile->IssueMessage(cmake::FATAL_ERROR, + "OUTPUT_VARIABLE must be followed by a variable name"); + return -1; } // compute the binary dir when TRY_COMPILE is called with a src file @@ -179,10 +159,10 @@ int cmCoreTryCompile::TryCompileCode(std::vector const& argv) else { // only valid for srcfile signatures - if (compileFlags.size()) + if (compileDefs.size()) { this->Makefile->IssueMessage(cmake::FATAL_ERROR, - "COMPILE_FLAGS specified on a srcdir type TRY_COMPILE"); + "COMPILE_DEFINITIONS specified on a srcdir type TRY_COMPILE"); return -1; } if (copyFile.size()) @@ -297,12 +277,12 @@ int cmCoreTryCompile::TryCompileCode(std::vector const& argv) fprintf(fout, "SET(CMAKE_SUPPRESS_REGENERATION 1)\n"); fprintf(fout, "LINK_DIRECTORIES(${LINK_DIRECTORIES})\n"); // handle any compile flags we need to pass on - if (compileFlags.size()) + if (compileDefs.size()) { fprintf(fout, "ADD_DEFINITIONS( "); - for (i = 0; i < compileFlags.size(); ++i) + for (size_t i = 0; i < compileDefs.size(); ++i) { - fprintf(fout,"%s ",compileFlags[i].c_str()); + fprintf(fout,"%s ",compileDefs[i].c_str()); } fprintf(fout, ")\n"); } @@ -398,16 +378,6 @@ int cmCoreTryCompile::TryCompileCode(std::vector const& argv) } } - // else the srcdir bindir project target signature - else - { - projectName = argv[3].c_str(); - - if (argv.size() - extraArgs == 5) - { - targetName = argv[4].c_str(); - } - } bool erroroc = cmSystemTools::GetErrorOccuredFlag(); cmSystemTools::ResetErrorOccuredFlag(); diff --git a/Tests/RunCMake/try_compile/NoCopyFile-stderr.txt b/Tests/RunCMake/try_compile/NoCopyFile-stderr.txt index 096fd9827..d65d94883 100644 --- a/Tests/RunCMake/try_compile/NoCopyFile-stderr.txt +++ b/Tests/RunCMake/try_compile/NoCopyFile-stderr.txt @@ -1,4 +1,4 @@ CMake Error at NoCopyFile.cmake:1 \(try_compile\): - COPY_FILE specified but there is no variable + COPY_FILE must be followed by a file path Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/NoLinkLibraries-result.txt b/Tests/RunCMake/try_compile/NoCopyFile2-result.txt similarity index 100% rename from Tests/RunCMake/try_compile/NoLinkLibraries-result.txt rename to Tests/RunCMake/try_compile/NoCopyFile2-result.txt diff --git a/Tests/RunCMake/try_compile/NoCopyFile2-stderr.txt b/Tests/RunCMake/try_compile/NoCopyFile2-stderr.txt new file mode 100644 index 000000000..e88952475 --- /dev/null +++ b/Tests/RunCMake/try_compile/NoCopyFile2-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at NoCopyFile2.cmake:1 \(try_compile\): + COPY_FILE must be followed by a file path +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/NoLinkLibraries.cmake b/Tests/RunCMake/try_compile/NoCopyFile2.cmake similarity index 72% rename from Tests/RunCMake/try_compile/NoLinkLibraries.cmake rename to Tests/RunCMake/try_compile/NoCopyFile2.cmake index 04c57589c..04b7f6882 100644 --- a/Tests/RunCMake/try_compile/NoLinkLibraries.cmake +++ b/Tests/RunCMake/try_compile/NoCopyFile2.cmake @@ -1,2 +1,2 @@ try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c - LINK_LIBRARIES) + COPY_FILE CMAKE_FLAGS -DA=B) diff --git a/Tests/RunCMake/try_compile/NoLinkLibraries-stderr.txt b/Tests/RunCMake/try_compile/NoLinkLibraries-stderr.txt deleted file mode 100644 index 507dce076..000000000 --- a/Tests/RunCMake/try_compile/NoLinkLibraries-stderr.txt +++ /dev/null @@ -1,4 +0,0 @@ -CMake Error at NoLinkLibraries.cmake:1 \(try_compile\): - LINK_LIBRARIES specified but there is no content -Call Stack \(most recent call first\): - CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/NoOutputVariable-stderr.txt b/Tests/RunCMake/try_compile/NoOutputVariable-stderr.txt index 77ede4cb6..18ad751cf 100644 --- a/Tests/RunCMake/try_compile/NoOutputVariable-stderr.txt +++ b/Tests/RunCMake/try_compile/NoOutputVariable-stderr.txt @@ -1,4 +1,4 @@ CMake Error at NoOutputVariable.cmake:1 \(try_compile\): - OUTPUT_VARIABLE specified but there is no variable + OUTPUT_VARIABLE must be followed by a variable name Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/NoOutputVariable2-result.txt b/Tests/RunCMake/try_compile/NoOutputVariable2-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/try_compile/NoOutputVariable2-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/try_compile/NoOutputVariable2-stderr.txt b/Tests/RunCMake/try_compile/NoOutputVariable2-stderr.txt new file mode 100644 index 000000000..8b2cc254c --- /dev/null +++ b/Tests/RunCMake/try_compile/NoOutputVariable2-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at NoOutputVariable2.cmake:1 \(try_compile\): + OUTPUT_VARIABLE must be followed by a variable name +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/NoOutputVariable2.cmake b/Tests/RunCMake/try_compile/NoOutputVariable2.cmake new file mode 100644 index 000000000..ad9ac9ac5 --- /dev/null +++ b/Tests/RunCMake/try_compile/NoOutputVariable2.cmake @@ -0,0 +1,2 @@ +try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c + OUTPUT_VARIABLE CMAKE_FLAGS -DA=B) diff --git a/Tests/RunCMake/try_compile/NonSourceCompileDefinitions-stderr.txt b/Tests/RunCMake/try_compile/NonSourceCompileDefinitions-stderr.txt index cf02efb87..025e65868 100644 --- a/Tests/RunCMake/try_compile/NonSourceCompileDefinitions-stderr.txt +++ b/Tests/RunCMake/try_compile/NonSourceCompileDefinitions-stderr.txt @@ -1,4 +1,4 @@ CMake Error at NonSourceCompileDefinitions.cmake:1 \(try_compile\): - COMPILE_FLAGS specified on a srcdir type TRY_COMPILE + COMPILE_DEFINITIONS specified on a srcdir type TRY_COMPILE Call Stack \(most recent call first\): CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/try_compile/RunCMakeTest.cmake b/Tests/RunCMake/try_compile/RunCMakeTest.cmake index a4248ecf3..31643cfbc 100644 --- a/Tests/RunCMake/try_compile/RunCMakeTest.cmake +++ b/Tests/RunCMake/try_compile/RunCMakeTest.cmake @@ -4,8 +4,9 @@ run_cmake(NoArgs) run_cmake(OneArg) run_cmake(TwoArgs) run_cmake(NoCopyFile) +run_cmake(NoCopyFile2) run_cmake(NoOutputVariable) -run_cmake(NoLinkLibraries) +run_cmake(NoOutputVariable2) run_cmake(BadLinkLibraries) run_cmake(NonSourceCopyFile) run_cmake(NonSourceCompileDefinitions)