Rollback of commit 41c2a26eef89167e807cbc9f33487dc66bb757d3.

This is not a clean rollback:
I had to change CppCompileAction on line 1278 and pass cppSemantics.getIncludeProcessing().
Please flag this if it doesn't make sense :)

--
PiperOrigin-RevId: 147868235
MOS_MIGRATED_REVID=147868235

4 files changed, 58 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index a8b460e823..7919529da0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -174,8 +174,7 @@ public abstract class AbstractAction implements Action, SkylarkValue {
   }
 
   @Override
-  public synchronized Iterable<Artifact> discoverInputs(
-      ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     throw new IllegalStateException("discoverInputs cannot be called for " + this.prettyPrint()
         + " since it does not discover inputs");
@@ -496,6 +495,27 @@ public abstract class AbstractAction implements Action, SkylarkValue {
     return ImmutableSet.of();
   }
 
+  /**
+   * Returns input files that need to be present to allow extra_action rules to shadow this action
+   * correctly when run remotely. This is at least the normal inputs of the action, but may include
+   * other files as well. For example C(++) compilation may perform include file header scanning.
+   * This needs to be mirrored by the extra_action rule. Called by
+   * {@link com.google.devtools.build.lib.rules.extra.ExtraAction} at execution time.
+   *
+   * <p>As this method is called from the ExtraAction, make sure it is ok to call
+   * this method from a different thread than the one this action is executed on.
+   *
+   * @param actionExecutionContext Services in the scope of the action, like the Out/Err streams.
+   * @throws ActionExecutionException only when code called from this method
+   *     throws that exception.
+   * @throws InterruptedException if interrupted
+   */
+  public Iterable<Artifact> getInputFilesForExtraAction(
+      ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException, InterruptedException {
+    return getInputs();
+  }
+
   @SkylarkCallable(
       name = "inputs",
       doc = "A set of the input files of this action.",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 3f8437087f..9936319435 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -436,8 +436,7 @@ public class CppCompileAction extends AbstractAction
 
   @Nullable
   @Override
-  public synchronized Iterable<Artifact> discoverInputs(
-      ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     Executor executor = actionExecutionContext.getExecutor();
     Iterable<Artifact> initialResult;
@@ -1264,6 +1263,32 @@ public class CppCompileAction extends AbstractAction
     }
   }
 
+  /**
+   * Provides list of include files needed for performing extra actions on this action when run
+   * remotely. The list of include files is created by performing a header scan on the known input
+   * files.
+   */
+  @Override
+  public Iterable<Artifact> getInputFilesForExtraAction(
+      ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException, InterruptedException {
+    Iterable<Artifact> scannedIncludes;
+    try {
+      scannedIncludes = actionExecutionContext.getExecutor().getContext(actionContext)
+          .findAdditionalInputs(this, actionExecutionContext,  cppSemantics.getIncludeProcessing());
+    } catch (ExecException e) {
+      throw e.toActionExecutionException(this);
+    }
+
+    if (scannedIncludes == null) {
+      return ImmutableList.of();
+    }
+
+    // Use a set to eliminate duplicates.
+    ImmutableSet.Builder<Artifact> result = ImmutableSet.builder();
+    return result.addAll(getInputs()).addAll(scannedIncludes).build();
+  }
+
   @Override
   public String getMnemonic() { return "CppCompile"; }
 
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 e6d35fe128..0b71041ceb 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
@@ -117,8 +117,7 @@ public final class LTOBackendAction extends SpawnAction {
 
   @Nullable
   @Override
-  public synchronized Iterable<Artifact> discoverInputs(
-      ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     // Build set of files this LTO backend artifact will import from.
     HashSet<PathFragment> importSet = new HashSet<>();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
index 9f07c98285..866e45e20a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
@@ -114,15 +114,18 @@ public final class ExtraAction extends SpawnAction {
 
   @Nullable
   @Override
-  public synchronized Iterable<Artifact> discoverInputs(
-      ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     Preconditions.checkState(discoversInputs(), this);
     // We need to update our inputs to take account of any additional
     // inputs the shadowed action may need to do its work.
-    Iterable<Artifact> additionalInputs = shadowedAction.discoverInputs(actionExecutionContext);
-    updateInputs(createInputs(additionalInputs, extraActionInputs, runfilesSupplier));
-    return additionalInputs;
+    if (shadowedAction.discoversInputs() && shadowedAction instanceof AbstractAction) {
+      Iterable<Artifact> additionalInputs =
+          ((AbstractAction) shadowedAction).getInputFilesForExtraAction(actionExecutionContext);
+      updateInputs(createInputs(additionalInputs, extraActionInputs, runfilesSupplier));
+      return ImmutableSet.copyOf(additionalInputs);
+    }
+    return null;
   }
 
   @Override