Fix broken dependency file when using base_dir

If a path in the dependency file matches base_dir, the dependency file
will be completely broken since newlines won’t be not retained by
use_relative_paths_in_depfile. Fix this by tokenizing lines instead of
the full file content once again.

Regression in aaadd43dbdf157ce20ab901fc9ce1c2e4924a73c (#555).

diff --git a/src/ccache.cpp b/src/ccache.cpp
index 264297c26dbdcf67dda7409fdbb23d3757bb8d26..ef97edc47eeff30341aed11cfefa19ad1697d00e 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -608,6 +608,54 @@ process_preprocessed_file(Context& ctx,
   return true;
 }
 
+nonstd::optional<std::string>
+rewrite_dep_file_paths(const Context& ctx, const std::string& file_content)
+{
+  ASSERT(!ctx.config.base_dir().empty());
+  ASSERT(ctx.has_absolute_include_headers);
+
+  // Fast path for the common case:
+  if (file_content.find(ctx.config.base_dir()) == std::string::npos) {
+    return nonstd::nullopt;
+  }
+
+  std::string adjusted_file_content;
+  adjusted_file_content.reserve(file_content.size());
+
+  bool content_rewritten = false;
+  for (const auto& line : Util::split_into_views(file_content, "\n")) {
+    const auto tokens = Util::split_into_views(line, " \t");
+    for (size_t i = 0; i < tokens.size(); ++i) {
+      DEBUG_ASSERT(line.length() > 0); // line.empty() -> no tokens
+      if (i > 0 || line[0] == ' ' || line[0] == '\t') {
+        adjusted_file_content.push_back(' ');
+      }
+
+      const auto& token = tokens[i];
+      bool token_rewritten = false;
+      if (Util::is_absolute_path(token)) {
+        const auto new_path = Util::make_relative_path(ctx, token);
+        if (new_path != token) {
+          adjusted_file_content.append(new_path);
+          token_rewritten = true;
+        }
+      }
+      if (token_rewritten) {
+        content_rewritten = true;
+      } else {
+        adjusted_file_content.append(token.begin(), token.end());
+      }
+    }
+    adjusted_file_content.push_back('\n');
+  }
+
+  if (content_rewritten) {
+    return adjusted_file_content;
+  } else {
+    return nonstd::nullopt;
+  }
+}
+
 // Replace absolute paths with relative paths in the provided dependency file.
 static void
 use_relative_paths_in_depfile(const Context& ctx)
@@ -629,33 +677,12 @@ use_relative_paths_in_depfile(const Context& ctx)
     log("Cannot open dependency file {}: {}", output_dep, e.what());
     return;
   }
-
-  std::string adjusted_file_content;
-  adjusted_file_content.reserve(file_content.size());
-
-  bool rewritten = false;
-
-  for (string_view token : Util::split_into_views(file_content, " \t\r\n")) {
-    if (Util::is_absolute_path(token)
-        && token.starts_with(ctx.config.base_dir())) {
-      adjusted_file_content.append(Util::make_relative_path(ctx, token));
-      rewritten = true;
-    } else {
-      adjusted_file_content.append(token.begin(), token.end());
-    }
-    adjusted_file_content.push_back(' ');
-  }
-
-  if (!rewritten) {
-    log(
-      "No paths in dependency file {} made relative, skip relative path usage",
-      output_dep);
-    return;
+  const auto new_content = rewrite_dep_file_paths(ctx, file_content);
+  if (new_content) {
+    Util::write_file(output_dep, *new_content);
+  } else {
+    log("No paths in dependency file {} made relative", output_dep);
   }
-
-  std::string tmp_file = output_dep + ".tmp";
-  Util::write_file(tmp_file, adjusted_file_content);
-  Util::rename(tmp_file, output_dep);
 }
 
 // Extract the used includes from the dependency file. Note that we cannot

diff --git a/src/ccache.hpp b/src/ccache.hpp
index 627f623b6cf86dedc1e545ba29ab0a400e447f98..bc8707f4f0e3cb98ca927739ce95200e220eeee4 100644 (file)
--- a/src/ccache.hpp
+++ b/src/ccache.hpp
@@ -21,6 +21,8 @@
 
 #include "system.hpp"
 
+#include "third_party/nonstd/optional.hpp"
+
 #include <functional>
 #include <string>
 
@@ -58,3 +60,5 @@ using FindExecutableFunction =
 // Tested by unit tests.
 void find_compiler(Context& ctx,
                    const FindExecutableFunction& find_executable_function);
+nonstd::optional<std::string>
+rewrite_dep_file_paths(const Context& ctx, const std::string& file_content);

diff --git a/unittest/test_ccache.cpp b/unittest/test_ccache.cpp
index 18b361f37dabb2b2f2dc7c6cc5b63a87c2e7eefa..7c4f9d4bc79fd2336503502ea3e58ae283aa85b8 100644 (file)
--- a/unittest/test_ccache.cpp
+++ b/unittest/test_ccache.cpp
@@ -144,4 +144,43 @@ TEST_CASE("find_compiler")
   }
 }
 
+TEST_CASE("rewrite_dep_file_paths")
+{
+  Context ctx;
+
+  const auto cwd = Util::get_actual_cwd();
+  ctx.actual_cwd = cwd;
+  ctx.apparent_cwd = cwd;
+  ctx.has_absolute_include_headers = true;
+
+  const auto content =
+    fmt::format("foo.o: bar.c {0}/bar.h \\\n {1}/fie.h {0}/fum.h\n",
+                cwd,
+                Util::dir_name(cwd));
+
+  SUBCASE("Base directory not in dep file content")
+  {
+    ctx.config.set_base_dir("/foo/bar");
+    CHECK(!rewrite_dep_file_paths(ctx, ""));
+    CHECK(!rewrite_dep_file_paths(ctx, content));
+  }
+
+  SUBCASE("Base directory in dep file content but not matching")
+  {
+    ctx.config.set_base_dir(fmt::format("{}/other", Util::dir_name(cwd)));
+    CHECK(!rewrite_dep_file_paths(ctx, ""));
+    CHECK(!rewrite_dep_file_paths(ctx, content));
+  }
+
+  SUBCASE("Absolute paths under base directory rewritten")
+  {
+    ctx.config.set_base_dir(cwd);
+    const auto actual = rewrite_dep_file_paths(ctx, content);
+    const auto expected = fmt::format(
+      "foo.o: bar.c ./bar.h \\\n {}/fie.h ./fum.h\n", Util::dir_name(cwd));
+    REQUIRE(actual);
+    CHECK(*actual == expected);
+  }
+}
+
 TEST_SUITE_END();