Remove OutputMetadataStore.constructMetadataForDigest().
RELNOTES: None.
PiperOrigin-RevId: 690742227
Change-Id: I8916a2eb1dd3ff333c9e1cffd3042e0b9dc66b5b
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
index 704972c..9d670a6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -272,7 +272,6 @@
return new UnresolvedSymlinkArtifactValue(symlink);
}
- @VisibleForTesting
public static FileArtifactValue createForNormalFile(
byte[] digest, @Nullable FileContentsProxy proxy, long size) {
return new RegularFileArtifactValue(digest, proxy, size);
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
index 3a9812a..1cff7b7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileStateValue.java
@@ -105,7 +105,6 @@
yield createWithStatNoFollow(
rootedPath,
checkNotNull(FileStatusWithDigestAdapter.maybeAdapt(stat), rootedPath),
- /* digestWillBeInjected= */ false,
syscallCache,
tsgm);
}
@@ -115,7 +114,6 @@
public static FileStateValue createWithStatNoFollow(
RootedPath rootedPath,
FileStatusWithDigest statNoFollow,
- boolean digestWillBeInjected,
XattrProvider xattrProvider,
@Nullable TimestampGranularityMonitor tsgm)
throws IOException {
@@ -123,8 +121,7 @@
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 @@
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) {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/OutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/actions/cache/OutputMetadataStore.java
index 732fd0e..e968b1b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/OutputMetadataStore.java
+++ b/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. */
@@ -51,17 +50,6 @@
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.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
index 735d6ae..3056f1a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputMetadataStore.java
+++ b/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 @@
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);
@@ -338,20 +336,6 @@
}
@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);
checkArgument(
@@ -423,15 +407,12 @@
/** 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 =
@@ -439,7 +420,6 @@
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 @@
// 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 @@
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 @@
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 @@
artifact,
ArtifactPathResolver.IDENTITY,
statNoFollow,
- /* digestWillBeInjected= */ false,
xattrProvider,
tsgm)
.fileArtifactValue();
@@ -548,7 +519,6 @@
Artifact artifact,
ArtifactPathResolver artifactPathResolver,
@Nullable FileStatusWithDigest statNoFollow,
- boolean digestWillBeInjected,
XattrProvider xattrProvider,
@Nullable TimestampGranularityMonitor tsgm)
throws IOException {
@@ -578,8 +548,7 @@
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 @@
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 @@
private static FileArtifactValue fileArtifactValueFromStat(
RootedPath rootedPath,
FileStatusWithDigest stat,
- boolean digestWillBeInjected,
XattrProvider xattrProvider,
@Nullable TimestampGranularityMonitor tsgm)
throws IOException {
@@ -649,8 +616,7 @@
}
FileStateValue fileStateValue =
- FileStateValue.createWithStatNoFollow(
- rootedPath, stat, digestWillBeInjected, xattrProvider, tsgm);
+ FileStateValue.createWithStatNoFollow(rootedPath, stat, xattrProvider, tsgm);
return FileArtifactValue.createForNormalFile(
fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize());
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index e970614..827254e 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/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;
@@ -974,12 +973,6 @@
}
@Override
- public FileArtifactValue constructMetadataForDigest(
- Artifact output, FileStatus statNoFollow, byte[] digest) {
- throw new UnsupportedOperationException();
- }
-
- @Override
public void injectFile(Artifact output, FileArtifactValue metadata) {
throw new UnsupportedOperationException();
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index ca59878..06f6c77 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/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;
@@ -606,51 +601,6 @@
}
@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");
RemoteFileArtifactValue remoteFile1 =
