Fix query --output=proto --order_output=deps not returning targets in topological order.

zhengwei143 · copybara-github · commit 2ff87be50810 · 2023-01-12T06:25:35.000-08:00
Fixes #17087. RELNOTES[INC]: query --output=proto --order_output=deps now returns targets in topological order (previously there was no ordering). PiperOrigin-RevId: 501548057 Change-Id: Ied86694fc6c7f9fc2243af1ce43bc778509cbe44
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/AbstractUnorderedFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/AbstractUnorderedFormatter.java
@@ -57,10 +57,15 @@ public void output(
   }
 
   protected Iterable<Target> getOrderedTargets(Digraph<Target> result, QueryOptions options) {
-    Iterable<Node<Target>> orderedResult =
-        options.orderOutput == OrderOutput.DEPS
-            ? result.getTopologicalOrder()
-            : result.getTopologicalOrder(new FormatUtils.TargetOrdering());
-    return Iterables.transform(orderedResult, Node::getLabel);
+    if (options.orderOutput == OrderOutput.FULL) {
+      // Get targets in total order, the difference here from topological ordering is the sorting of
+      // nodes before post-order visitation (which ensures determinism at a time cost).
+      return Iterables.transform(
+          result.getTopologicalOrder(new FormatUtils.TargetOrdering()), Node::getLabel);
+    } else if (options.orderOutput == OrderOutput.DEPS) {
+      // Get targets in topological order.
+      return Iterables.transform(result.getTopologicalOrder(), Node::getLabel);
+    }
+    return result.getLabels();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
@@ -34,8 +34,6 @@
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.graph.Digraph;
-import com.google.devtools.build.lib.graph.Node;
 import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeFormatter;
@@ -61,7 +59,6 @@
 import com.google.devtools.build.lib.query2.proto.proto2api.Build.QueryResult;
 import com.google.devtools.build.lib.query2.proto.proto2api.Build.SourceFile;
 import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver;
-import com.google.devtools.build.lib.query2.query.output.QueryOptions.OrderOutput;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -169,16 +166,6 @@ public ThreadSafeOutputFormatterCallback<Target> createStreamCallback(
         createPostFactoStreamCallback(out, options, env.getMainRepoMapping()));
   }
 
-  private static Iterable<Target> getSortedLabels(Digraph<Target> result) {
-    return Iterables.transform(
-        result.getTopologicalOrder(new FormatUtils.TargetOrdering()), Node::getLabel);
-  }
-
-  @Override
-  protected Iterable<Target> getOrderedTargets(Digraph<Target> result, QueryOptions options) {
-    return options.orderOutput == OrderOutput.FULL ? getSortedLabels(result) : result.getLabels();
-  }
-
   /** Converts a logical {@link Target} object into a {@link Build.Target} protobuffer. */
   public Build.Target toTargetProtoBuffer(Target target) throws InterruptedException {
     return toTargetProtoBuffer(target, /*extraDataForAttrHash=*/ "");
