Merge topic 'ninja_bad_cmcldeps_paths'

9275554 Ninja: Update BuildDepends test to verify cmcldeps depfiles.
6fa9d0a Ninja: Make cmcldeps depfile output more consistent with 'ninja -t msvc'

Source/cmcldeps.cxx

@@ -62,8 +62,8 @@ static std::string trimLeadingSpace(const std::string& cmdline) {
   return cmdline.substr(i);
 }
 
-static void doEscape(std::string& str, const std::string& search,
-                                       const std::string& repl) {
+static void replaceAll(std::string& str, const std::string& search,
+                                         const std::string& repl) {
   std::string::size_type pos = 0;
   while ((pos = str.find(search, pos)) != std::string::npos) {
     str.replace(pos, search.size(), repl);
@@ -71,6 +71,10 @@ static void doEscape(std::string& str, const std::string& search,
   }
 }
 
+bool startsWith(const std::string& str, const std::string& what) {
+  return str.compare(0, what.size(), what) == 0;
+}
+
 // Strips one argument from the cmdline and returns it. "surrounding quotes"
 // are removed from the argument if there were any.
 static std::string getArg(std::string& cmdline) {
@@ -117,6 +121,13 @@ static void parseCommandLine(LPTSTR wincmdline,
   rest = trimLeadingSpace(cmdline);
 }
 
+// Not all backslashes need to be escaped in a depfile, but it's easier that
+// way.  See the re2c grammar in ninja's source code for more info.
+static void escapePath(std::string &path) {
+  replaceAll(path, "\\", "\\\\");
+  replaceAll(path, " ", "\\ ");
+}
+
 static void outputDepFile(const std::string& dfile, const std::string& objfile,
         std::vector<std::string>& incs) {
 
@@ -132,16 +143,24 @@ static void outputDepFile(const std::string& dfile, const std::string& objfile,
   // FIXME should this be fatal or not? delete obj? delete d?
   if (!out)
     return;
+  std::string cwd = cmSystemTools::GetCurrentWorkingDirectory();
+  replaceAll(cwd, "/", "\\");
+  cwd += "\\";
 
   std::string tmp = objfile;
-  doEscape(tmp, " ", "\\ ");
+  escapePath(tmp);
   fprintf(out, "%s: \\\n", tmp.c_str());
 
   std::vector<std::string>::iterator it = incs.begin();
   for (; it != incs.end(); ++it) {
     tmp = *it;
-    doEscape(tmp, "\\", "/");
-    doEscape(tmp, " ", "\\ ");
+    // The paths need to match the ones used to identify build artifacts in the
+    // build.ninja file.  Therefore we need to canonicalize the path to use
+    // backward slashes and relativize the path to the build directory.
+    replaceAll(tmp, "/", "\\");
+    if (startsWith(tmp, cwd))
+      tmp = tmp.substr(cwd.size());
+    escapePath(tmp);
     fprintf(out, "%s \\\n", tmp.c_str());
   }
 
@@ -150,10 +169,6 @@ static void outputDepFile(const std::string& dfile, const std::string& objfile,
 }
 
 
-bool startsWith(const std::string& str, const std::string& what) {
-  return str.compare(0, what.size(), what) == 0;
-}
-
 bool contains(const std::string& str, const std::string& what) {
   return str.find(what) != std::string::npos;
 }

Tests/BuildDepends/CMakeLists.txt

@@ -53,6 +53,8 @@ write_file(${BuildDepends_BINARY_DIR}/Project/foo.cxx
 
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/zot.hxx.in
   "static const char* zot = \"zot\";\n")
+file(WRITE ${BuildDepends_BINARY_DIR}/Project/dir/header.txt
+  "#define HEADER_STRING \"ninja\"\n" )
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/zot_custom.hxx.in
   "static const char* zot_custom = \"zot_custom\";\n")
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/zot_macro_dir.hxx
@@ -93,6 +95,26 @@ if(NOT RESULT)
   message(SEND_ERROR "Could not build test project (1)!")
 endif()
 
+# find and save the ninjadep executable
+set(ninjadep ${BuildDepends_BINARY_DIR}/Project/ninjadep${CMAKE_EXECUTABLE_SUFFIX})
+if(EXISTS
+    "${BuildDepends_BINARY_DIR}/Project/Debug/ninjadep${CMAKE_EXECUTABLE_SUFFIX}" )
+  message("found debug")
+  set(ninjadep
+    "${BuildDepends_BINARY_DIR}/Project/Debug/ninjadep${CMAKE_EXECUTABLE_SUFFIX}")
+endif()
+message("Running ${ninjadep}  ")
+execute_process(COMMAND ${ninjadep} OUTPUT_VARIABLE out RESULT_VARIABLE runResult)
+string(REGEX REPLACE "[\r\n]" " " out "${out}")
+message("Run result: ${runResult} Output: \"${out}\"")
+
+if("${out}" STREQUAL "HEADER_STRING: ninja ")
+  message("Worked!")
+else()
+  message(SEND_ERROR "Project did not rebuild properly. Output[${out}]\n"
+    " expected [HEADER_STRING: ninja]")
+endif()
+
 set(bar ${BuildDepends_BINARY_DIR}/Project/bar${CMAKE_EXECUTABLE_SUFFIX})
 if(EXISTS
     "${BuildDepends_BINARY_DIR}/Project/Debug/bar${CMAKE_EXECUTABLE_SUFFIX}" )
@@ -151,6 +173,8 @@ execute_process(COMMAND ${bar} -infinite TIMEOUT 3 OUTPUT_VARIABLE out)
 message("Modifying Project/foo.cxx")
 write_file(${BuildDepends_BINARY_DIR}/Project/foo.cxx
   "const char* foo() { return \"foo changed\";}" )
+file(WRITE "${BuildDepends_BINARY_DIR}/Project/dir/header.txt"
+  "#define HEADER_STRING \"ninja changed\"\n" )
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/zot.hxx.in
   "static const char* zot = \"zot changed\";\n")
 file(WRITE ${BuildDepends_BINARY_DIR}/Project/zot_custom.hxx.in
@@ -204,6 +228,18 @@ if(EXISTS
   message("found debug")
 endif()
 
+message("Running ${ninjadep}  ")
+execute_process(COMMAND ${ninjadep} OUTPUT_VARIABLE out RESULT_VARIABLE runResult)
+string(REGEX REPLACE "[\r\n]" " " out "${out}")
+message("Run result: ${runResult} Output: \"${out}\"")
+
+if("${out}" STREQUAL "HEADER_STRING: ninja changed ")
+  message("Worked!")
+else()
+  message(SEND_ERROR "Project did not rebuild properly. Output[${out}]\n"
+    " expected [HEADER_STRING: ninja changed]")
+endif()
+
 message("Running ${bar}  ")
 execute_process(COMMAND ${bar} OUTPUT_VARIABLE out RESULT_VARIABLE runResult)
 string(REGEX REPLACE "[\r\n]" " " out "${out}")

Tests/BuildDepends/Project/CMakeLists.txt

@@ -123,3 +123,19 @@ add_custom_target(link_depends_no_shared_check ALL
    -P ${CMAKE_CURRENT_SOURCE_DIR}/link_depends_no_shared_check.cmake
   )
 add_dependencies(link_depends_no_shared_check link_depends_no_shared_exe)
+
+add_custom_command(
+  OUTPUT  ${CMAKE_CURRENT_BINARY_DIR}/dir/header.h
+  DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/dir/header.txt
+  COMMAND ${CMAKE_COMMAND} -E copy_if_different
+  ${CMAKE_CURRENT_BINARY_DIR}/dir/header.txt
+  ${CMAKE_CURRENT_BINARY_DIR}/dir/header.h
+  )
+
+set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/dir/header.h
+  PROPERTIES GENERATED 1)
+
+add_custom_target(header_tgt DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/dir/header.h)
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+add_executable(ninjadep ninjadep.cpp)
+add_dependencies(ninjadep header_tgt)

Tests/BuildDepends/Project/ninjadep.cpp

@@ -0,0 +1,6 @@
+#include <stdio.h>
+#include "dir/header.h"
+
+int main() {
+  printf("HEADER_STRING: %s\n", HEADER_STRING);
+}