Merge pull request #2485 from digit-google/fix-inputs-tool

Fix `inputs` tool logic and add new formatting options.

misc/output_test.py

@@ -228,6 +228,71 @@ def test_tool_inputs(self) -> None:
 out2
 ''')
 
+        self.assertEqual(run(plan, flags='-t inputs --dependency-order out3'),
+'''in2
+in1
+out1
+out2
+implicit
+order_only
+''')
+
+        # Verify that results are shell-escaped by default, unless --no-shell-escape
+        # is used. Also verify that phony outputs are never part of the results.
+        quote = '"' if platform.system() == "Windows" else "'"
+
+        plan = '''
+rule cat
+  command = cat $in $out
+build out1 : cat in1
+build out$ 2 : cat out1
+build out$ 3 : phony out$ 2
+build all: phony out$ 3
+'''
+
+        # Quoting changes the order of results when sorting alphabetically.
+        self.assertEqual(run(plan, flags='-t inputs all'),
+f'''{quote}out 2{quote}
+in1
+out1
+''')
+
+        self.assertEqual(run(plan, flags='-t inputs --no-shell-escape all'),
+'''in1
+out 2
+out1
+''')
+
+        # But not when doing dependency order.
+        self.assertEqual(
+            run(
+              plan,
+              flags='-t inputs --dependency-order all'
+            ),
+            f'''in1
+out1
+{quote}out 2{quote}
+''')
+
+        self.assertEqual(
+          run(
+            plan,
+            flags='-t inputs --dependency-order --no-shell-escape all'
+          ),
+          f'''in1
+out1
+out 2
+''')
+
+        self.assertEqual(
+          run(
+            plan,
+            flags='-t inputs --dependency-order --no-shell-escape --print0 all'
+          ),
+          f'''in1\0out1\0out 2\0'''
+        )
+
+
     def test_explain_output(self):
         b = BuildDir('''\
             build .FORCE: phony

src/graph.cc

@@ -496,28 +496,6 @@ std::string EdgeEnv::MakePathList(const Node* const* const span,
   return result;
 }
 
-void Edge::CollectInputs(bool shell_escape,
-                         std::vector<std::string>* out) const {
-  for (std::vector<Node*>::const_iterator it = inputs_.begin();
-       it != inputs_.end(); ++it) {
-    std::string path = (*it)->PathDecanonicalized();
-    if (shell_escape) {
-      std::string unescaped;
-      unescaped.swap(path);
-#ifdef _WIN32
-      GetWin32EscapedString(unescaped, &path);
-#else
-      GetShellEscapedString(unescaped, &path);
-#endif
-    }
-#if __cplusplus >= 201103L
-    out->push_back(std::move(path));
-#else
-    out->push_back(path);
-#endif
-  }
-}
-
 std::string Edge::EvaluateCommand(const bool incl_rsp_file) const {
   string command = GetBinding("command");
   if (incl_rsp_file) {
@@ -779,3 +757,47 @@ vector<Node*>::iterator ImplicitDepLoader::PreallocateSpace(Edge* edge,
   edge->implicit_deps_ += count;
   return edge->inputs_.end() - edge->order_only_deps_ - count;
 }
+
+void InputsCollector::VisitNode(const Node* node) {
+  const Edge* edge = node->in_edge();
+
+  if (!edge)  // A source file.
+    return;
+
+  // Add inputs of the producing edge to the result,
+  // except if they are themselves produced by a phony
+  // edge.
+  for (const Node* input : edge->inputs_) {
+    if (!visited_nodes_.insert(input).second)
+      continue;
+
+    VisitNode(input);
+
+    const Edge* input_edge = input->in_edge();
+    if (!(input_edge && input_edge->is_phony())) {
+      inputs_.push_back(input);
+    }
+  }
+}
+
+std::vector<std::string> InputsCollector::GetInputsAsStrings(
+    bool shell_escape) const {
+  std::vector<std::string> result;
+  result.reserve(inputs_.size());
+
+  for (const Node* input : inputs_) {
+    std::string unescaped = input->PathDecanonicalized();
+    if (shell_escape) {
+      std::string path;
+#ifdef _WIN32
+      GetWin32EscapedString(unescaped, &path);
+#else
+      GetShellEscapedString(unescaped, &path);
+#endif
+      result.push_back(std::move(path));
+    } else {
+      result.push_back(std::move(unescaped));
+    }
+  }
+  return result;
+}

src/graph.h

@@ -201,9 +201,6 @@ struct Edge {
 
   void Dump(const char* prefix="") const;
 
-  // Append all edge explicit inputs to |*out|. Possibly with shell escaping.
-  void CollectInputs(bool shell_escape, std::vector<std::string>* out) const;
-
   // critical_path_weight is the priority during build scheduling. The
   // "critical path" between this edge's inputs and any target node is
   // the path which maximises the sum oof weights along that path.
@@ -425,4 +422,41 @@ class EdgePriorityQueue:
   }
 };
 
+/// A class used to collect the transitive set of inputs from a given set
+/// of starting nodes. Used to implement the `inputs` tool.
+///
+/// When collecting inputs, the outputs of phony edges are always ignored
+/// from the result, but are followed by the dependency walk.
+///
+/// Usage is:
+/// - Create instance.
+/// - Call VisitNode() for each root node to collect inputs from.
+/// - Call inputs() to retrieve the list of input node pointers.
+/// - Call GetInputsAsStrings() to retrieve the list of inputs as a string
+/// vector.
+///
+struct InputsCollector {
+  /// Visit a single @arg node during this collection.
+  void VisitNode(const Node* node);
+
+  /// Retrieve list of visited input nodes. A dependency always appears
+  /// before its dependents in the result, but final order depends on the
+  /// order of the VisitNode() calls performed before this.
+  const std::vector<const Node*>& inputs() const { return inputs_; }
+
+  /// Same as inputs(), but returns the list of visited nodes as a list of
+  /// strings, with optional shell escaping.
+  std::vector<std::string> GetInputsAsStrings(bool shell_escape = false) const;
+
+  /// Reset collector state.
+  void Reset() {
+    inputs_.clear();
+    visited_nodes_.clear();
+  }
+
+ private:
+  std::vector<const Node*> inputs_;
+  std::set<const Node*> visited_nodes_;
+};
+
 #endif  // NINJA_GRAPH_H_

src/graph_test.cc

@@ -215,28 +215,90 @@ TEST_F(GraphTest, RootNodes) {
   }
 }
 
