From 0f0ca6e25d4248c460338630b27e2bfcf7e69951 Mon Sep 17 00:00:00 2001
From: ajurkowski <ajurkowski@google.com>
Date: Mon, 8 Jun 2020 07:18:36 -0700
Subject: [PATCH] Expand error message for LTO compilation with list of missing
 files in imports.

LtoBackendAction checks whether bitcode files are available for all of the
imports. Expand the error message not only to indicate there was a difference
but also to list the missing files. Please note that there cannot be extra
files given the constructed NetsedSet is always a subset of the imports.

PiperOrigin-RevId: 315269362
---
 .../build/lib/rules/cpp/LtoBackendAction.java | 20 ++++++++++++++--
 .../lib/rules/cpp/LtoBackendActionTest.java   | 24 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
index 0ce4042a1d9a2b..bf50286d91dddd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
@@ -17,6 +17,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.ActionEnvironment;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -41,6 +42,8 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -157,9 +160,22 @@ public NestedSet<Artifact> discoverInputs(ActionExecutionContext actionExecution
     // Convert the import set of paths to the set of bitcode file artifacts.
     NestedSet<Artifact> bitcodeInputSet = computeBitcodeInputs(importSet);
     if (bitcodeInputSet.memoizedFlattenAndGetSize() != importSet.size()) {
+      Set<PathFragment> missingInputs =
+          Sets.difference(
+              importSet,
+              bitcodeInputSet.toList().stream()
+                  .map(Artifact::getExecPath)
+                  .collect(Collectors.toSet()));
       throw new ActionExecutionException(
-          "error computing inputs from imports file "
-              + actionExecutionContext.getInputPath(imports),
+          String.format(
+              "error computing inputs from imports file: %s, missing bitcode files (first 10): %s",
+              actionExecutionContext.getInputPath(imports),
+              // Limit the reported count to protect against a large error message.
+              missingInputs.stream()
+                  .map(Object::toString)
+                  .sorted()
+                  .limit(10)
+                  .collect(Collectors.joining(", "))),
           this,
           false);
     }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
index 63c618965517be..be3b751bbb09d6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.rules.cpp;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -21,6 +22,7 @@
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Executor;
@@ -38,6 +40,7 @@
 import com.google.devtools.build.lib.exec.BinTools;
 import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
 import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -245,4 +248,25 @@ public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) {
         },
         actionKeyContext);
   }
+
+  @Test
+  public void discoverInputs_missingInputErrorMessage() throws Exception {
+    FileSystemUtils.writeIsoLatin1(imports1Artifact.getPath(), "file1.o", "file2.o", "file3.o");
+
+    Action[] actions =
+        new LtoBackendAction.Builder()
+            .addImportsInfo(
+                new BitcodeFiles(
+                    new NestedSetBuilder<Artifact>(Order.STABLE_ORDER)
+                        .add(getSourceArtifact("file2.o"))
+                        .build()),
+                imports1Artifact)
+            .setExecutable(scratch.file("/bin/clang").asFragment())
+            .addOutput(destinationArtifact)
+            .build(ActionsTestUtil.NULL_ACTION_OWNER, targetConfig);
+    ActionExecutionException e =
+        assertThrows(ActionExecutionException.class, () -> actions[0].discoverInputs(context));
+
+    assertThat(e).hasMessageThat().endsWith("(first 10): file1.o, file3.o");
+  }
 }