depfile,dyndep: reconcile relative and absolute paths with build manifest

doc/manual.asciidoc

@@ -589,6 +589,13 @@ build edges such that it is not an error if the listed dependency is
 missing.  This allows you to delete a header file and rebuild without
 the build aborting due to a missing input.
 
+Paths emitted by the compiler may be either absolute or relative paths,
+depending on compiler behavior and the include directory flags given to it.
+Paths loaded from a depfile are matched to paths in build statements
+if they match exactly.  _Since Ninja 1.11_, paths are also matched if
+one of them is relative and matches the other when prefixed by the
+working directory.
+
 depfile
 ^^^^^^^
 
@@ -650,11 +657,6 @@ rule cc
   command = cl /showIncludes -c $in /Fo$out
 ----
 
-If the include directory directives are using absolute paths, your depfile
-may result in a mixture of relative and absolute paths. Paths used by other
-build rules need to match exactly. Therefore, it is recommended to use
-relative paths in these cases.
-
 [[ref_pool]]
 Pools
 ~~~~~
@@ -997,8 +999,11 @@ header file before starting a subsequent compilation step.  (Once the
 header is used in compilation, a generated dependency file will then
 express the implicit dependency.)
 
-File paths are compared as is, which means that an absolute path and a
-relative path, pointing to the same file, are considered different by Ninja.
+File paths in `build.ninja` are compared as is, which means that an
+absolute path and a relative path, pointing to the same file, are
+considered different by Ninja.
+See <<ref_headers, discussion of header dependencies>> for treatment
+of paths in a depfile.
 
 Variable expansion
 ~~~~~~~~~~~~~~~~~~

src/build.cc

@@ -846,7 +846,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result,
       // all backslashes (as some of the slashes will certainly be backslashes
       // anyway). This could be fixed if necessary with some additional
       // complexity in IncludesNormalize::Relativize.
-      deps_nodes->push_back(state_->GetNode(*i, ~0u));
+      deps_nodes->push_back(state_->GetNodeForDepfile(*i, ~0u));
     }
   } else if (deps_type == "gcc") {
     string depfile = result->edge->GetUnescapedDepfile();
@@ -881,7 +881,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result,
       if (!CanonicalizePath(const_cast<char*>(i->str_), &i->len_, &slash_bits,
                             err))
         return false;
-      deps_nodes->push_back(state_->GetNode(*i, slash_bits));
+      deps_nodes->push_back(state_->GetNodeForDepfile(*i, slash_bits));
     }
 
     if (!g_keep_depfile) {

src/dyndep_parser.cc

@@ -119,7 +119,7 @@ bool DyndepParser::ParseEdge(string* err) {
     uint64_t slash_bits;
     if (!CanonicalizePath(&path, &slash_bits, &path_err))
       return lexer_.Error(path_err, err);
-    Node* node = state_->LookupNode(path);
+    Node* node = state_->LookupNodeForDepfile(path);
     if (!node || !node->in_edge())
       return lexer_.Error("no build statement exists for '" + path + "'", err);
     Edge* edge = node->in_edge();
@@ -206,7 +206,7 @@ bool DyndepParser::ParseEdge(string* err) {
     uint64_t slash_bits;
     if (!CanonicalizePath(&path, &slash_bits, &path_err))
       return lexer_.Error(path_err, err);
-    Node* n = state_->GetNode(path, slash_bits);
+    Node* n = state_->GetNodeForDepfile(path, slash_bits);
     dyndeps->implicit_inputs_.push_back(n);
   }
 
@@ -217,7 +217,7 @@ bool DyndepParser::ParseEdge(string* err) {
     uint64_t slash_bits;
     if (!CanonicalizePath(&path, &slash_bits, &path_err))
       return lexer_.Error(path_err, err);
-    Node* n = state_->GetNode(path, slash_bits);
+    Node* n = state_->GetNodeForDepfile(path, slash_bits);
     dyndeps->implicit_outputs_.push_back(n);
   }
 

src/graph.cc

@@ -605,7 +605,7 @@ bool ImplicitDepLoader::ProcessDepfileDeps(
                           err))
       return false;
 
-    Node* node = state_->GetNode(*i, slash_bits);
+    Node* node = state_->GetNodeForDepfile(*i, slash_bits);
     *implicit_dep = node;
     node->AddOutEdge(edge);
     CreatePhonyInEdge(node);

src/graph_test.cc

@@ -247,6 +247,52 @@ TEST_F(GraphTest, DepfileWithCanonicalizablePath) {
   EXPECT_FALSE(GetNode("out.o")->dirty());
 }
 
+TEST_F(GraphTest, DepfileWithAbsolutePathToRelativeNode) {
+  state_.workdir_ = "/path/to/";
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule catdep\n"
+"  depfile = $out.d\n"
+"  command = cat $in > $out\n"
+"build ./out.o: catdep ./foo.cc\n"));
+  fs_.Create("foo.cc", "");
+  fs_.Create("out.o.d", "out.o: /path/to/foo.cc\n");
+  fs_.Create("out.o", "");
+
+  Edge* edge = GetNode("out.o")->in_edge();
+  string err;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err));
+  ASSERT_EQ("", err);
+
+  EXPECT_EQ(1, edge->implicit_deps_);
+  ASSERT_EQ(2, edge->inputs_.size());
+  EXPECT_EQ("foo.cc", edge->inputs_[1]->path());
+
+  EXPECT_FALSE(GetNode("out.o")->dirty());
+}
+
+TEST_F(GraphTest, DepfileWithRelativePathToAbsoluteNode) {
+  state_.workdir_ = "/path/to/";
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule catdep\n"
+"  depfile = $out.d\n"
+"  command = cat $in > $out\n"
+"build ./out.o: catdep /path/to/foo.cc\n"));
+  fs_.Create("/path/to/foo.cc", "");
+  fs_.Create("out.o.d", "out.o: foo.cc\n");
+  fs_.Create("out.o", "");
+
+  string err;
+  EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err));
+  ASSERT_EQ("", err);
+
+  Edge* edge = GetNode("out.o")->in_edge();
+  EXPECT_EQ(1, edge->implicit_deps_);
+  ASSERT_EQ(2, edge->inputs_.size());
+  EXPECT_EQ("/path/to/foo.cc", edge->inputs_[1]->path());
+
+  EXPECT_FALSE(GetNode("out.o")->dirty());
+}
+
 // Regression test for https://github.com/ninja-build/ninja/issues/404
 TEST_F(GraphTest, DepfileRemoved) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
