Automated rollback of commit 24f19ec2679dd93b1ac5b06e46f3b35807d6e217.

*** Reason for rollback ***

See: http://b/78455900

*** Original change description ***

Properly report completion of shared actions with input discovery

Currently we report "Analyzing" when include scanning runs. But since we can
have shared C++ compile actions, only one of the group will be executed and only
one will be reported completed. Remaining shared actions currently stay with
"analyzing" forever.

This cl makes sure that these actions are properly handled when finished.

RELNOTES: None
PiperOrigin-RevId: 193955856

2 files changed, 10 insertions, 36 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 ab9e1bd35a..936ed10f1b 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
@@ -191,9 +191,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
 
     ActionExecutionValue result;
     try {
-      result =
-          checkCacheAndExecuteIfNeeded(
-              action, state, env, clientEnv, actionLookupData, sharedActionAlreadyRan);
+      result = checkCacheAndExecuteIfNeeded(action, state, env, clientEnv, actionLookupData);
     } catch (ActionExecutionException e) {
       // Remove action from state map in case it's there (won't be unless it discovers inputs).
       stateMap.remove(action);
@@ -341,20 +339,13 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
       ContinuationState state,
       Environment env,
       Map<String, String> clientEnv,
-      ActionLookupData actionLookupData,
-      boolean sharedActionAlreadyRan)
+      ActionLookupData actionLookupData)
       throws ActionExecutionException, InterruptedException {
     // If this is a shared action and the other action is the one that executed, we must use that
     // other action's value, provided here, since it is populated with metadata for the outputs.
-    if (sharedActionAlreadyRan) {
-      return skyframeActionExecutor.executeAction(
-          env.getListener(),
-          action,
-          /* metadataHandler= */ null,
-          /* actionStartTime= */ -1,
-          /* actionExecutionContext= */ null,
-          actionLookupData,
-          /* inputDiscoveryRan= */ false);
+    if (!state.hasArtifactData()) {
+      return skyframeActionExecutor
+          .executeAction(env.getListener(), action, null, -1, null, actionLookupData);
     }
     // This may be recreated if we discover inputs.
     ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData,
@@ -438,13 +429,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
       if (!state.hasExecutedAction()) {
         state.value =
             skyframeActionExecutor.executeAction(
-                env.getListener(),
-                action,
-                metadataHandler,
-                actionStartTime,
-                actionExecutionContext,
-                actionLookupData,
-                /* inputDiscoveryRan= */ true);
+                env.getListener(), action, metadataHandler, actionStartTime, actionExecutionContext,
+                actionLookupData);
       }
     } catch (IOException e) {
       throw new ActionExecutionException(
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 18352ac52c..49017083e6 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
@@ -389,8 +389,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
       ActionMetadataHandler metadataHandler,
       long actionStartTime,
       ActionExecutionContext actionExecutionContext,
-      ActionLookupData actionLookupData,
-      boolean inputDiscoveryRan)
+      ActionLookupData actionLookupData)
       throws ActionExecutionException, InterruptedException {
     Exception exception = badActionMap.get(action);
     if (exception != null) {
@@ -410,10 +409,8 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
     // Check to see if another action is already executing/has executed this value.
     Pair<ActionLookupData, FutureTask<ActionExecutionValue>> oldAction =
         buildActionMap.putIfAbsent(primaryOutput, Pair.of(actionLookupData, actionTask));
-    // true if this is a non-shared action or it's shared and to be executed.
-    boolean isPrimaryActionForTheValue = oldAction == null;
 
-    if (isPrimaryActionForTheValue) {
+    if (oldAction == null) {
       actionTask.run();
     } else {
       // Wait for other action to finish, so any actions that depend on its outputs can execute.
@@ -426,15 +423,6 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
           ActionExecutionException.class, InterruptedException.class);
       throw new IllegalStateException(e);
     } finally {
-      if (!isPrimaryActionForTheValue && action.discoversInputs() && inputDiscoveryRan) {
-        /**
-         * If this is a shared action that does input discovery, but was not executed, we need to
-         * remove it from the active actions pool (it was added there by {@link
-         * ActionRunner#call()}).
-         */
-        // TODO(b/72764586): Cleanup once we can properly skip input discovery for shared actions
-        statusReporterRef.get().remove(action);
-      }
       String message = action.getProgressMessage();
       if (message != null) {
         // Tell the receiver that the action has completed *before* telling the reporter.
@@ -844,7 +832,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto
           context.getFileOutErr(),
           outputDumped);
     } finally {
-      statusReporterRef.get().remove(action);
+      statusReporter.remove(action);
       eventHandler.post(new ActionCompletionEvent(actionStartTime, action, actionLookupData));
     }
   }