A partial, manual rollback of commit 7af14dfdbd6addb779226c0a103b2a8dc72c16b1.

This became necessary because extra actions for C++ compile actions require .h files, but the compiler only returns the .pcm files in the .d file for headers that it reads from the .pcm file. This is not a problem for correctness because the .pcm files depend on the headers, but that doesn't help the extra actions that would then only get the .pcm files.

--
PiperOrigin-RevId: 150052839
MOS_MIGRATED_REVID=150052839

6 files changed, 95 insertions, 9 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 78a1d5e15a..101cc3932a 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
@@ -522,6 +522,25 @@ 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 for actions
+   * that return true for {link #discoversInputs()}.
+   *
+   * @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 ImmutableList.of();
+  }
+
   @SkylarkCallable(
       name = "inputs",
       doc = "A set of the input files of this action.",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
index 4b45965036..c2e2d3dfcf 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
@@ -88,6 +88,22 @@ public interface ActionAnalysisMetadata {
   ImmutableSet<Artifact> getOutputs();
 
   /**
+   * 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 for actions
+   * that return true for {link #discoversInputs()}.
+   *
+   * @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
+   */
+  Iterable<Artifact> getInputFilesForExtraAction(ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException, InterruptedException;
+
+  /**
    * Returns the set of output Artifacts that are required to be saved. This is
    * used to identify items that would otherwise be potentially identified as
    * orphaned (not consumed by any downstream {@link Action}s and potentially
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
index 6394c0cbf1..6a64709d7b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis.actions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -183,6 +184,12 @@ public final class SpawnActionTemplate implements ActionTemplate<SpawnAction> {
   }
 
   @Override
+  public Iterable<Artifact> getInputFilesForExtraAction(
+      ActionExecutionContext actionExecutionContext) {
+    return ImmutableList.of();
+  }
+
+  @Override
   public ImmutableSet<Artifact> getMandatoryOutputs() {
     return ImmutableSet.of();
   }
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 d5b6c8564c..268dbfff6f 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
@@ -1231,6 +1231,31 @@ public class CppCompileAction extends AbstractAction
     }
   }
 
+  /**
+   * When compiling with modules, the C++ compile action only has the {@code .pcm} files on its
+   * inputs, which is not enough for extra actions that parse header files. Thus, re-run include
+   * scanning and add headers to the inputs of the extra action, too.
+   */
+  @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();
+    }
+
+    return Sets.<Artifact>difference(
+        ImmutableSet.<Artifact>copyOf(scannedIncludes), ImmutableSet.<Artifact>copyOf(getInputs()));
+  }
+
   @Override
   public String getMnemonic() {
     if (CppFileTypes.OBJC_SOURCE.matches(sourceFile.getExecPath())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index d132c6a983..8b80179a44 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -16,7 +16,8 @@ package com.google.devtools.build.lib.rules.cpp;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -145,6 +146,13 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile
   }
 
   @Override
+  public Iterable<Artifact> getInputFilesForExtraAction(
+      ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException, InterruptedException {
+    return ImmutableList.of();
+  }
+
+  @Override
   public ImmutableSet<Artifact> getMandatoryOutputs() {
     return ImmutableSet.<Artifact>of();
   }
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 b2f7fdbbac..930cc932c7 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
@@ -80,7 +80,9 @@ public final class ExtraAction extends SpawnAction {
     super(
         shadowedAction.getOwner(),
         ImmutableList.<Artifact>of(),
-        createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier),
+        createInputs(
+            shadowedAction.getInputs(), ImmutableList.<Artifact>of(), extraActionInputs,
+            runfilesSupplier),
         outputs,
         AbstractAction.DEFAULT_RESOURCE_SET,
         argv,
@@ -121,20 +123,29 @@ public final class ExtraAction extends SpawnAction {
 
     // We need to update our inputs to take account of any additional
     // inputs the shadowed action may need to do its work.
-    updateInputs(createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier));
-    return Sets.difference(ImmutableSet.copyOf(shadowedAction.getInputs()),
-        ImmutableSet.copyOf(originalShadowedActionInputs));
+    Iterable<Artifact> oldInputs = getInputs();
+    updateInputs(createInputs(
+        shadowedAction.getInputs(),
+        shadowedAction.getInputFilesForExtraAction(actionExecutionContext),
+        extraActionInputs,
+        runfilesSupplier));
+    return Sets.<Artifact>difference(
+        ImmutableSet.<Artifact>copyOf(getInputs()), ImmutableSet.<Artifact>copyOf(oldInputs));
   }
 
   private static NestedSet<Artifact> createInputs(
       Iterable<Artifact> shadowedActionInputs,
+      Iterable<Artifact> inputFilesForExtraAction,
       ImmutableSet<Artifact> extraActionInputs,
       RunfilesSupplier extraActionRunfilesSupplier) {
     NestedSetBuilder<Artifact> result = new NestedSetBuilder<>(Order.STABLE_ORDER);
-    if (shadowedActionInputs instanceof NestedSet) {
-      result.addTransitive((NestedSet<Artifact>) shadowedActionInputs);
-    } else {
-      result.addAll(shadowedActionInputs);
+    for (Iterable<Artifact> inputSet : ImmutableList.of(
+        shadowedActionInputs, inputFilesForExtraAction)) {
+      if (inputSet instanceof NestedSet) {
+        result.addTransitive((NestedSet<Artifact>) inputSet);
+      } else {
+        result.addAll(inputSet);
+      }
     }
     result.addAll(extraActionRunfilesSupplier.getArtifacts());
     return result.addAll(extraActionInputs).build();