Merge topic 'cache-truncate-newlines'

c42b377c cmCacheManager: Test and document newline truncation behavior
a9c48307 cmCacheManager: Truncate values containing newlines

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !1652

Help/release/dev/cache-newline.rst

@@ -0,0 +1,7 @@
+cache-newline
+-------------
+
+* Variables containing newlines in their values now get truncated before the
+  newline when they are written to the cache file. In addition, a warning
+  comment is written to the cache file, and a warning message is displayed to
+  the user on the console.

Source/cmCacheManager.cxx

@@ -10,6 +10,7 @@
 #include <string.h>
 
 #include "cmGeneratedFileStream.h"
+#include "cmMessenger.h"
 #include "cmState.h"
 #include "cmSystemTools.h"
 #include "cmVersion.h"
@@ -205,7 +206,8 @@ bool cmCacheManager::ReadPropertyEntry(std::string const& entryKey,
   return false;
 }
 
-void cmCacheManager::WritePropertyEntries(std::ostream& os, CacheIterator i)
+void cmCacheManager::WritePropertyEntries(std::ostream& os, CacheIterator i,
+                                          cmMessenger* messenger)
 {
   for (const char** p = this->PersistentProperties; *p; ++p) {
     if (const char* value = i.GetProperty(*p)) {
@@ -221,11 +223,13 @@ void cmCacheManager::WritePropertyEntries(std::ostream& os, CacheIterator i)
       os << ":INTERNAL=";
       this->OutputValue(os, value);
       os << "\n";
+      cmCacheManager::OutputNewlineTruncationWarning(os, key, value,
+                                                     messenger);
     }
   }
 }
 
-bool cmCacheManager::SaveCache(const std::string& path)
+bool cmCacheManager::SaveCache(const std::string& path, cmMessenger* messenger)
 {
   std::string cacheFile = path;
   cacheFile += "/CMakeCache.txt";
@@ -316,7 +320,10 @@ bool cmCacheManager::SaveCache(const std::string& path)
       this->OutputKey(fout, i.first);
       fout << ":" << cmState::CacheEntryTypeToString(t) << "=";
       this->OutputValue(fout, ce.Value);
-      fout << "\n\n";
+      fout << "\n";
+      cmCacheManager::OutputNewlineTruncationWarning(fout, i.first, ce.Value,
+                                                     messenger);
+      fout << "\n";
     }
   }
 
@@ -333,7 +340,7 @@ bool cmCacheManager::SaveCache(const std::string& path)
     }
 
     cmStateEnums::CacheEntryType t = i.GetType();
-    this->WritePropertyEntries(fout, i);
+    this->WritePropertyEntries(fout, i, messenger);
     if (t == cmStateEnums::INTERNAL) {
       // Format is key:type=value
       if (const char* help = i.GetProperty("HELPSTRING")) {
@@ -343,6 +350,8 @@ bool cmCacheManager::SaveCache(const std::string& path)
       fout << ":" << cmState::CacheEntryTypeToString(t) << "=";
       this->OutputValue(fout, i.GetValue());
       fout << "\n";
+      cmCacheManager::OutputNewlineTruncationWarning(fout, i.GetName(),
+                                                     i.GetValue(), messenger);
     }
   }
   fout << "\n";
@@ -389,6 +398,19 @@ void cmCacheManager::OutputKey(std::ostream& fout, std::string const& key)
 }
 
 void cmCacheManager::OutputValue(std::ostream& fout, std::string const& value)
