Merge topic 'refactor-try_compile-argument-processing'

d06db7e try_compile: Refactor argument processing
b680824 try_compile: Add test for bad call error cases

Source/cmCoreTryCompile.cxx

@@ -23,100 +23,67 @@ int cmCoreTryCompile::TryCompileCode(std::vector<std::string> 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<std::string> 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<std::string> 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<std::string> 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<cmTarget*> 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;
+      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]);
       }
-      extraArgs++;
-      ++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;
       }
+    else if(argv[i] == "OUTPUT_VARIABLE")
+      {
+      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()))
+        {
         switch(tgt->GetType())
           {
           case cmTarget::SHARED_LIBRARY:
@@ -135,38 +102,51 @@ int cmCoreTryCompile::TryCompileCode(std::vector<std::string> const& argv)
               "type " + tgt->GetTargetTypeName(tgt->GetType()) + ".");
             return -1;
           }
-        if (!tgt->IsImported())
+        if (tgt->IsImported())
           {
-          continue;
-          }
           targets.push_back(tgt);
           }
-      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());
       }
     }
 
-  // look for COPY_FILE
-  std::string copyFile;
-  for (i = 3; i < argv.size(); ++i)
-    {
-    if (argv[i] == "COPY_FILE")
-      {
-      if ( argv.size() <= (i+1) )
+  if(didCopyFile && copyFile.empty())
     {
     this->Makefile->IssueMessage(cmake::FATAL_ERROR,
-          "COPY_FILE specified but there is no variable");
+      "COPY_FILE must be followed by a file path");
     return -1;
     }
-      extraArgs += 2;
-      copyFile = argv[i+1];
-      break;
-      }
-    }
 
-  // do we have a srcfile signature
-  if (argv.size() - extraArgs == 3)
+  if(didOutputVariable && outputVariable.empty())
     {
-    this->SrcFileSignature = true;
+    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<std::string> 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<std::string> 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<std::string> 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();

Tests/RunCMake/CMakeLists.txt

@@ -76,6 +76,7 @@ add_RunCMake_test(if)
 add_RunCMake_test(include)
 add_RunCMake_test(include_directories)
 add_RunCMake_test(list)
+add_RunCMake_test(try_compile)
 add_RunCMake_test(CMP0004)
 
 find_package(Qt4 QUIET)

Tests/RunCMake/try_compile/BadLinkLibraries-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/BadLinkLibraries-stderr.txt

@@ -0,0 +1,5 @@
+CMake Error at BadLinkLibraries.cmake:2 \(try_compile\):
+  Only libraries may be used as try_compile IMPORTED LINK_LIBRARIES.  Got
+  not_a_library of type UTILITY.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/BadLinkLibraries.cmake

@@ -0,0 +1,3 @@
+add_custom_target(not_a_library)
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c
+  LINK_LIBRARIES not_a_library)

Tests/RunCMake/try_compile/CMakeLists.txt

@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 2.8)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)

Tests/RunCMake/try_compile/NoArgs-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/NoArgs-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at NoArgs.cmake:1 \(try_compile\):
+  try_compile unknown error.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/NoArgs.cmake

@@ -0,0 +1 @@
+try_compile()

Tests/RunCMake/try_compile/NoCopyFile-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/NoCopyFile-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at NoCopyFile.cmake:1 \(try_compile\):
+  COPY_FILE must be followed by a file path
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/NoCopyFile.cmake

@@ -0,0 +1,2 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c
+  COPY_FILE)

Tests/RunCMake/try_compile/NoCopyFile2-result.txt

@@ -0,0 +1 @@
+1

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\)

Tests/RunCMake/try_compile/NoCopyFile2.cmake

@@ -0,0 +1,2 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c
+  COPY_FILE CMAKE_FLAGS -DA=B)

Tests/RunCMake/try_compile/NoOutputVariable-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/NoOutputVariable-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at NoOutputVariable.cmake:1 \(try_compile\):
+  OUTPUT_VARIABLE must be followed by a variable name
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/NoOutputVariable.cmake

@@ -0,0 +1,2 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src.c
+  OUTPUT_VARIABLE)

Tests/RunCMake/try_compile/NoOutputVariable2-result.txt

@@ -0,0 +1 @@
+1

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\)

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)

Tests/RunCMake/try_compile/NonSourceCompileDefinitions-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/NonSourceCompileDefinitions-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at NonSourceCompileDefinitions.cmake:1 \(try_compile\):
+  COMPILE_DEFINITIONS specified on a srcdir type TRY_COMPILE
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/NonSourceCompileDefinitions.cmake

@@ -0,0 +1,2 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/proj
+  TestProject COMPILE_DEFINITIONS DEF)

Tests/RunCMake/try_compile/NonSourceCopyFile-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/NonSourceCopyFile-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at NonSourceCopyFile.cmake:1 \(try_compile\):
+  COPY_FILE specified on a srcdir type TRY_COMPILE
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/NonSourceCopyFile.cmake

@@ -0,0 +1,2 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/proj
+  TestProject COPY_FILE ${CMAKE_CURRENT_BINARY_DIR}/result)

Tests/RunCMake/try_compile/OneArg-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/OneArg-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at OneArg.cmake:1 \(try_compile\):
+  try_compile unknown error.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/OneArg.cmake

@@ -0,0 +1 @@
+try_compile(RESULT)

Tests/RunCMake/try_compile/RunCMakeTest.cmake

@@ -0,0 +1,12 @@
+include(RunCMake)
+
+run_cmake(NoArgs)
+run_cmake(OneArg)
+run_cmake(TwoArgs)
+run_cmake(NoCopyFile)
+run_cmake(NoCopyFile2)
+run_cmake(NoOutputVariable)
+run_cmake(NoOutputVariable2)
+run_cmake(BadLinkLibraries)
+run_cmake(NonSourceCopyFile)
+run_cmake(NonSourceCompileDefinitions)

Tests/RunCMake/try_compile/TwoArgs-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/try_compile/TwoArgs-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at TwoArgs.cmake:1 \(try_compile\):
+  try_compile unknown error.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)

Tests/RunCMake/try_compile/TwoArgs.cmake

@@ -0,0 +1 @@
+try_compile(RESULT ${CMAKE_CURRENT_BINARY_DIR})

Tests/RunCMake/try_compile/proj/CMakeLists.txt

@@ -0,0 +1,2 @@
+cmake_minimum_required(VERSION 2.8.10)
+project(TestProject NONE)

Tests/RunCMake/try_compile/src.c

@@ -0,0 +1 @@
+int main(void) { return 0; }