@@ -511,6 +557,42 @@ TEST_F(GraphTest, DyndepLoadTrivial) {
   EXPECT_FALSE(edge->GetBindingBool("restat"));
 }
 
+TEST_F(GraphTest, DyndepLoadImplicit) {
+  state_.workdir_ = "/path/to/";
+  AssertParse(&state_,
+"rule r\n"
+"  command = unused\n"
+"build out1: r in || dd\n"
+"  dyndep = dd\n"
+"build out2: r in\n"
+"build out3: r in\n"
+"build /path/to/out4: r in\n"
+  );
+  fs_.Create("dd",
+"ninja_dyndep_version = 1\n"
+"build /path/to/out1: dyndep | out2 /path/to/out3 out4\n"
+  );
+
+  string err;
+  ASSERT_TRUE(GetNode("dd")->dyndep_pending());
+  EXPECT_TRUE(scan_.LoadDyndeps(GetNode("dd"), &err));
+  EXPECT_EQ("", err);
+  EXPECT_FALSE(GetNode("dd")->dyndep_pending());
+
+  Edge* edge = GetNode("out1")->in_edge();
+  ASSERT_EQ(1u, edge->outputs_.size());
+  EXPECT_EQ("out1", edge->outputs_[0]->path());
+  ASSERT_EQ(5u, edge->inputs_.size());
+  EXPECT_EQ("in", edge->inputs_[0]->path());
+  EXPECT_EQ("out2", edge->inputs_[1]->path());
+  EXPECT_EQ("out3", edge->inputs_[2]->path());
+  EXPECT_EQ("/path/to/out4", edge->inputs_[3]->path());
+  EXPECT_EQ("dd", edge->inputs_[4]->path());
+  EXPECT_EQ(3u, edge->implicit_deps_);
+  EXPECT_EQ(1u, edge->order_only_deps_);
+  EXPECT_FALSE(edge->GetBindingBool("restat"));
+}
+
 TEST_F(GraphTest, DyndepLoadMissingFile) {
   AssertParse(&state_,
 "rule r\n"

src/ninja.cc

@@ -147,6 +147,10 @@ struct NinjaMain : public BuildLogUser {
   /// @return false on error.
   bool EnsureBuildDirExists();
 
+  /// Save the process working directory.
+  /// @return false on error.
+  bool MemoizeWorkDir(const Options* options, Status *status);
+
   /// Rebuild the manifest, if necessary.
   /// Fills in \a err on error.
   /// @return true if the manifest was rebuilt.
@@ -849,18 +853,10 @@ int NinjaMain::ToolCompilationDatabase(const Options* options, int argc,
   argc -= optind;
 
   bool first = true;
-  vector<char> cwd;
-  char* success = NULL;
-
-  do {
-    cwd.resize(cwd.size() + 1024);
-    errno = 0;
-    success = getcwd(&cwd[0], cwd.size());
-  } while (!success && errno == ERANGE);
-  if (!success) {
-    Error("cannot determine working directory: %s", strerror(errno));
+  std::string cwd;
+
+  if (!GetCWD(&cwd))
     return 1;
-  }
 
   putchar('[');
   for (vector<Edge*>::iterator e = state_.edges_.begin();
@@ -1236,6 +1232,21 @@ bool NinjaMain::EnsureBuildDirExists() {
   return true;
 }
 
+bool NinjaMain::MemoizeWorkDir(const Options* options, Status* status) {
+  std::string workdir;
+  if (!GetCWD(&workdir))
+    return false;
+
+  std::string err;
+  if (!NormalizeWorkDir(&workdir, &err)) {
+    Error("invalid working directory: %s", err.c_str());
+    return false;
+  }
+
+  state_.workdir_ = workdir;
+  return true;
+}
+
 int NinjaMain::RunBuild(int argc, char** argv, Status* status) {
   string err;
   vector<Node*> targets;
@@ -1448,6 +1459,9 @@ NORETURN void real_main(int argc, char** argv) {
       exit(1);
     }
 
+    if (!ninja.MemoizeWorkDir(&options, status))
+      exit(1);
+
     if (options.tool && options.tool->when == Tool::RUN_AFTER_LOAD)
       exit((ninja.*options.tool->func)(&options, argc, argv));
 

src/state.cc

@@ -94,13 +94,24 @@ Edge* State::AddEdge(const Rule* rule) {
   return edge;
 }
 
+Node* State::AddNode(StringPiece path, uint64_t slash_bits) {
+  Node* node = new Node(path.AsString(), slash_bits);
+  paths_[node->path()] = node;
+  return node;
+}
+
 Node* State::GetNode(StringPiece path, uint64_t slash_bits) {
   Node* node = LookupNode(path);
   if (node)
     return node;
-  node = new Node(path.AsString(), slash_bits);
-  paths_[node->path()] = node;
-  return node;
+  return AddNode(path, slash_bits);
+}
+
+Node* State::GetNodeForDepfile(StringPiece path, uint64_t slash_bits) {
+  Node* node = LookupNodeForDepfile(path);
+  if (node)
+    return node;
+  return AddNode(path, slash_bits);
 }
 
 Node* State::LookupNode(StringPiece path) const {
@@ -111,6 +122,49 @@ Node* State::LookupNode(StringPiece path) const {
   return NULL;
 }
 
+Node* State::LookupNodeForDepfile(StringPiece path) const {
+  // First look for a node with the exact path given.
+  Node* node = LookupNode(path);
+  if (node)
+    return node;
+
+  if (workdir_.empty())
+    return NULL;
+
+  // Account for differences between absolute and relative paths
+  // to the same node.
+  if (IsAbsolutePath(path)) {
+    // If the absolute path is prefixed with the working directory then
+    // look for a node with the matching relative path.
+    // This will not help for nodes that use relative paths starting in `../`
+    // so it is the responsibility of the build manifest generator not to
+    // produce those.  Note that 'workdir_' already has a trailing slash.
+    if (path.len_ > workdir_.size() &&
+        memcmp(path.str_, workdir_.c_str(), workdir_.size()) == 0) {
+      path.str_ += workdir_.size();
+      path.len_ -= workdir_.size();
+      node = LookupNode(path);
+      if (node)
+        return node;
+    }
+  } else {
+    // Convert this relative path to an absolute path by prefixing it with
+    // the working directory.  Look for a node with the absolute path.
+    // Note that 'workdir_' already has a trailing slash.
+    // If CanonicalizePath fails on the combined path then just skip this.
+    std::string p = workdir_ + path.AsString();
+    std::string path_err;
+    uint64_t slash_bits;
+    if (CanonicalizePath(&p, &slash_bits, &path_err)) {
+      node = LookupNode(StringPiece(p));
+      if (node)
+        return node;
+    }
+  }
+
+  return NULL;
+}
+
 Node* State::SpellcheckNode(const string& path) {
   const bool kAllowReplacements = true;
   const int kMaxValidEditDistance = 3;

src/state.h

@@ -102,7 +102,9 @@ struct State {
   Edge* AddEdge(const Rule* rule);
 
   Node* GetNode(StringPiece path, uint64_t slash_bits);
+  Node* GetNodeForDepfile(StringPiece path, uint64_t slash_bits);
   Node* LookupNode(StringPiece path) const;
+  Node* LookupNodeForDepfile(StringPiece path) const;
   Node* SpellcheckNode(const std::string& path);
 
   void AddIn(Edge* edge, StringPiece path, uint64_t slash_bits);
@@ -125,6 +127,10 @@ struct State {
   typedef ExternalStringHashMap<Node*>::Type Paths;
   Paths paths_;
 
+  /// The working directory in which commands run, ending in a trailing slash.
+  /// This is computed from the process working directory, or set by tests.
+  std::string workdir_;
+
   /// All the pools used in the graph.
   std::map<std::string, Pool*> pools_;
 
@@ -133,6 +139,9 @@ struct State {
 
   BindingEnv bindings_;
   std::vector<Node*> defaults_;
+
+ private:
+  Node* AddNode(StringPiece path, uint64_t slash_bits);
 };
 
 #endif  // NINJA_STATE_H_