Move the chmod call from SkyframeActionExecutor to ActionMetadataHandler

This relies on the explicit state transition in MetadataHandler to decide
whether the chmod call is necessary (we must never call chmod if the action
was not executed).

This is a prerequisite for #1525. If we want to use ctime for detecting file
content changes (which is more reliable than just mtime), then we must call
chmod before stat, since chmod affects ctime.

Before this change, we were caching the stat in ActionMetadataHandler, but
calling chmod after action execution in SkyframeActionExecutor, which is the
wrong order of calls. However, we must be able to stat in ActionMetadataHandler
for cases where a single action runs multiple Spawns where one spawn's output
is a subsequent spawn's input.

Remove MetadataHandler.isInjected, which is no longer used anywhere.
PiperOrigin-RevId: 152387133

1 files changed, 3 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
index aa26ed5a12..0874097ef7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
@@ -24,6 +24,9 @@ import java.io.IOException;
  * Retrieves {@link Metadata} of {@link Artifact}s, and inserts virtual metadata as well. Some
  * methods on this interface may only be called after a call to {@link #discardOutputMetadata}.
  * Calling them before such a call results in an {@link IllegalStateException}.
+ *
+ * <p>Note that implementations of this interface call chmod on output files if
+ * {@link #discardOutputMetadata} has been called.
  */
 public interface MetadataHandler {
   /**
@@ -87,16 +90,6 @@ public interface MetadataHandler {
   boolean artifactOmitted(Artifact artifact);
 
   /**
-   * @return Whether the artifact's data was injected.
-   * @throws IOException if implementation tried to stat the Artifact which threw an exception.
-   *     Technically, this means that the artifact could not have been injected, but by throwing
-   *     here we save the caller trying to stat this file on their own and throwing the same
-   *     exception. Implementations are not guaranteed to throw in this case if they are able to
-   *     determine that the artifact is not injected without statting it.
-   */
-  boolean isInjected(Artifact file) throws IOException;
-
-  /**
    * Discards all known output artifact metadata, presumably because outputs will be modified. May
    * only be called before any metadata is injected using {@link #injectDigest} or {@link
    * #markOmitted};