Return the set of files found when discovering inputs. Skyframe will probably want to store these and may declare dependencies in advance so all metadata is present in the graph.

Long-term, we may be able to use the returned set to make input-discovering actions immutable, which would be nice.

--
MOS_MIGRATED_REVID=88348068

7 files changed, 27 insertions, 17 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 54997a4046..82d9248bfc 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
@@ -92,7 +92,7 @@ public abstract class AbstractAction implements Action {
   }
 
   @Override
-  public void discoverInputs(ActionExecutionContext actionExecutionContext)
+  public Collection<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     throw new IllegalStateException("discoverInputs cannot be called for " + this.prettyPrint()
         + " since it does not discover inputs");
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java
index efaa0dc3c8..09c50fb1fc 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Action.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java
@@ -99,9 +99,11 @@ public interface Action extends ActionMetadata, Describable {
 
   /**
    * Method used to find inputs before execution for an action that
-   * {@link ActionMetadata#discoversInputs}.
+   * {@link ActionMetadata#discoversInputs}. Returns null if action's inputs will be discovered
+   * during execution proper.
    */
-  void discoverInputs(ActionExecutionContext actionExecutionContext)
+  @Nullable
+  Collection<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException;
 
   /**
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 d963dda924..23d841deba 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
@@ -167,7 +167,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
    * Set when the action prepares for execution. Used to preserve state between preparation and
    * execution.
    */
-  private Collection<? extends ActionInput> additionalInputs = null;
+  private Collection<Artifact> additionalInputs = null;
 
   /**
    * Creates a new action to compile C/C++ source files.
@@ -303,17 +303,21 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     return true;
   }
 
+  @Nullable
   @Override
-  public void discoverInputs(ActionExecutionContext actionExecutionContext)
+  public Collection<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     Executor executor = actionExecutionContext.getExecutor();
+    Collection<Artifact> returnValue = null;
     try {
-      this.additionalInputs = executor.getContext(CppCompileActionContext.class)
+      returnValue = executor.getContext(CppCompileActionContext.class)
           .findAdditionalInputs(this, actionExecutionContext);
     } catch (ExecException e) {
       throw e.toActionExecutionException("Include scanning of rule '" + getOwner().getLabel() + "'",
           executor.getVerboseFailures(), this);
     }
+    this.additionalInputs = returnValue == null ? ImmutableList.<Artifact>of() : returnValue;
+    return returnValue;
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
index 4bbfa44d61..2a0f6a1b51 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.cpp;
 import com.google.devtools.build.lib.actions.ActionContextMarker;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Executor.ActionContext;
@@ -43,7 +42,7 @@ public interface CppCompileActionContext extends ActionContext {
   }
 
   /** Does include scanning to find the list of files needed to execute the action. */
-  public Collection<? extends ActionInput> findAdditionalInputs(CppCompileAction action,
+  public Collection<Artifact> findAdditionalInputs(CppCompileAction action,
       ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException, ActionExecutionException;
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LocalGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LocalGccStrategy.java
index 80ee23d70b..6d3702f095 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LocalGccStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LocalGccStrategy.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -60,9 +59,9 @@ public class LocalGccStrategy implements CppCompileActionContext {
   }
 
   @Override
-  public Collection<? extends ActionInput> findAdditionalInputs(CppCompileAction action,
+  public Collection<Artifact> findAdditionalInputs(CppCompileAction action,
       ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException {
-    return ImmutableList.of();
+    return null;
   }
 
   @Override
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 9290fc79a2..a1f82a9785 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
@@ -52,6 +52,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 /**
  * Action used by extra_action rules to create an action that shadows an existing action. Runs a
  * command-line using {@link SpawnActionContext} for executions.
@@ -115,8 +117,9 @@ public final class ExtraAction extends SpawnAction {
     return shadowedAction.discoversInputs();
   }
 
+  @Nullable
   @Override
-  public void discoverInputs(ActionExecutionContext actionExecutionContext)
+  public Collection<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     Preconditions.checkState(discoversInputs(), this);
     if (getContext(actionExecutionContext.getExecutor()).isRemotable(getMnemonic(),
@@ -124,10 +127,13 @@ public final class ExtraAction extends SpawnAction {
       // If we're running remotely, we need to update our inputs to take account of any additional
       // inputs the shadowed action may need to do its work.
       if (shadowedAction.discoversInputs() && shadowedAction instanceof AbstractAction) {
-        updateInputs(
-            ((AbstractAction) shadowedAction).getInputFilesForExtraAction(actionExecutionContext));
+        Iterable<Artifact> additionalInputs =
+            ((AbstractAction) shadowedAction).getInputFilesForExtraAction(actionExecutionContext);
+        updateInputs(additionalInputs);
+        return ImmutableSet.copyOf(additionalInputs);
       }
     }
+    return null;
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index c89631526f..ebae390f50 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -586,12 +586,12 @@ public final class SkyframeActionExecutor {
    * <p>This method is just a wrapper around {@link Action#discoverInputs} that properly processes
    * any ActionExecutionException thrown before rethrowing it to the caller.
    */
-  void discoverInputs(Action action, ActionExecutionContext actionExecutionContext)
+  Collection<Artifact> discoverInputs(Action action, ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     try {
-      action.discoverInputs(actionExecutionContext);
+      return action.discoverInputs(actionExecutionContext);
     } catch (ActionExecutionException e) {
-      processAndThrow(e, action, actionExecutionContext.getFileOutErr());
+      throw processAndThrow(e, action, actionExecutionContext.getFileOutErr());
     }
   }