Remove OutputMetadataStore.constructMetadataForDigest().

RELNOTES: None.
PiperOrigin-RevId: 690742227
Change-Id: I8916a2eb1dd3ff333c9e1cffd3042e0b9dc66b5b

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

@@ -272,7 +272,6 @@ public static FileArtifactValue createForUnresolvedSymlink(Path symlink) throws
     return new UnresolvedSymlinkArtifactValue(symlink);
   }
 
-  @VisibleForTesting
   public static FileArtifactValue createForNormalFile(
       byte[] digest, @Nullable FileContentsProxy proxy, long size) {
     return new RegularFileArtifactValue(digest, proxy, size);

src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java

@@ -105,7 +105,6 @@ public static FileStateValue create(
         yield createWithStatNoFollow(
             rootedPath,
             checkNotNull(FileStatusWithDigestAdapter.maybeAdapt(stat), rootedPath),
-            /* digestWillBeInjected= */ false,
             syscallCache,
             tsgm);
       }
@@ -115,16 +114,14 @@ yield createWithStatNoFollow(
   public static FileStateValue createWithStatNoFollow(
       RootedPath rootedPath,
       FileStatusWithDigest statNoFollow,
-      boolean digestWillBeInjected,
       XattrProvider xattrProvider,
       @Nullable TimestampGranularityMonitor tsgm)
       throws IOException {
     Path path = rootedPath.asPath();
     if (statNoFollow.isFile()) {
       return statNoFollow.isSpecialFile()
           ? SpecialFileStateValue.fromStat(path.asFragment(), statNoFollow, tsgm)
-          : createRegularFileStateValueFromPath(
-              path, statNoFollow, digestWillBeInjected, xattrProvider, tsgm);
+          : createRegularFileStateValueFromPath(path, statNoFollow, xattrProvider, tsgm);
     } else if (statNoFollow.isDirectory()) {
       return DIRECTORY_FILE_STATE_NODE;
     } else if (statNoFollow.isSymbolicLink()) {
@@ -147,17 +144,13 @@ public static FileStateValue createWithStatNoFollow(
   private static FileStateValue createRegularFileStateValueFromPath(
       Path path,
       FileStatusWithDigest stat,
-      boolean digestWillBeInjected,
       XattrProvider xattrProvider,
       @Nullable TimestampGranularityMonitor tsgm)
       throws InconsistentFilesystemException {
     checkState(stat.isFile(), path);
 
     try {
-      // If the digest will be injected, we can skip calling getFastDigest, but we need to store a
-      // contents proxy because if the digest is injected but is not available from the filesystem,
-      // we will need the proxy to determine whether the file was modified.
-      byte[] digest = digestWillBeInjected ? null : tryGetDigest(path, stat, xattrProvider);
+      byte[] digest = tryGetDigest(path, stat, xattrProvider);
       if (digest == null) {
         // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe method.
         if (tsgm != null) {

src/main/java/com/google/devtools/build/lib/actions/cache/OutputMetadataStore.java

@@ -21,7 +21,6 @@
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileStateType;
 import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
-import com.google.devtools.build.lib.vfs.FileStatus;
 import java.io.IOException;
 
 /** Handles the metadata of the outputs of the action during its execution. */
@@ -50,17 +49,6 @@ public interface OutputMetadataStore extends MetadataInjector {
   /** Sets digest for virtual artifacts (e.g. middlemen). {@code digest} must not be null. */
   void setDigestForVirtualArtifact(Artifact artifact, byte[] digest);
 
-  /**
-   * Constructs a {@link FileArtifactValue} for the given output whose digest is known.
-   *
-   * <p>This call does not inject the returned metadata. It should be injected with a followup call
-   * to {@link #injectFile} or {@link #injectTree} as appropriate.
-   *
-   * <p>chmod will not be called on the output.
-   */
-  FileArtifactValue constructMetadataForDigest(
-      Artifact output, FileStatus statNoFollow, byte[] injectedDigest) throws IOException;
-
   /**
    * Retrieves the children of a tree artifact, returning an empty set if there is no data
    * available.

src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java

@@ -26,7 +26,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.flogger.GoogleLogger;
-import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
@@ -310,8 +309,7 @@ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifa
             archivedTreeArtifact,
             constructFileArtifactValue(
                 archivedTreeArtifact,
-                FileStatusWithDigestAdapter.maybeAdapt(archivedStatNoFollow),
-                /* injectedDigest= */ null));
+                FileStatusWithDigestAdapter.maybeAdapt(archivedStatNoFollow)));
       } else {
         logger.atInfo().atMostEvery(5, MINUTES).log(
             "Archived tree artifact: %s not created", archivedTreeArtifact);
@@ -337,20 +335,6 @@ public ImmutableSet<TreeFileArtifact> getTreeArtifactChildren(SpecialArtifact tr
     return tree != null ? tree.getChildren() : ImmutableSet.of();
   }
 
-  @Override
-  public FileArtifactValue constructMetadataForDigest(
-      Artifact output, FileStatus statNoFollow, byte[] digest) throws IOException {
-    checkArgument(!output.isSymlink(), "%s is a symlink", output);
-    checkNotNull(digest, "Missing digest for %s", output);
-    checkNotNull(statNoFollow, "Missing stat for %s", output);
-    checkState(
-        executionMode.get(), "Tried to construct metadata for %s outside of execution", output);
-
-    // We already have a stat, so no need to call chmod.
-    return constructFileArtifactValue(
-        output, FileStatusWithDigestAdapter.maybeAdapt(statNoFollow), digest);
-  }
-
   @Override
   public void injectFile(Artifact output, FileArtifactValue metadata) {
     checkArgument(isKnownOutput(output), "%s is not a declared output of this action", output);
@@ -423,23 +407,19 @@ public String toString() {
   /** Constructs a new {@link FileArtifactValue} by reading from the file system. */
   private FileArtifactValue constructFileArtifactValueFromFilesystem(Artifact artifact)
       throws IOException {
-    return constructFileArtifactValue(artifact, /*statNoFollow=*/ null, /*injectedDigest=*/ null);
+    return constructFileArtifactValue(artifact, /* statNoFollow= */ null);
   }
 
-  /** Constructs a new {@link FileArtifactValue}, optionally taking a known stat and digest. */
+  /** Constructs a new {@link FileArtifactValue}, optionally taking a known stat. */
   private FileArtifactValue constructFileArtifactValue(
-      Artifact artifact,
-      @Nullable FileStatusWithDigest statNoFollow,
-      @Nullable byte[] injectedDigest)
-      throws IOException {
+      Artifact artifact, @Nullable FileStatusWithDigest statNoFollow) throws IOException {
     checkState(!artifact.isTreeArtifact(), "%s is a tree artifact", artifact);
 
     var statAndValue =
         fileArtifactValueFromArtifact(
             artifact,
             artifactPathResolver,
             statNoFollow,
-            injectedDigest != null,
             xattrProvider,
             // Prevent constant metadata artifacts from notifying the timestamp granularity monitor
             // and potentially delaying the build for no reason.
@@ -462,15 +442,6 @@ private FileArtifactValue constructFileArtifactValue(
 
     // Ensure that we don't have both an injected digest and a digest from the filesystem.
     byte[] fileDigest = value.getDigest();
-    if (fileDigest != null && injectedDigest != null) {
-      throw new IllegalStateException(
-          String.format(
-              "Digest %s was injected for artifact %s, but got %s from the filesystem (%s)",
-              BaseEncoding.base16().encode(injectedDigest),
-              artifact,
-              BaseEncoding.base16().encode(fileDigest),
-              value));
-    }
 
     FileStateType type = value.getType();
 
@@ -502,7 +473,8 @@ private FileArtifactValue constructFileArtifactValue(
           artifactPathResolver.toPath(artifact).getLastModifiedTime());
     }
 
-    if (injectedDigest == null && type.isFile()) {
+    byte[] digest = null;
+    if (type.isFile()) {
       // We don't have an injected digest and there is no digest in the file value (which attempts a
       // fast digest). Manually compute the digest instead.
       Path path = statAndValue.pathNoFollow();
@@ -515,9 +487,9 @@ private FileArtifactValue constructFileArtifactValue(
         path = statAndValue.realPath();
       }
 
-      injectedDigest = DigestUtils.manuallyComputeDigest(path);
+      digest = DigestUtils.manuallyComputeDigest(path);
     }
-    return FileArtifactValue.createFromInjectedDigest(value, injectedDigest);
+    return FileArtifactValue.createFromInjectedDigest(value, digest);
   }
 
   /**
@@ -538,7 +510,6 @@ static FileArtifactValue fileArtifactValueFromArtifact(
             artifact,
             ArtifactPathResolver.IDENTITY,
             statNoFollow,
-            /* digestWillBeInjected= */ false,
             xattrProvider,
             tsgm)
         .fileArtifactValue();
@@ -548,7 +519,6 @@ private static FileArtifactStatAndValue fileArtifactValueFromArtifact(
       Artifact artifact,
       ArtifactPathResolver artifactPathResolver,
       @Nullable FileStatusWithDigest statNoFollow,
-      boolean digestWillBeInjected,
       XattrProvider xattrProvider,
       @Nullable TimestampGranularityMonitor tsgm)
       throws IOException {
@@ -578,8 +548,7 @@ private static FileArtifactStatAndValue fileArtifactValueFromArtifact(
 
     if (statNoFollow == null || !statNoFollow.isSymbolicLink()) {
       var fileArtifactValue =
-          fileArtifactValueFromStat(
-              rootedPathNoFollow, statNoFollow, digestWillBeInjected, xattrProvider, tsgm);
+          fileArtifactValueFromStat(rootedPathNoFollow, statNoFollow, xattrProvider, tsgm);
       return FileArtifactStatAndValue.create(
           pathNoFollow, /* realPath= */ null, statNoFollow, fileArtifactValue);
     }
@@ -602,8 +571,7 @@ private static FileArtifactStatAndValue fileArtifactValueFromArtifact(
     FileStatus realStat = realRootedPath.asPath().statIfFound(Symlinks.NOFOLLOW);
     FileStatusWithDigest realStatWithDigest = FileStatusWithDigestAdapter.maybeAdapt(realStat);
     var fileArtifactValue =
-        fileArtifactValueFromStat(
-            realRootedPath, realStatWithDigest, digestWillBeInjected, xattrProvider, tsgm);
+        fileArtifactValueFromStat(realRootedPath, realStatWithDigest, xattrProvider, tsgm);
     return FileArtifactStatAndValue.create(pathNoFollow, realPath, statNoFollow, fileArtifactValue);
   }
 
@@ -632,7 +600,6 @@ public static FileArtifactStatAndValue create(
   private static FileArtifactValue fileArtifactValueFromStat(
       RootedPath rootedPath,
       FileStatusWithDigest stat,
-      boolean digestWillBeInjected,
       XattrProvider xattrProvider,
       @Nullable TimestampGranularityMonitor tsgm)
       throws IOException {
@@ -649,8 +616,7 @@ private static FileArtifactValue fileArtifactValueFromStat(
     }
 
     FileStateValue fileStateValue =
-        FileStateValue.createWithStatNoFollow(
-            rootedPath, stat, digestWillBeInjected, xattrProvider, tsgm);
+        FileStateValue.createWithStatNoFollow(rootedPath, stat, xattrProvider, tsgm);
 
     return FileArtifactValue.createForNormalFile(
         fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize());

src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java

@@ -89,7 +89,6 @@
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
-import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
@@ -973,12 +972,6 @@ public TreeArtifactValue getTreeArtifactValue(SpecialArtifact treeArtifact)
       throw new UnsupportedOperationException();
     }
 
-    @Override
-    public FileArtifactValue constructMetadataForDigest(
-        Artifact output, FileStatus statNoFollow, byte[] digest) {
-      throw new UnsupportedOperationException();
-    }
-
     @Override
     public void injectFile(Artifact output, FileArtifactValue metadata) {
       throw new UnsupportedOperationException();

src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java

@@ -40,10 +40,8 @@
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
 import com.google.devtools.build.lib.actions.BuildFailedException;
-import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
 import com.google.devtools.build.lib.actions.InputMetadataProvider;
-import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.actions.util.TestAction;
 import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
@@ -62,14 +60,11 @@
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.util.CrashFailureDetails;
 import com.google.devtools.build.lib.util.DetailedExitCode;
-import com.google.devtools.build.lib.vfs.DigestHashFunction;
-import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
-import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
@@ -605,51 +600,6 @@ void run(ActionExecutionContext actionExecutionContext) throws IOException {
     assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
   }
 
-  @Test
-  public void constructMetadataForDigest() throws Exception {
-    SpecialArtifact out = createTreeArtifact("output");
-    Action action =
-        new SimpleTestAction(out) {
-          @Override
-          void run(ActionExecutionContext actionExecutionContext) throws IOException {
-            TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(out, "one");
-            TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(out, "two");
-            writeFile(child1, "one");
-            writeFile(child2, "two");
-
-            OutputMetadataStore md = actionExecutionContext.getOutputMetadataStore();
-            FileStatus stat = child1.getPath().stat(Symlinks.NOFOLLOW);
-            FileArtifactValue metadata1 =
-                md.constructMetadataForDigest(
-                    child1,
-                    stat,
-                    DigestHashFunction.SHA256.getHashFunction().hashString("one", UTF_8).asBytes());
-
-            stat = child2.getPath().stat(Symlinks.NOFOLLOW);
-            FileArtifactValue metadata2 =
-                md.constructMetadataForDigest(
-                    child2,
-                    stat,
-                    DigestHashFunction.SHA256.getHashFunction().hashString("two", UTF_8).asBytes());
-
-            // The metadata will not be equal to reading from the filesystem since the filesystem
-            // won't have the digest. However, we should be able to detect that nothing could have
-            // been modified.
-            assertThat(
-                    metadata1.couldBeModifiedSince(
-                        FileArtifactValue.createForTesting(child1.getPath())))
-                .isFalse();
-            assertThat(
-                    metadata2.couldBeModifiedSince(
-                        FileArtifactValue.createForTesting(child2.getPath())))
-                .isFalse();
-          }
-        };
-
-    registerAction(action);
-    buildArtifact(out);
-  }
-
   @Test
   public void remoteDirectoryInjection() throws Exception {
     SpecialArtifact out = createTreeArtifact("output");