From: Joel Rosdahl <joel@rosdahl.net>
Date: Tue, 19 Jul 2022 12:24:22 +0000 (+0200)
Subject: fix: Log "bad output file" instead of "cache miss" on output file error
X-Git-Tag: v4.6.2~18
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50dab47b9cb7740e7bed3f0efca7cacc87e7d342;p=thirdparty%2Fccache.git

fix: Log "bad output file" instead of "cache miss" on output file error

(cherry picked from commit e08af162df0f921116c0883bcf4e43ca0ff078a3)
---

diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc
index ec58e13bf..8f14b5334 100644
--- a/doc/MANUAL.adoc
+++ b/doc/MANUAL.adoc
@@ -1313,8 +1313,7 @@ Preconditions for using <<Precompiled headers,precompiled headers>> were not
 fulfilled.
 
 | Could not write to output file |
-The output path specified with `-o` is not a file (e.g. a directory or a device
-node).
+The output path specified with `-o` could not be written to.
 
 | Compilation failed |
 The compilation failed. No result stored in the cache.
diff --git a/src/ResultRetriever.cpp b/src/ResultRetriever.cpp
index 28a043c49..43b8f1604 100644
--- a/src/ResultRetriever.cpp
+++ b/src/ResultRetriever.cpp
@@ -24,6 +24,7 @@
 
 #include <core/exceptions.hpp>
 #include <core/wincompat.hpp>
+#include <fmtmacros.hpp>
 #include <util/file.hpp>
 
 #include <fcntl.h>
@@ -128,8 +129,8 @@ ResultRetriever::on_entry_start(uint8_t entry_number,
     m_dest_fd = Fd(
       open(dest_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666));
     if (!m_dest_fd) {
-      throw core::Error(
-        "Failed to open {} for writing: {}", dest_path, strerror(errno));
+      throw WriteError(
+        FMT("Failed to open {} for writing: {}", dest_path, strerror(errno)));
     }
     m_dest_path = dest_path;
   }
@@ -150,7 +151,7 @@ ResultRetriever::on_entry_data(const uint8_t* data, size_t size)
     try {
       Util::write_fd(*m_dest_fd, data, size);
     } catch (core::Error& e) {
-      throw core::Error("Failed to write to {}: {}", m_dest_path, e.what());
+      throw WriteError(FMT("Failed to write to {}: {}", m_dest_path, e.what()));
     }
   }
 }
@@ -195,6 +196,6 @@ ResultRetriever::write_dependency_file()
                    m_dest_data.data() + start_pos,
                    m_dest_data.length() - start_pos);
   } catch (core::Error& e) {
-    throw core::Error("Failed to write to {}: {}", m_dest_path, e.what());
+    throw WriteError(FMT("Failed to write to {}: {}", m_dest_path, e.what()));
   }
 }
diff --git a/src/ResultRetriever.hpp b/src/ResultRetriever.hpp
index 98f0b9111..d4aa0ec6e 100644
--- a/src/ResultRetriever.hpp
+++ b/src/ResultRetriever.hpp
@@ -21,12 +21,19 @@
 #include "Fd.hpp"
 #include "Result.hpp"
 
+#include <core/exceptions.hpp>
+
 class Context;
 
 // This class retrieves a result entry to the local file system.
 class ResultRetriever : public Result::Reader::Consumer
 {
 public:
+  class WriteError : public core::Error
+  {
+    using core::Error::Error;
+  };
+
   ResultRetriever(Context& ctx, bool rewrite_dependency_target);
 
   void on_entry_start(uint8_t entry_number,
diff --git a/src/ccache.cpp b/src/ccache.cpp
index d9a42c2b7..d83fb1a80 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -1844,7 +1844,7 @@ calculate_result_and_manifest_key(Context& ctx,
 enum class FromCacheCallMode { direct, cpp };
 
 // Try to return the compile result from cache.
-static bool
+static nonstd::expected<bool, Failure>
 from_cache(Context& ctx, FromCacheCallMode mode, const Digest& result_key)
 {
   UmaskScope umask_scope(ctx.original_umask);
@@ -1887,6 +1887,10 @@ from_cache(Context& ctx, FromCacheCallMode mode, const Digest& result_key)
       ctx, should_rewrite_dependency_target(ctx.args_info));
 
     result_reader.read(result_retriever);
+  } catch (ResultRetriever::WriteError& e) {
+    LOG(
+      "Write error when retrieving result from {}: {}", *result_path, e.what());
+    return nonstd::make_unexpected(Statistic::bad_output_file);
   } catch (core::Error& e) {
     LOG("Failed to get result from {}: {}", *result_path, e.what());
     return false;
@@ -2256,9 +2260,11 @@ do_cache_compilation(Context& ctx, const char* const* argv)
     std::tie(result_key, manifest_key) = *result_and_manifest_key;
     if (result_key) {
       // If we can return from cache at this point then do so.
-      const bool found =
+      const auto from_cache_result =
         from_cache(ctx, FromCacheCallMode::direct, *result_key);
-      if (found) {
+      if (!from_cache_result) {
+        return nonstd::make_unexpected(from_cache_result.error());
+      } else if (*from_cache_result) {
         return Statistic::direct_cache_hit;
       }
 
@@ -2323,8 +2329,11 @@ do_cache_compilation(Context& ctx, const char* const* argv)
     }
 
     // If we can return from cache at this point then do.
-    const auto found = from_cache(ctx, FromCacheCallMode::cpp, *result_key);
-    if (found) {
+    const auto from_cache_result =
+      from_cache(ctx, FromCacheCallMode::cpp, *result_key);
+    if (!from_cache_result) {
+      return nonstd::make_unexpected(from_cache_result.error());
+    } else if (*from_cache_result) {
       if (ctx.config.direct_mode() && manifest_key && put_result_in_manifest) {
         update_manifest_file(ctx, *manifest_key, *result_key);
       }
diff --git a/test/suites/base.bash b/test/suites/base.bash
index 8c4622fd8..0c32fab5d 100644
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -1181,6 +1181,24 @@ EOF
     expect_stat preprocessed_cache_hit 1
     expect_stat cache_miss 1
 
+    # -------------------------------------------------------------------------
+    TEST "Failure to write output file"
+
+    mkdir dir
+
+    $CCACHE_COMPILE -c test1.c -o dir/test1.o
+    expect_stat preprocessed_cache_hit 0
+    expect_stat cache_miss 1
+    expect_stat bad_output_file 0
+
+    rm dir/test1.o
+    chmod a-w dir
+
+    $CCACHE_COMPILE -c test1.c -o dir/test1.o 2>/dev/null
+    expect_stat preprocessed_cache_hit 0
+    expect_stat cache_miss 1
+    expect_stat bad_output_file 1
+
     # -------------------------------------------------------------------------
     TEST "Caching stderr"