Improve order-only dependency behaviour

For incremental builds, changes in order-only dependencies do not cause
a build command to be rerun by ninja.  The idea being that after their
first run, their real dependencies have been loaded into `.ninja_deps`
and, as they are more accurate, can be used instead.

trimja is trying to emulate an incremental build to rebuild only those
commands affected by a certain set of files.  So if an affected file is
mentioned as an order-only dependencies, but it doesn't appear for that
output in `.ninja_deps` - it wasn't **actually** needed.

This should reduce the number of build commands that we include in the
trimmed file.

Additionally, there was a bug where we incorrectly included the
order-only dependencies in the `$in` variables - `out5` in the
dependencies test case should cover this.

src/graph.cpp

@@ -137,6 +137,11 @@ void Graph::addEdge(std::size_t in, std::size_t out) {
   m_outputToInput[out].insert(in);
 }
 
+void Graph::addOneWayEdge(std::size_t in, std::size_t out) {
+  assert(in != out);
+  m_inputToOutput[in].insert(out);
+}
+
 bool Graph::isDefault(std::size_t pathIndex) const {
   return pathIndex == m_defaultIndex;
 }

src/graph.h

@@ -112,12 +112,20 @@ class Graph {
   std::size_t addDefault();
 
   /**
-   * @brief Adds an edge between the specified input and output nodes.
+   * @brief Adds bidirectional edge between the specified input and output
+   * nodes.
    * @param in The index of the input node.
    * @param out The index of the output node.
    */
   void addEdge(std::size_t in, std::size_t out);
 
+  /**
+   * @brief Adds an one-way edge from the specified input to output node.
+   * @param in The index of the input node.
+   * @param out The index of the output node.
+   */
+  void addOneWayEdge(std::size_t in, std::size_t out);
+
   /**
    * @brief Checks if the specified path index is the default node.
    * @param pathIndex The index of the path to check.
@@ -146,7 +154,8 @@ class Graph {
   const std::set<std::size_t>& out(std::size_t pathIndex) const;
 
   /**
-   * @brief Gets the set of input nodes for the specified path index.
+   * @brief Gets the set of input nodes for the specified path index.  Note that
+   * this does not include order-only dependencies.
    * @param pathIndex The index of the path.
    * @return The set of input nodes.
    */

src/trimutil.cpp

@@ -224,8 +224,10 @@ struct BuildContext {
     for (const EvalString& path : r.implicitIn()) {
       ins.push_back(evaluatePath(path));
     }
+
+    std::vector<std::string> orderOnlyDeps;
     for (const EvalString& path : r.orderOnlyDeps()) {
-      ins.push_back(evaluatePath(path));
+      orderOnlyDeps.push_back(evaluatePath(path));
     }
 
     // Collect validations but ignore what they are. If we include a build
@@ -273,6 +275,16 @@ struct BuildContext {
       }
     }
 
+    // We only need to add an input to output edge and not a bidirectional
+    // one for order-only dependencies. This is because we only include a
+    // build edge if an input (implicit or not) is affected.
+    for (std::string& orderOnlyDep : orderOnlyDeps) {
+      const std::size_t inIndex = getPathIndex(orderOnlyDep);
+      for (const std::size_t outIndex : outIndices) {
+        graph.addOneWayEdge(inIndex, outIndex);
+      }
+    }
+
     {
       std::string command;
       scope.appendValue(command, "command");

tests/dependencies/build.ninja

@@ -4,6 +4,7 @@ build out1: copy in
 build out2: copy foo | in
 build out3: copy foo || in
 build out4: copy foo | bar || in
+build out5: copy foo || bar
 build extra1: copy out1
 build extra2: copy foo | out2
 build extra3: copy foo || out3

tests/dependencies/expected.ninja

@@ -2,9 +2,10 @@ rule copy
   command = ninja --version $in -> $out
 build out1: copy in
 build out2: copy foo | in
-build out3: copy foo || in
-build out4: copy foo | bar || in
+build out3: phony
+build out4: phony
+build out5: phony
 build extra1: copy out1
 build extra2: copy foo | out2
-build extra3: copy foo || out3
-build extra4: copy foo | bar || out4
+build extra3: phony
+build extra4: phony

tests/phony/build.ninja

@@ -10,4 +10,4 @@ build out5: copy affected3
 build out6: copy notAffected3
 build out7: copy notAffected4
 build bootstrap: phony out5 out6
-build topout: copy out7 || bootstrap
+build topout: copy out7 bootstrap

tests/phony/expected.ninja

@@ -10,4 +10,4 @@ build out5: copy affected3
 build out6: copy notAffected3
 build out7: copy notAffected4
 build bootstrap: phony out5 out6
-build topout: copy out7 || bootstrap
+build topout: copy out7 bootstrap