Add support for caching stdout from MSVC (#962)

diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc
index 2fef6762bdb3c92ed965afa601e8b3a2603dc260..6ac6127a6296385f179629bc5dbf48bb45d32451 100644 (file)
--- a/doc/MANUAL.adoc
+++ b/doc/MANUAL.adoc
@@ -1317,6 +1317,8 @@ compilation.
 | Compiler produced stdout |
 The compiler wrote data to standard output. This is something that compilers
 normally never do, so ccache is not designed to store such output in the cache.
+For compilers that may normally write data to standard output, as such MSVC,
+the output is cached properly and it is not considered to be an error.
 
 | Could not find the compiler |
 The compiler to execute could not be found.

diff --git a/src/Result.cpp b/src/Result.cpp
index 22dfc0e408a4465eaf62861fbbd2de94db2bfd73..24a0dbe8e645e1985c906dc0cf90efc854926f78 100644 (file)
--- a/src/Result.cpp
+++ b/src/Result.cpp
@@ -153,6 +153,9 @@ file_type_to_string(FileType type)
 
   case FileType::coverage_mangled:
     return ".gcno-mangled";
+
+  case FileType::stdout_output:
+    return "<stdout>";
   }
 
   return k_unknown_file_type;

diff --git a/src/Result.hpp b/src/Result.hpp
index f36cbb6cd09ec7a1f75e6385523049a2f571c53c..cca900f2f95ba03cd7d673f284b08fbdac0536c1 100644 (file)
--- a/src/Result.hpp
+++ b/src/Result.hpp
@@ -55,7 +55,7 @@ enum class FileType : UnderlyingFileTypeInt {
   // Dependency file specified with -MF or implicitly from the output filename.
   dependency = 1,
 
-  // Text sent to standard output.
+  // Text sent to standard error output.
   stderr_output = 2,
 
   // Coverage notes file generated by -ftest-coverage with filename in unmangled
@@ -77,6 +77,9 @@ enum class FileType : UnderlyingFileTypeInt {
   // form, i.e. full output file path but with a .gcno extension and with
   // slashes replaced with hashes.
   coverage_mangled = 7,
+
+  // Text sent to standard output.
+  stdout_output = 8,
 };
 
 const char* file_type_to_string(FileType type);

diff --git a/src/ResultRetriever.cpp b/src/ResultRetriever.cpp
index 5298c711bd865a8f44a02241c445e782d28e9b0a..54b1f50b9c78d823bc090021b7eedf89d45d1aee 100644 (file)
--- a/src/ResultRetriever.cpp
+++ b/src/ResultRetriever.cpp
@@ -76,9 +76,10 @@ ResultRetriever::on_entry_start(uint32_t entry_number,
     }
     break;
 
+  case FileType::stdout_output:
   case FileType::stderr_output:
-    // Stderr data: Don't open a destination file. Instead accumulate it in
-    // m_dest_data and write it in on_entry_end.
+    // Stdout/stderr data: Don't open a destination file. Instead accumulate it
+    // in m_dest_data and write it in on_entry_end.
     m_dest_data.reserve(file_len);
     break;
 
@@ -114,7 +115,8 @@ ResultRetriever::on_entry_start(uint32_t entry_number,
     break;
   }
 
-  if (file_type == FileType::stderr_output) {
+  if (file_type == FileType::stdout_output
+      || file_type == FileType::stderr_output) {
     // Written in on_entry_end.
   } else if (dest_path.empty()) {
     LOG_RAW("Not writing");
@@ -141,9 +143,12 @@ ResultRetriever::on_entry_start(uint32_t entry_number,
 void
 ResultRetriever::on_entry_data(const uint8_t* data, size_t size)
 {
-  ASSERT(!(m_dest_file_type == FileType::stderr_output && m_dest_fd));
+  ASSERT(!((m_dest_file_type == FileType::stdout_output
+            || m_dest_file_type == FileType::stderr_output)
+           && m_dest_fd));
 
-  if (m_dest_file_type == FileType::stderr_output
+  if (m_dest_file_type == FileType::stdout_output
+      || m_dest_file_type == FileType::stderr_output
       || (m_dest_file_type == FileType::dependency && !m_dest_path.empty())) {
     m_dest_data.append(reinterpret_cast<const char*>(data), size);
   } else if (m_dest_fd) {
@@ -158,7 +163,10 @@ ResultRetriever::on_entry_data(const uint8_t* data, size_t size)
 void
 ResultRetriever::on_entry_end()
 {
-  if (m_dest_file_type == FileType::stderr_output) {
+  if (m_dest_file_type == FileType::stdout_output) {
+    LOG("Writing to file descriptor {}", STDOUT_FILENO);
+    Util::send_to_fd(m_ctx, m_dest_data, STDOUT_FILENO);
+  } else if (m_dest_file_type == FileType::stderr_output) {
     LOG("Writing to file descriptor {}", STDERR_FILENO);
     Util::send_to_fd(m_ctx, m_dest_data, STDERR_FILENO);
   } else if (m_dest_file_type == FileType::dependency && !m_dest_path.empty()) {

diff --git a/src/ccache.cpp b/src/ccache.cpp
index 413f17e73e3bacf0c7c884f23a20a7914bdc711b..a01f2bd88a6945f5595e433660db415fafc90e91 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -789,18 +789,25 @@ static bool
 write_result(Context& ctx,
              const std::string& result_path,
              const Stat& obj_stat,
+             const std::string& stdout_path,
              const std::string& stderr_path)
 {
   Result::Writer result_writer(ctx, result_path);
 
+  const auto stdout_stat = Stat::stat(stdout_path, Stat::OnError::log);
   const auto stderr_stat = Stat::stat(stderr_path, Stat::OnError::log);
-  if (!stderr_stat) {
+  if (!stdout_stat || !stderr_stat) {
     return false;
   }
 
   if (stderr_stat.size() > 0) {
     result_writer.write(Result::FileType::stderr_output, stderr_path);
   }
+  // Write stdout only after stderr (better with MSVC), as ResultRetriever
+  // will later print process them in the order they are read.
+  if (stdout_stat.size() > 0) {
+    result_writer.write(Result::FileType::stdout_output, stdout_path);
+  }
   if (obj_stat) {
     result_writer.write(Result::FileType::object, ctx.args_info.output_obj);
   }
@@ -936,24 +943,6 @@ to_cache(Context& ctx,
     return nonstd::make_unexpected(Statistic::missing_cache_file);
   }
 
-  // MSVC compiler always print the input file name to stdout,
-  // plus parts of the warnings/error messages.
-  // So we have to fusion that into stderr...
-  // Transform \r\n into \n. This way ninja won't produce empty newlines
-  // for the /showIncludes argument.
-  if (ctx.config.compiler_type() == CompilerType::cl) {
-    const std::string merged_output =
-      Util::read_file(tmp_stdout_path) + Util::read_file(tmp_stderr_path);
-    const std::string merged_output_with_unix_line_endings =
-      util::replace_all(merged_output, "\r\n", "\n");
-    try {
-      Util::write_file(tmp_stderr_path, merged_output_with_unix_line_endings);
-    } catch (const core::Error& e) {
-      LOG("Failed writing to {}: {}", tmp_stderr_path, e.what());
-      return nonstd::make_unexpected(Statistic::internal_error);
-    }
-  }
-
   // distcc-pump outputs lines like this:
   // __________Using # distcc servers in pump mode
   if (st.size() != 0 && ctx.config.compiler_type() != CompilerType::pump
@@ -962,8 +951,8 @@ to_cache(Context& ctx,
     return nonstd::make_unexpected(Statistic::compiler_produced_stdout);
   }
 
-  // Merge stderr from the preprocessor (if any) and stderr from the real
-  // compiler into tmp_stderr.
+  // Merge stderr from the preprocessor (if any) and stderr from
+  // the real compiler into tmp_stderr.
   if (!ctx.cpp_stderr.empty()) {
     std::string combined_stderr =
       Util::read_file(ctx.cpp_stderr) + Util::read_file(tmp_stderr_path);
@@ -975,6 +964,7 @@ to_cache(Context& ctx,
 
     // We can output stderr immediately instead of rerunning the compiler.
     Util::send_to_fd(ctx, Util::read_file(tmp_stderr_path), STDERR_FILENO);
+    Util::send_to_fd(ctx, Util::read_file(tmp_stdout_path), STDOUT_FILENO);
 
     auto failure = Failure(Statistic::compile_failed);
     failure.set_exit_code(*status);
@@ -1014,7 +1004,8 @@ to_cache(Context& ctx,
   MTR_BEGIN("result", "result_put");
   const bool added = ctx.storage.put(
     *result_key, core::CacheEntryType::result, [&](const auto& path) {
-      return write_result(ctx, path, obj_stat, tmp_stderr_path);
+      return write_result(
+        ctx, path, obj_stat, tmp_stdout_path, tmp_stderr_path);
     });
   MTR_END("result", "result_put");
   if (!added) {
@@ -1023,6 +1014,8 @@ to_cache(Context& ctx,
 
   // Everything OK.
   Util::send_to_fd(ctx, Util::read_file(tmp_stderr_path), STDERR_FILENO);
+  // Send stdout after stderr, it makes the output clearer with MSVC.
+  Util::send_to_fd(ctx, Util::read_file(tmp_stdout_path), STDOUT_FILENO);
 
   return *result_key;
 }

diff --git a/test/suites/base.bash b/test/suites/base.bash
index 8cc2e6cb776b88e64a8a3eaaa115689f1ffeb751..fb9d0ee93d93a4ba5aa9c9c9252b4a0aa01eb09c 100644 (file)
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -1212,6 +1212,26 @@ EOF
     expect_equal_content reference.stderr test.stderr
     expect_equal_content reference.d test.d
 
+    # -------------------------------------------------------------------------
+    TEST "No stdout"
+
+    cat <<EOF >test.c
+// just something random
+int test() { return 1; }
+EOF
+
+    stderr=$($CCACHE_COMPILE -c test.c >stdout 2>stderr)
+    expect_stat preprocessed_cache_hit 0
+    expect_stat cache_miss 1
+    expect_content stdout ""
+    expect_content stderr ""
+
+    stderr=$($CCACHE_COMPILE -c test.c >stdout 2>stderr)
+    expect_stat preprocessed_cache_hit 1
+    expect_stat cache_miss 1
+    expect_content stdout ""
+    expect_content stderr ""
+
     # -------------------------------------------------------------------------
     TEST "--zero-stats"