Delay updating inputs of an action when processing the action cache until it is known that the action is a cache hit.

This adds momentary memory overhead when checking the action cache, but should prevent a host of potential errors.

Note that this cl assumes that an action that discovers its inputs does *not* take the names of its inputs into account when calculating its key, which is already stated as part of the javadoc of Action#getKey.

--
MOS_MIGRATED_REVID=88971626

1 files changed, 12 insertions, 3 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 09457474bd..324d0b4608 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
@@ -36,6 +36,8 @@ import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.IOException;
 import java.util.Collection;
 
+import javax.annotation.Nullable;
+
 /**
  * Abstract implementation of Action which implements basic functionality: the
  * inputs, outputs, and toString method.  Both input and output sets are
@@ -98,13 +100,20 @@ public abstract class AbstractAction implements Action {
         + " since it does not discover inputs");
   }
 
+  @Nullable
   @Override
-  public boolean updateInputsFromCache(ArtifactResolver artifactResolver,
+  public Iterable<Artifact> resolveInputsFromCache(ArtifactResolver artifactResolver,
       PackageRootResolver resolver, Collection<PathFragment> inputPaths) {
     throw new IllegalStateException(
         "Method must be overridden for actions that may have unknown inputs.");
   }
 
+  @Override
+  public void updateInputs(Iterable<Artifact> inputs) {
+    throw new IllegalStateException(
+        "Method must be overridden for actions that may have unknown inputs.");
+  }
+
   /**
    * Should only be overridden by actions that need to optionally insert inputs. Actions that
    * discover their inputs should use {@link #setInputs} to set the new iterable of inputs when they
@@ -119,8 +128,8 @@ public abstract class AbstractAction implements Action {
    * Set the inputs of the action. May only be used by an action that {@link #discoversInputs()}.
    * The iterable passed in is automatically made immutable.
    */
-  public void setInputs(Iterable<Artifact> inputs) {
-    Preconditions.checkState(discoversInputs());
+  protected void setInputs(Iterable<Artifact> inputs) {
+    Preconditions.checkState(discoversInputs(), this);
     this.inputs = CollectionUtils.makeImmutable(inputs);
     cachedInputCount = -1;
   }