BUG: improve bad argument handling for INCLUDE_DIRECTORIES and ADD_DEFINITIONS bug 4364

6 files changed, 111 insertions, 19 deletions

diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index a8c262a..e3f53a4 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -571,6 +571,16 @@ IF(BUILD_TESTING)
     --build-two-config
     --test-command cxxonly)
 
+  ADD_TEST(NewlineArgs  ${CMAKE_CTEST_COMMAND}
+    --build-and-test
+    "${CMake_SOURCE_DIR}/Tests/NewlineArgs"
+    "${CMake_BINARY_DIR}/Tests/NewlineArgs"
+    --build-generator ${CMAKE_TEST_GENERATOR}
+    --build-project cxxonly
+    --build-makeprogram ${CMAKE_TEST_MAKEPROGRAM}
+    --build-two-config
+    --test-command cxxonly)
+
   ADD_TEST(MacroTest ${CMAKE_CTEST_COMMAND}
     --build-and-test
     "${CMake_SOURCE_DIR}/Tests/MacroTest"
diff --git a/Source/cmIncludeDirectoryCommand.cxx b/Source/cmIncludeDirectoryCommand.cxx
index cf55f29..edccc9b 100644
--- a/Source/cmIncludeDirectoryCommand.cxx
+++ b/Source/cmIncludeDirectoryCommand.cxx
@@ -64,24 +64,77 @@ bool cmIncludeDirectoryCommand
         return 0;
         }
       }
-    std::string unixPath = *i;
-    if (!cmSystemTools::IsOff(unixPath.c_str()))
+
+    this->AddDirectory(i->c_str(),before,system);
+
+    }
+  return true;
+}
+
+// do a lot of cleanup on the arguments because this is one place where folks
+// sometimes take the output of a program and pass it directly into this
+// command not thinking that a single argument could be filled with spaces
+// and newlines etc liek below:
+//
+// "   /foo/bar
+//    /boo/hoo /dingle/berry "
+//
+// ideally that should be three seperate arguments but when sucking the
+// output from a program and passing it into a command the cleanup doesn't
+// always happen
+//
+void cmIncludeDirectoryCommand::AddDirectory(const char *i, 
+                                             bool before, 
+                                             bool system)
+{
+  // break apart any line feed arguments
+  std::string ret = i;
+  std::string::size_type pos = 0;
+  if((pos = ret.find('\n', pos)) != std::string::npos)
+    {
+    if (pos)
       {
-      cmSystemTools::ConvertToUnixSlashes(unixPath);
-      if(!cmSystemTools::FileIsFullPath(unixPath.c_str()))
-        {
-        std::string tmp = this->Makefile->GetStartDirectory();
-        tmp += "/";
-        tmp += unixPath;
-        unixPath = tmp;
-        }
+      this->AddDirectory(ret.substr(0,pos).c_str(), before, system);
       }
-    this->Makefile->AddIncludeDirectory(unixPath.c_str(), before);
-    if(system)
+    if (ret.size()-pos-1)
       {
-      this->Makefile->AddSystemIncludeDirectory(unixPath.c_str());
+      this->AddDirectory(ret.substr(pos+1,ret.size()-pos-1).c_str(), before, system);
       }
+    return;
+    }
+
+  // remove any leading or trailing spaces and \r
+  pos = ret.size()-1;
+  while(ret[pos] == ' ' || ret[pos] == '\r')
+    {
+    ret.erase(pos);
+    pos--;
+    }
+  pos = 0;
+  while(ret.size() && ret[pos] == ' ' || ret[pos] == '\r')
+    {
+    ret.erase(pos,1);
+    }
+  if (!ret.size())
+    {
+    return;
+    }
+  
+  if (!cmSystemTools::IsOff(ret.c_str()))
+    {
+    cmSystemTools::ConvertToUnixSlashes(ret);
+    if(!cmSystemTools::FileIsFullPath(ret.c_str()))
+      {
+      std::string tmp = this->Makefile->GetStartDirectory();
+      tmp += "/";
+      tmp += ret;
+      ret = tmp;
+      }
+    }
+  this->Makefile->AddIncludeDirectory(ret.c_str(), before);
+  if(system)
+    {
+    this->Makefile->AddSystemIncludeDirectory(ret.c_str());
     }
-  return true;
 }
 
diff --git a/Source/cmIncludeDirectoryCommand.h b/Source/cmIncludeDirectoryCommand.h
index 9247bbb..4c0be37 100644
--- a/Source/cmIncludeDirectoryCommand.h
+++ b/Source/cmIncludeDirectoryCommand.h
@@ -74,6 +74,10 @@ public:
     }
   
   cmTypeMacro(cmIncludeDirectoryCommand, cmCommand);
+
+protected:
+  // used internally
+  void AddDirectory(const char *arg, bool before, bool system);
 };
 
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index e9b6750..cce03b1 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -859,8 +859,28 @@ void cmMakefile::AddUtilityCommand(const char* utilityName, bool all,
 
 void cmMakefile::AddDefineFlag(const char* flag)
 {
+  if (!flag)
+    {
+    return;
+    }
+
+  // remove any \n\r
+  std::string ret = flag;
+  std::string::size_type pos = 0;
+  while((pos = ret.find('\n', pos)) != std::string::npos)
+    {
+    ret[pos] = ' ';
+    pos++;
+    }
+  pos = 0;
+  while((pos = ret.find('\r', pos)) != std::string::npos)
+    {
+    ret[pos] = ' ';
+    pos++;
+    }
+
   this->DefineFlags += " ";
-  this->DefineFlags += flag;
+  this->DefineFlags += ret;
 }
 
 
@@ -1111,6 +1131,12 @@ void cmMakefile::AddSubDirectory(const char* srcPath, const char *binPath,
 
 void cmMakefile::AddIncludeDirectory(const char* inc, bool before)
 {
+  // if there is a newline then break it into multiple arguments
+  if (!inc)
+    {
+    return;
+    }
+
   // Don't add an include directory that is already present.  Yes,
   // this linear search results in n^2 behavior, but n won't be
   // getting much bigger than 20.  We cannot use a set because of
diff --git a/Source/cmSeparateArgumentsCommand.cxx b/Source/cmSeparateArgumentsCommand.cxx
index 44ad5be..5f3d88d 100644
--- a/Source/cmSeparateArgumentsCommand.cxx
+++ b/Source/cmSeparateArgumentsCommand.cxx
@@ -31,8 +31,7 @@ bool cmSeparateArgumentsCommand
     return true;
     }
   std::string value = cacheValue;
-  cmSystemTools::ReplaceString(value,
-                               " ", ";");
+  cmSystemTools::ReplaceString(value," ", ";");
   this->Makefile->AddDefinition(args[0].c_str(), value.c_str());
   return true;
 }
diff --git a/Tests/NewlineArgs/CMakeLists.txt b/Tests/NewlineArgs/CMakeLists.txt
index d582639..ed9a341 100644
--- a/Tests/NewlineArgs/CMakeLists.txt
+++ b/Tests/NewlineArgs/CMakeLists.txt
@@ -4,8 +4,8 @@ project (newlineargs CXX)
 add_definitions("-DTEST_FLAG_1
 -DTEST_FLAG_2")
 
-include_directories("${newlineargs_BINARY_DIR} 
-${newlineargs_SOURCE_DIR}")
+include_directories(" ${newlineargs_BINARY_DIR} 
+ ${newlineargs_SOURCE_DIR} ")
 
 configure_file("${newlineargs_SOURCE_DIR}/libcxx2.h.in" 
   "${newlineargs_BINARY_DIR}/libcxx2.h")