Set maximum limit on the number of actifacts shown in the output caused by conflicting actions.

Currently Blaze outputs all mandatory inputs diffs between two actions if they result in conflictions. It is useful debugging information however could be quite large. This change sets the maximum number of records in the diff list to 5.

--
MOS_MIGRATED_REVID=117449664

1 files changed, 57 insertions, 24 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
index c72e02e51d..d5da897c43 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/MutableActionGraph.java
@@ -15,12 +15,12 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Sets.SetView;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.util.StringUtil;
 
 import java.util.Set;
 
@@ -67,11 +67,17 @@ public interface MutableActionGraph extends ActionGraph {
     private final Artifact artifact;
     private final Action previousAction;
     private final Action attemptedAction;
+    
+    private static final int MAX_DIFF_ARTIFACTS_TO_REPORT = 5;
 
     public ActionConflictException(Artifact artifact, Action previousAction,
         Action attemptedAction) {
-      super(String.format("for %s, previous action: %s, attempted action: %s",
-          artifact, previousAction, attemptedAction));
+      super(
+          String.format(
+              "for %s, previous action: %s, attempted action: %s",
+              artifact.prettyPrint(),
+              previousAction.prettyPrint(),
+              attemptedAction.prettyPrint()));
       this.artifact = artifact;
       this.previousAction = previousAction;
       this.attemptedAction = attemptedAction;
@@ -82,13 +88,16 @@ public interface MutableActionGraph extends ActionGraph {
     }
 
     public void reportTo(EventHandler eventListener) {
-      String msg = "file '" + artifact.prettyPrint()
-              + "' is generated by these conflicting actions:\n" +
-              suffix(attemptedAction, previousAction);
+      String msg =
+          "file '"
+              + artifact.prettyPrint()
+              + "' is generated by these conflicting actions:\n"
+              + suffix(attemptedAction, previousAction);
       eventListener.handle(Event.error(msg));
     }
 
-    private void addStringDetail(StringBuilder sb, String key, String valueA, String valueB) {
+    private static void addStringDetail(
+        StringBuilder sb, String key, String valueA, String valueB) {
       valueA = valueA != null ? valueA : "(null)";
       valueB = valueB != null ? valueB : "(null)";
 
@@ -99,8 +108,8 @@ public interface MutableActionGraph extends ActionGraph {
       sb.append("\n");
     }
 
-    private void addListDetail(StringBuilder sb, String key,
-        Iterable<Artifact> valueA, Iterable<Artifact> valueB) {
+    private static void addListDetail(
+        StringBuilder sb, String key, Iterable<Artifact> valueA, Iterable<Artifact> valueB) {
       Set<Artifact> setA = ImmutableSet.copyOf(valueA);
       Set<Artifact> setB = ImmutableSet.copyOf(valueB);
       SetView<Artifact> diffA = Sets.difference(setA, setB);
@@ -108,21 +117,31 @@ public interface MutableActionGraph extends ActionGraph {
 
       sb.append(key).append(": ");
       if (diffA.isEmpty() && diffB.isEmpty()) {
-        sb.append("are equal");
+        sb.append("are equal\n");
       } else {
-        if (!diffA.isEmpty() && !diffB.isEmpty()) {
-          sb.append("attempted action contains artifacts not in previous action and "
-              + "previous action contains artifacts not in attempted action: "
-              + diffA + ", " + diffB);
-        } else if (!diffA.isEmpty()) {
-          sb.append("attempted action contains artifacts not in previous action: ");
-          sb.append(StringUtil.joinEnglishList(diffA, "and"));
-        } else if (!diffB.isEmpty()) {
-          sb.append("previous action contains artifacts not in attempted action: ");
-          sb.append(StringUtil.joinEnglishList(diffB, "and"));
+        if (!diffA.isEmpty()) {
+          sb.append(
+              "Attempted action contains artifacts not in previous action (first "
+                  + MAX_DIFF_ARTIFACTS_TO_REPORT
+                  + "): \n");
+          prettyPrintArtifactDiffs(sb, diffA);
+        }
+
+        if (!diffB.isEmpty()) {
+          sb.append(
+              "Previous action contains artifacts not in attempted action (first "
+                  + MAX_DIFF_ARTIFACTS_TO_REPORT
+                  + "): \n");
+          prettyPrintArtifactDiffs(sb, diffB);
         }
       }
-      sb.append("\n");
+    }
+
+    /** Pretty print action diffs (at most {@code MAX_DIFF_ARTIFACTS_TO_REPORT} lines). */
+    private static void prettyPrintArtifactDiffs(StringBuilder sb, SetView<Artifact> diff) {
+      for (Artifact artifact : Iterables.limit(diff, MAX_DIFF_ARTIFACTS_TO_REPORT)) {
+        sb.append("\t" + artifact.prettyPrint() + "\n");
+      }
     }
 
     // See also Actions.canBeShared()
@@ -144,9 +163,23 @@ public interface MutableActionGraph extends ActionGraph {
           bNull ? null : bOwner.getConfigurationChecksum());
       addStringDetail(sb, "Mnemonic", a.getMnemonic(), b.getMnemonic());
       addStringDetail(sb, "Progress message", a.getProgressMessage(), b.getProgressMessage());
-
-      addListDetail(sb, "MandatoryInputs", a.getMandatoryInputs(), b.getMandatoryInputs());
-      addListDetail(sb, "Outputs", a.getOutputs(), b.getOutputs());
+      addStringDetail(
+          sb,
+          "PrimaryInput",
+          a.getPrimaryInput() == null ? null : a.getPrimaryInput().toString(),
+          b.getPrimaryInput() == null ? null : b.getPrimaryInput().toString());
+      addStringDetail(
+          sb, "PrimaryOutput", a.getPrimaryOutput().toString(), b.getPrimaryOutput().toString());
+
+      // Only add list details if the primary input of A matches the input of B. Otherwise
+      // the above information is enough and list diff detail is not needed.
+      if ((a.getPrimaryInput() == null && b.getPrimaryInput() == null)
+          || (a.getPrimaryInput() != null
+              && b.getPrimaryInput() != null
+              && a.getPrimaryInput().toString().equals(b.getPrimaryInput().toString()))) {
+        addListDetail(sb, "MandatoryInputs", a.getMandatoryInputs(), b.getMandatoryInputs());
+        addListDetail(sb, "Outputs", a.getOutputs(), b.getOutputs());
+      }
 
       return sb.toString();
     }