+{
+  // look for and truncate newlines
+  std::string::size_type newline = value.find('\n');
+  if (newline != std::string::npos) {
+    std::string truncated = value.substr(0, newline);
+    OutputValueNoNewlines(fout, truncated);
+  } else {
+    OutputValueNoNewlines(fout, value);
+  }
+}
+
+void cmCacheManager::OutputValueNoNewlines(std::ostream& fout,
+                                           std::string const& value)
 {
   // if value has trailing space or tab, enclose it in single quotes
   if (!value.empty() &&
@@ -423,6 +445,50 @@ void cmCacheManager::OutputHelpString(std::ostream& fout,
   }
 }
 
+void cmCacheManager::OutputWarningComment(std::ostream& fout,
+                                          std::string const& message,
+                                          bool wrapSpaces)
+{
+  std::string::size_type end = message.size();
+  std::string oneLine;
+  std::string::size_type pos = 0;
+  for (std::string::size_type i = 0; i <= end; i++) {
+    if ((i == end) || (message[i] == '\n') ||
+        ((i - pos >= 60) && (message[i] == ' ') && wrapSpaces)) {
+      fout << "# ";
+      if (message[pos] == '\n') {
+        pos++;
+        fout << "\\n";
+      }
+      oneLine = message.substr(pos, i - pos);
+      fout << oneLine << "\n";
+      pos = i;
+    }
+  }
+}
+
+void cmCacheManager::OutputNewlineTruncationWarning(std::ostream& fout,
+                                                    std::string const& key,
+                                                    std::string const& value,
+                                                    cmMessenger* messenger)
+{
+  if (value.find('\n') != std::string::npos) {
+    if (messenger) {
+      std::string message = "Value of ";
+      message += key;
+      message += " contained a newline; truncating";
+      messenger->IssueMessage(cmake::WARNING, message);
+    }
+
+    std::string comment = "WARNING: Value of ";
+    comment += key;
+    comment += " contained a newline and was truncated. Original value:";
+
+    OutputWarningComment(fout, comment, true);
+    OutputWarningComment(fout, value, false);
+  }
+}
+
 void cmCacheManager::RemoveCacheEntry(const std::string& key)
 {
   CacheEntryMap::iterator i = this->Cache.find(key);

Source/cmCacheManager.h

@@ -15,7 +15,7 @@
 #include "cmPropertyMap.h"
 #include "cmStateTypes.h"
 
-class cmake;
+class cmMessenger;
 
 /** \class cmCacheManager
  * \brief Control class for cmake's cache
@@ -108,7 +108,7 @@ public:
                  std::set<std::string>& includes);
 
   ///! Save cache for given makefile.  Saves to output path/CMakeCache.txt
-  bool SaveCache(const std::string& path);
+  bool SaveCache(const std::string& path, cmMessenger* messenger);
 
   ///! Delete the cache given
   bool DeleteCache(const std::string& path);
@@ -218,16 +218,25 @@ protected:
   unsigned int CacheMinorVersion;
 
 private:
-  cmake* CMakeInstance;
   typedef std::map<std::string, CacheEntry> CacheEntryMap;
   static void OutputHelpString(std::ostream& fout,
                                const std::string& helpString);
+  static void OutputWarningComment(std::ostream& fout,
+                                   std::string const& message,
+                                   bool wrapSpaces);
+  static void OutputNewlineTruncationWarning(std::ostream& fout,
+                                             std::string const& key,
+                                             std::string const& value,
+                                             cmMessenger* messenger);
   static void OutputKey(std::ostream& fout, std::string const& key);
   static void OutputValue(std::ostream& fout, std::string const& value);
+  static void OutputValueNoNewlines(std::ostream& fout,
+                                    std::string const& value);
 
   static const char* PersistentProperties[];
   bool ReadPropertyEntry(std::string const& key, CacheEntry& e);
-  void WritePropertyEntries(std::ostream& os, CacheIterator i);
+  void WritePropertyEntries(std::ostream& os, CacheIterator i,
+                            cmMessenger* messenger);
 
   CacheEntryMap Cache;
   // Only cmake and cmState should be able to add cache values

Source/cmState.cxx

@@ -107,9 +107,9 @@ bool cmState::LoadCache(const std::string& path, bool internal,
   return this->CacheManager->LoadCache(path, internal, excludes, includes);
 }
 
-bool cmState::SaveCache(const std::string& path)
+bool cmState::SaveCache(const std::string& path, cmMessenger* messenger)
 {
-  return this->CacheManager->SaveCache(path);
+  return this->CacheManager->SaveCache(path, messenger);
 }
 
 bool cmState::DeleteCache(const std::string& path)

Source/cmState.h

@@ -23,6 +23,7 @@ class cmCacheManager;
 class cmCommand;
 class cmPropertyDefinition;
 class cmStateSnapshot;
+class cmMessenger;
 
 class cmState
 {
@@ -59,7 +60,7 @@ public:
                  std::set<std::string>& excludes,
                  std::set<std::string>& includes);
 
-  bool SaveCache(const std::string& path);
+  bool SaveCache(const std::string& path, cmMessenger* messenger);
 
   bool DeleteCache(const std::string& path);
 

Source/cmake.cxx

@@ -1771,7 +1771,7 @@ bool cmake::LoadCache(const std::string& path, bool internal,
 
 bool cmake::SaveCache(const std::string& path)
 {
-  bool result = this->State->SaveCache(path);
+  bool result = this->State->SaveCache(path, this->GetMessenger());
   static const char* entries[] = { "CMAKE_CACHE_MAJOR_VERSION",
                                    "CMAKE_CACHE_MINOR_VERSION",
                                    "CMAKE_CACHE_PATCH_VERSION",

Tests/RunCMake/CMakeLists.txt

@@ -339,6 +339,7 @@ add_RunCMake_test(CPackInstallProperties)
 add_RunCMake_test(ExternalProject)
 add_RunCMake_test(FetchContent)
 add_RunCMake_test(CTestCommandLine)
+add_RunCMake_test(CacheNewline)
 # Only run this test on unix platforms that support
 # symbolic links
 if(UNIX)

Tests/RunCMake/CacheNewline/CMakeLists.txt

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

Tests/RunCMake/CacheNewline/CacheNewline-check.cmake

@@ -0,0 +1,16 @@
+set(CACHE_EXPECTED_FILE "${RunCMake_TEST_SOURCE_DIR}/cache-regex.txt")
+set(CACHE_ACTUAL_FILE "${RunCMake_BINARY_DIR}/CacheNewline-build/CMakeCache.txt")
+
+file(READ ${CACHE_EXPECTED_FILE} CACHE_EXPECTED)
+string(REGEX REPLACE "\r\n" "\n" CACHE_EXPECTED "${CACHE_EXPECTED}")
+string(REGEX REPLACE "\n+$" "" CACHE_EXPECTED "${CACHE_EXPECTED}")
+file(READ ${CACHE_ACTUAL_FILE} CACHE_ACTUAL)
+string(REGEX REPLACE "\r\n" "\n" CACHE_ACTUAL "${CACHE_ACTUAL}")
+string(REGEX REPLACE "\n+$" "" CACHE_ACTUAL "${CACHE_ACTUAL}")
+
+if(NOT "${CACHE_ACTUAL}" MATCHES "${CACHE_EXPECTED}")
+  set(RunCMake_TEST_FAILED "${CACHE_ACTUAL_FILE} does not match ${CACHE_EXPECTED_FILE}:
+
+CMakeCache.txt contents = [\n${CACHE_ACTUAL}\n]
+Expected = [\n${CACHE_EXPECTED}\n]")
+endif()

Tests/RunCMake/CacheNewline/CacheNewline-stderr.txt

@@ -0,0 +1,2 @@
+CMake Warning:
+  Value of NEWLINE_VARIABLE contained a newline; truncating

Tests/RunCMake/CacheNewline/CacheNewline.cmake

@@ -0,0 +1,5 @@
+cmake_minimum_required(VERSION 3.5)
+
+project(CacheNewlineTest NONE)
+
+set(NEWLINE_VARIABLE "a\nb" CACHE STRING "Offending entry")

Tests/RunCMake/CacheNewline/RunCMakeTest.cmake

@@ -0,0 +1,3 @@
+include(RunCMake)
+
+run_cmake(CacheNewline)

Tests/RunCMake/CacheNewline/cache-regex.txt

@@ -0,0 +1,6 @@
+//Offending entry
+NEWLINE_VARIABLE:STRING=a
+# WARNING: Value of NEWLINE_VARIABLE contained a newline and was
+#  truncated\. Original value:
+# a
+# \\nb