-TEST_F(GraphTest, CollectInputs) {
+TEST_F(GraphTest, InputsCollector) {
+  // Build plan for the following graph:
+  //
+  //      in1
+  //       |___________
+  //       |           |
+  //      ===         ===
+  //       |           |
+  //      out1        mid1
+  //       |       ____|_____
+  //       |      |          |
+  //       |     ===      =======
+  //       |      |       |     |
+  //       |     out2    out3  out4
+  //       |      |       |
+  //      =======phony======
+  //              |
+  //             all
+  //
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+                                      "build out1: cat in1\n"
+                                      "build mid1: cat in1\n"
+                                      "build out2: cat mid1\n"
+                                      "build out3 out4: cat mid1\n"
+                                      "build all: phony out1 out2 out3\n"));
+
+  InputsCollector collector;
+
+  // Start visit from out1, this should add in1 to the inputs.
+  collector.Reset();
+  collector.VisitNode(GetNode("out1"));
+  auto inputs = collector.GetInputsAsStrings();
+  ASSERT_EQ(1u, inputs.size());
+  EXPECT_EQ("in1", inputs[0]);
+
+  // Add a visit from out2, this should add mid1.
+  collector.VisitNode(GetNode("out2"));
+  inputs = collector.GetInputsAsStrings();
+  ASSERT_EQ(2u, inputs.size());
+  EXPECT_EQ("in1", inputs[0]);
+  EXPECT_EQ("mid1", inputs[1]);
+
+  // Another visit from all, this should add out1, out2 and out3,
+  // but not out4.
+  collector.VisitNode(GetNode("all"));
+  inputs = collector.GetInputsAsStrings();
+  ASSERT_EQ(5u, inputs.size());
+  EXPECT_EQ("in1", inputs[0]);
+  EXPECT_EQ("mid1", inputs[1]);
+  EXPECT_EQ("out1", inputs[2]);
+  EXPECT_EQ("out2", inputs[3]);
+  EXPECT_EQ("out3", inputs[4]);
+
+  collector.Reset();
+
+  // Starting directly from all, will add out1 before mid1 compared
+  // to the previous example above.
+  collector.VisitNode(GetNode("all"));
+  inputs = collector.GetInputsAsStrings();
+  ASSERT_EQ(5u, inputs.size());
+  EXPECT_EQ("in1", inputs[0]);
+  EXPECT_EQ("out1", inputs[1]);
+  EXPECT_EQ("mid1", inputs[2]);
+  EXPECT_EQ("out2", inputs[3]);
+  EXPECT_EQ("out3", inputs[4]);
+}
+
+TEST_F(GraphTest, InputsCollectorWithEscapes) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(
       &state_,
       "build out$ 1: cat in1 in2 in$ with$ space | implicit || order_only\n"));
 
-  std::vector<std::string> inputs;
-  Edge* edge = GetNode("out 1")->in_edge();
-
-  // Test without shell escaping.
-  inputs.clear();
-  edge->CollectInputs(false, &inputs);
-  EXPECT_EQ(5u, inputs.size());
+  InputsCollector collector;
+  collector.VisitNode(GetNode("out 1"));
+  auto inputs = collector.GetInputsAsStrings();
+  ASSERT_EQ(5u, inputs.size());
   EXPECT_EQ("in1", inputs[0]);
   EXPECT_EQ("in2", inputs[1]);
   EXPECT_EQ("in with space", inputs[2]);
   EXPECT_EQ("implicit", inputs[3]);
   EXPECT_EQ("order_only", inputs[4]);
 
-  // Test with shell escaping.
-  inputs.clear();
-  edge->CollectInputs(true, &inputs);
-  EXPECT_EQ(5u, inputs.size());
+  inputs = collector.GetInputsAsStrings(true);
+  ASSERT_EQ(5u, inputs.size());
   EXPECT_EQ("in1", inputs[0]);
   EXPECT_EQ("in2", inputs[1]);
 #ifdef _WIN32