Move dependency declaration out of finally block.

This means that we will not declare additional dependencies if the action fails to execute, but that's correct -- if the action fails to execute, its inputs aren't updated, so there's nothing we'd discover.

--
MOS_MIGRATED_REVID=97255120

1 files changed, 18 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 7496f0d6e2..5b5c8ead04 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -319,7 +319,6 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
     PerActionFileCache perActionFileCache = new PerActionFileCache(state.inputArtifactData);
     ActionExecutionContext actionExecutionContext = null;
     boolean inputsDiscoveredDuringActionExecution = false;
-    Map<Artifact, FileArtifactValue> metadataFoundDuringActionExecution = null;
     try {
       if (action.discoversInputs()) {
         if (!state.hasDiscoveredInputs()) {
@@ -366,25 +365,25 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
           // Nothing we can do here.
         }
       }
-      if (inputsDiscoveredDuringActionExecution) {
-        metadataFoundDuringActionExecution =
-            declareAdditionalDependencies(env, action, state.inputArtifactData.keySet());
-        state.discoveredInputs = metadataFoundDuringActionExecution.keySet();
-      }
-    }
-    if (env.valuesMissing()) {
-      return null;
     }
-    if (inputsDiscoveredDuringActionExecution && !metadataFoundDuringActionExecution.isEmpty()) {
-      // We are in the interesting case of an action that discovered its inputs during execution,
-      // and found some new ones, but the new ones were already present in the graph. We must
-      // therefore cache the metadata for those new ones.
-      Map<Artifact, FileArtifactValue> inputArtifactData = new HashMap<>();
-      inputArtifactData.putAll(state.inputArtifactData);
-      inputArtifactData.putAll(metadataFoundDuringActionExecution);
-      state.inputArtifactData = inputArtifactData;
-      metadataHandler =
-          new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm);
+    if (inputsDiscoveredDuringActionExecution) {
+      Map<Artifact, FileArtifactValue> metadataFoundDuringActionExecution =
+          declareAdditionalDependencies(env, action, state.inputArtifactData.keySet());
+      state.discoveredInputs = metadataFoundDuringActionExecution.keySet();
+      if (env.valuesMissing()) {
+        return null;
+      }
+      if (!metadataFoundDuringActionExecution.isEmpty()) {
+        // We are in the interesting case of an action that discovered its inputs during execution,
+        // and found some new ones, but the new ones were already present in the graph. We must
+        // therefore cache the metadata for those new ones.
+        Map<Artifact, FileArtifactValue> inputArtifactData = new HashMap<>();
+        inputArtifactData.putAll(state.inputArtifactData);
+        inputArtifactData.putAll(metadataFoundDuringActionExecution);
+        state.inputArtifactData = inputArtifactData;
+        metadataHandler =
+            new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm);
+      }
     }
     skyframeActionExecutor.afterExecution(action, metadataHandler, state.token);
     return state.value;