Replace FileValue with ArtifactFileMetadata for tracking metadata during execution. FileValue unnecessarily keeps a reference to a Path (via RootedPath) which is not needed for execution.
Did a drive-by functional change of getting rid of special-casing for empty file in ActionMetadataHandler: special handling of empty files has been gone for years. Also reworked comments to reflect the current state of the world (namely, additionalOutputData is often the primary source of metadata).
Only potential semantic change I can see is that we now get the last modified time of an artifact in FilesystemValueChecker by doing a getLastModifiedTime on the artifact's path directly, versus the real path of the FileValue. That is safe, though, because the real path of the FileValue came directly from our just reading the artifact this time.
PiperOrigin-RevId: 213005841
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFileMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFileMetadata.java
new file mode 100644
index 0000000..8554f7a
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFileMetadata.java
@@ -0,0 +1,266 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.actions;
+
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Objects;
+import javax.annotation.Nullable;
+
+/**
+ * A value that corresponds to the metadata for an artifact, which may be a file or directory or
+ * symlink or non-existent file, fully accounting for symlinks (e.g. proper dependencies on ancestor
+ * symlinks so as to be incrementally correct).
+ *
+ * <p>Note that the existence of this object does not imply that the file exists on the filesystem.
+ * Values for missing files may be created on purpose in order to facilitate incremental builds in
+ * the case those files have reappeared.
+ *
+ * <p>Very similar to {@link FileValue}, but contains strictly less information: does not have a
+ * {@link com.google.devtools.build.lib.vfs.RootedPath}, since execution never needs to access the
+ * filesystem via this object.
+ */
+@Immutable
+@ThreadSafe
+public abstract class ArtifactFileMetadata {
+ /**
+ * Exists to accommodate the control flow of {@link
+ * com.google.devtools.build.lib.skyframe.ActionMetadataHandler#getMetadata}.
+ *
+ * <p>{@link com.google.devtools.build.lib.skyframe.ActionMetadataHandler#getMetadata} always
+ * checks {@link com.google.devtools.build.lib.skyframe.ActionMetadataHandler#outputArtifactData}
+ * before checking {@link
+ * com.google.devtools.build.lib.skyframe.ActionMetadataHandler#additionalOutputData} so some
+ * placeholder value is needed to allow an injected {@link FileArtifactValue} to be returned.
+ *
+ * <p>Similarly, {@link
+ * com.google.devtools.build.lib.skyframe.ActionExecutionValue#getAllFileValues} replaces this
+ * placeholder with metadata from {@link
+ * com.google.devtools.build.lib.skyframe.ActionExecutionValue#additionalOutputData}.
+ */
+ @AutoCodec public static final ArtifactFileMetadata PLACEHOLDER = new PlaceholderFileValue();
+
+ // No implementations outside this class.
+ private ArtifactFileMetadata() {}
+
+ public boolean exists() {
+ return realFileStateValue().getType() != FileStateType.NONEXISTENT;
+ }
+
+ /** Returns true if the original path is a symlink; the target path can never be a symlink. */
+ public boolean isSymlink() {
+ return false;
+ }
+
+ /**
+ * Returns true if this value corresponds to a file or symlink to an existing regular or special
+ * file. If so, its parent directory is guaranteed to exist.
+ */
+ public boolean isFile() {
+ return realFileStateValue().getType() == FileStateType.REGULAR_FILE
+ || realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
+ }
+
+ /**
+ * Returns true if this value corresponds to a file or symlink to an existing special file. If so,
+ * its parent directory is guaranteed to exist.
+ */
+ public boolean isSpecialFile() {
+ return realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
+ }
+
+ /**
+ * Returns true if the file is a directory or a symlink to an existing directory. If so, its
+ * parent directory is guaranteed to exist.
+ */
+ public boolean isDirectory() {
+ return realFileStateValue().getType() == FileStateType.DIRECTORY;
+ }
+
+ public abstract FileStateValue realFileStateValue();
+
+ public long getSize() {
+ Preconditions.checkState(isFile(), this);
+ return realFileStateValue().getSize();
+ }
+
+ @Nullable
+ public byte[] getDigest() {
+ Preconditions.checkState(isFile(), this);
+ return realFileStateValue().getDigest();
+ }
+
+ public static ArtifactFileMetadata value(
+ PathFragment pathFragment,
+ FileStateValue fileStateValue,
+ PathFragment realPathFragment,
+ FileStateValue realFileStateValue) {
+ Preconditions.checkState(pathFragment.isAbsolute(), pathFragment);
+ Preconditions.checkState(realPathFragment.isAbsolute(), realPathFragment);
+ if (pathFragment.equals(realPathFragment)) {
+ Preconditions.checkState(
+ fileStateValue.getType() != FileStateType.SYMLINK,
+ "path: %s, fileStateValue: %s, realPath: %s, realFileStateValue: %s",
+ pathFragment,
+ fileStateValue,
+ realPathFragment,
+ realFileStateValue);
+ return new Regular(pathFragment, fileStateValue);
+ } else {
+ if (fileStateValue.getType() == FileStateType.SYMLINK) {
+ return new Symlink(realPathFragment, realFileStateValue, fileStateValue.getSymlinkTarget());
+ } else {
+ return new DifferentRealPath(realPathFragment, realFileStateValue);
+ }
+ }
+ }
+
+ /**
+ * Implementation of {@link ArtifactFileMetadata} for files whose fully resolved path is the same
+ * as the requested path. For example, this is the case for the path "foo/bar/baz" if neither
+ * 'foo' nor 'foo/bar' nor 'foo/bar/baz' are symlinks.
+ */
+ private static final class Regular extends ArtifactFileMetadata {
+ private final PathFragment realPath;
+ private final FileStateValue fileStateValue;
+
+ Regular(PathFragment realPath, FileStateValue fileStateValue) {
+ this.realPath = Preconditions.checkNotNull(realPath);
+ this.fileStateValue = Preconditions.checkNotNull(fileStateValue);
+ }
+
+ @Override
+ public FileStateValue realFileStateValue() {
+ return fileStateValue;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ if (obj.getClass() != Regular.class) {
+ return false;
+ }
+ Regular other = (Regular) obj;
+ return realPath.equals(other.realPath) && fileStateValue.equals(other.fileStateValue);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(realPath, fileStateValue);
+ }
+
+ @Override
+ public String toString() {
+ return realPath + ", " + fileStateValue;
+ }
+ }
+
+ /**
+ * Base class for {@link ArtifactFileMetadata}s for files whose fully resolved path is different
+ * than the requested path. For example, this is the case for the path "foo/bar/baz" if at least
+ * one of 'foo', 'foo/bar', or 'foo/bar/baz' is a symlink.
+ */
+ private static class DifferentRealPath extends ArtifactFileMetadata {
+ protected final PathFragment realPath;
+ protected final FileStateValue realFileStateValue;
+
+ DifferentRealPath(PathFragment realPath, FileStateValue realFileStateValue) {
+ this.realPath = Preconditions.checkNotNull(realPath);
+ this.realFileStateValue = Preconditions.checkNotNull(realFileStateValue);
+ }
+
+ @Override
+ public FileStateValue realFileStateValue() {
+ return realFileStateValue;
+ }
+
+ @SuppressWarnings("EqualsGetClass") // Only subclass should never be equal to this class.
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ if (obj.getClass() != DifferentRealPath.class) {
+ return false;
+ }
+ DifferentRealPath other = (DifferentRealPath) obj;
+ return realPath.equals(other.realPath) && realFileStateValue.equals(other.realFileStateValue);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(realPath, realFileStateValue);
+ }
+
+ @Override
+ public String toString() {
+ return realPath + ", " + realFileStateValue + " (symlink ancestor)";
+ }
+ }
+
+ /** Implementation of {@link ArtifactFileMetadata} for files that are symlinks. */
+ private static final class Symlink extends DifferentRealPath {
+ private final PathFragment linkTarget;
+
+ private Symlink(
+ PathFragment realPath, FileStateValue realFileStateValue, PathFragment linkTarget) {
+ super(realPath, realFileStateValue);
+ this.linkTarget = linkTarget;
+ }
+
+ @Override
+ public boolean isSymlink() {
+ return true;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj == null) {
+ return false;
+ }
+ if (obj.getClass() != Symlink.class) {
+ return false;
+ }
+ Symlink other = (Symlink) obj;
+ return realPath.equals(other.realPath)
+ && realFileStateValue.equals(other.realFileStateValue)
+ && linkTarget.equals(other.linkTarget);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(realPath, realFileStateValue, linkTarget, Boolean.TRUE);
+ }
+
+ @Override
+ public String toString() {
+ return String.format(
+ "symlink (real_path=%s, real_state=%s, link_value=%s)",
+ realPath, realFileStateValue, linkTarget);
+ }
+ }
+
+ private static final class PlaceholderFileValue extends ArtifactFileMetadata {
+ private PlaceholderFileValue() {}
+ @Override
+ public FileStateValue realFileStateValue() {
+ throw new UnsupportedOperationException();
+ }
+ }
+}
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 aa6b7cc..416b2a6 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
@@ -167,9 +167,24 @@
isFile ? fileValue.getDigest() : null);
}
+ public static FileArtifactValue create(Artifact artifact, ArtifactFileMetadata metadata)
+ throws IOException {
+ boolean isFile = metadata.isFile();
+ FileContentsProxy proxy = getProxyFromFileStateValue(metadata.realFileStateValue());
+ return create(
+ artifact.getPath(),
+ isFile,
+ isFile ? metadata.getSize() : 0,
+ proxy,
+ isFile ? metadata.getDigest() : null);
+ }
+
public static FileArtifactValue create(
- Artifact artifact, ArtifactPathResolver resolver, FileValue fileValue,
- @Nullable byte[] injectedDigest) throws IOException {
+ Artifact artifact,
+ ArtifactPathResolver resolver,
+ ArtifactFileMetadata fileValue,
+ @Nullable byte[] injectedDigest)
+ throws IOException {
boolean isFile = fileValue.isFile();
FileContentsProxy proxy = getProxyFromFileStateValue(fileValue.realFileStateValue());
return create(
@@ -217,9 +232,10 @@
return new RegularFileArtifactValue(digest, proxy, size);
}
- public static FileArtifactValue createNormalFile(FileValue fileValue) {
- FileContentsProxy proxy = getProxyFromFileStateValue(fileValue.realFileStateValue());
- return new RegularFileArtifactValue(fileValue.getDigest(), proxy, fileValue.getSize());
+ public static FileArtifactValue createNormalFile(ArtifactFileMetadata artifactMetadata) {
+ FileContentsProxy proxy = getProxyFromFileStateValue(artifactMetadata.realFileStateValue());
+ return new RegularFileArtifactValue(
+ artifactMetadata.getDigest(), proxy, artifactMetadata.getSize());
}
@VisibleForTesting
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
index 89450d2..314101d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileValue.java
@@ -52,16 +52,6 @@
// Depends non-hermetically on package path.
public static final SkyFunctionName FILE = SkyFunctionName.createNonHermetic("FILE");
- /**
- * Exists to accommodate the control flow of {@link ActionMetadataHandler#getMetadata}.
- *
- * <p>{@link ActionMetadataHandler#getMetadata} always checks {@link
- * ActionMetadataHandler#outputArtifactData} before checking {@link
- * ActionMetadataHandler#additionalOutputData} so some placeholder value is needed to allow an
- * injected {@link FileArtifactValue} to be returned.
- */
- @AutoCodec public static final FileValue PLACEHOLDER = new PlaceholderFileValue();
-
public boolean exists() {
return realFileStateValue().getType() != FileStateType.NONEXISTENT;
}
@@ -327,18 +317,4 @@
realRootedPath, realFileStateValue, linkTarget);
}
}
-
- private static final class PlaceholderFileValue extends FileValue {
- private PlaceholderFileValue() {}
-
- @Override
- public RootedPath realRootedPath() {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public FileStateValue realFileStateValue() {
- throw new UnsupportedOperationException();
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index c6197ee..4c7f32a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateValue;
@@ -35,7 +36,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.skyframe.serialization.UnshareableValue;
-import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Map;
@@ -45,29 +46,28 @@
/**
* A value representing an executed action.
+ *
+ * <p>Concerning the data in this class:
+ *
+ * <p>We want to track all output data from an ActionExecutionValue. See {@link
+ * ActionMetadataHandler} for a discussion of how its fields {@link
+ * ActionMetadataHandler#outputArtifactData} and {@link ActionMetadataHandler#additionalOutputData}
+ * relate. The relationship is the same between {@link #artifactData} and {@link
+ * #additionalOutputData} here.
*/
@Immutable
@ThreadSafe
public class ActionExecutionValue implements SkyValue {
- /*
- Concerning the data in this class:
-
- We want to track all output data from an ActionExecutionValue. However, we want to separate
- quickly-accessible Filesystem data from other kinds of data. We use FileValues
- to represent data that may be quickly accessed, TreeArtifactValues to give us directory contents,
- and FileArtifactValues inside TreeArtifactValues or the additionalOutputData map
- to give us full mtime/digest information on all output files.
-
- The reason for this separation is so that FileSystemValueChecker remains fast. When it checks
- the validity of an ActionExecutionValue, it only checks the quickly-accessible data stored
- in FileValues and TreeArtifactValues.
- */
/**
- * The FileValues of all files for this ActionExecutionValue. These FileValues can be
- * read and checked quickly from the filesystem, unlike FileArtifactValues.
+ * The metadata of all files for this ActionExecutionValue whose filesystem metadata differs from
+ * the metadata in {@link #additionalOutputData} or is not present there. This metadata can be
+ * checked quickly against the actual filesystem on incremental builds.
+ *
+ * <p>If additional metadata is not needed here over what is in {@link #additionalOutputData}, the
+ * value will be {@link ArtifactFileMetadata#PLACEHOLDER}.
*/
- private final ImmutableMap<Artifact, FileValue> artifactData;
+ private final ImmutableMap<Artifact, ArtifactFileMetadata> artifactData;
/** The TreeArtifactValue of all TreeArtifacts output by this Action. */
private final ImmutableMap<Artifact, TreeArtifactValue> treeArtifactData;
@@ -83,7 +83,7 @@
@Nullable private final NestedSet<Artifact> discoveredModules;
/**
- * @param artifactData Map from Artifacts to corresponding FileValues.
+ * @param artifactData Map from Artifacts to corresponding {@link ArtifactFileMetadata}.
* @param treeArtifactData All tree artifact data.
* @param additionalOutputData Map from Artifacts to values if the FileArtifactValue for this
* artifact cannot be derived from the corresponding FileValue (see {@link
@@ -94,12 +94,12 @@
* @param discoveredModules cpp modules discovered
*/
private ActionExecutionValue(
- Map<Artifact, FileValue> artifactData,
+ Map<Artifact, ArtifactFileMetadata> artifactData,
Map<Artifact, TreeArtifactValue> treeArtifactData,
Map<Artifact, FileArtifactValue> additionalOutputData,
@Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
@Nullable NestedSet<Artifact> discoveredModules) {
- this.artifactData = ImmutableMap.<Artifact, FileValue>copyOf(artifactData);
+ this.artifactData = ImmutableMap.copyOf(artifactData);
this.additionalOutputData = ImmutableMap.copyOf(additionalOutputData);
this.treeArtifactData = ImmutableMap.copyOf(treeArtifactData);
this.outputSymlinks = outputSymlinks;
@@ -107,7 +107,7 @@
}
static ActionExecutionValue create(
- Map<Artifact, FileValue> artifactData,
+ Map<Artifact, ArtifactFileMetadata> artifactData,
Map<Artifact, TreeArtifactValue> treeArtifactData,
Map<Artifact, FileArtifactValue> additionalOutputData,
@Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
@@ -136,9 +136,9 @@
/**
* @return The data for each non-middleman output of this action, in the form of the {@link
- * FileValue} that would be created for the file if it were to be read from disk.
+ * FileValue} that would be created for the file if it were to be read from disk.
*/
- FileValue getData(Artifact artifact) {
+ ArtifactFileMetadata getData(Artifact artifact) {
Preconditions.checkState(!additionalOutputData.containsKey(artifact),
"Should not be requesting data for already-constructed FileArtifactValue: %s", artifact);
return artifactData.get(artifact);
@@ -154,7 +154,7 @@
* returned by {@link #getData}. Primarily needed by {@link FilesystemValueChecker}, also
* called by {@link ArtifactFunction} when aggregating a {@link TreeArtifactValue}.
*/
- Map<Artifact, FileValue> getAllFileValues() {
+ Map<Artifact, ArtifactFileMetadata> getAllFileValues() {
return Maps.transformEntries(artifactData, this::transformIfPlaceholder);
}
@@ -220,18 +220,20 @@
return Objects.hashCode(artifactData, treeArtifactData, additionalOutputData);
}
- /** Transforms PLACEHOLDER values into RegularFileValue instances. */
- private FileValue transformIfPlaceholder(Artifact artifact, FileValue value) {
- if (value == FileValue.PLACEHOLDER) {
+ /** Transforms PLACEHOLDER values into normal instances. */
+ private ArtifactFileMetadata transformIfPlaceholder(
+ Artifact artifact, ArtifactFileMetadata value) {
+ if (value == ArtifactFileMetadata.PLACEHOLDER) {
FileArtifactValue metadata =
Preconditions.checkNotNull(
additionalOutputData.get(artifact),
"Placeholder without corresponding FileArtifactValue for: %s",
artifact);
- return new FileValue.RegularFileValue(
- RootedPath.toRootedPath(artifact.getRoot().getRoot(), artifact.getRootRelativePath()),
+ FileStateValue.RegularFileStateValue fileStateValue =
new FileStateValue.RegularFileStateValue(
- metadata.getSize(), metadata.getDigest(), /*contentsProxy=*/ null));
+ metadata.getSize(), metadata.getDigest(), /*contentsProxy=*/ null);
+ PathFragment pathFragment = artifact.getPath().asFragment();
+ return ArtifactFileMetadata.value(pathFragment, fileStateValue, pathFragment, fileStateValue);
}
return value;
}
@@ -243,7 +245,7 @@
private static class CrossServerUnshareableActionExecutionValue extends ActionExecutionValue
implements UnshareableValue {
CrossServerUnshareableActionExecutionValue(
- Map<Artifact, FileValue> artifactData,
+ Map<Artifact, ArtifactFileMetadata> artifactData,
Map<Artifact, TreeArtifactValue> treeArtifactData,
Map<Artifact, FileArtifactValue> additionalOutputData,
@Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 4b0a1a6..5dfe7b0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -26,10 +26,10 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateValue;
-import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.cache.Md5Digest;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -84,8 +84,12 @@
*/
private final ActionInputMap inputArtifactData;
- /** FileValues for each output Artifact. */
- private final ConcurrentMap<Artifact, FileValue> outputArtifactData =
+ /**
+ * {@link ArtifactFileMetadata} for each output Artifact. The value is {@link
+ * ArtifactFileMetadata#PLACEHOLDER} if the artifact's metadata is not fully captured in {@link
+ * #additionalOutputData}.
+ */
+ private final ConcurrentMap<Artifact, ArtifactFileMetadata> outputArtifactData =
new ConcurrentHashMap<>();
/**
@@ -210,8 +214,8 @@
throw new FileNotFoundException(artifact + " not found");
}
// It's an ordinary artifact.
- FileValue fileValue = outputArtifactData.get(artifact);
- if (fileValue != null) {
+ ArtifactFileMetadata fileMetadata = outputArtifactData.get(artifact);
+ if (fileMetadata != null) {
// Non-middleman artifacts should only have additionalOutputData if they have
// outputArtifactData. We don't assert this because of concurrency possibilities, but at least
// we don't check additionalOutputData unless we expect that we might see the artifact there.
@@ -221,15 +225,15 @@
if (value != null) {
return metadataFromValue(value);
}
- if (!fileValue.exists()) {
+ if (!fileMetadata.exists()) {
throw new FileNotFoundException(artifact.prettyPrint() + " does not exist");
}
- return FileArtifactValue.createNormalFile(fileValue);
+ return FileArtifactValue.createNormalFile(fileMetadata);
}
// We do not cache exceptions besides nonexistence here, because it is unlikely that the file
// will be requested from this cache too many times.
- fileValue = constructFileValue(artifact, /*statNoFollow=*/ null);
- return maybeStoreAdditionalData(artifact, fileValue, null);
+ fileMetadata = constructArtifactFileMetadata(artifact, /*statNoFollow=*/ null);
+ return maybeStoreAdditionalData(artifact, fileMetadata, null);
}
/**
@@ -238,7 +242,8 @@
*/
@Nullable
private FileArtifactValue maybeStoreAdditionalData(
- Artifact artifact, FileValue data, @Nullable byte[] injectedDigest) throws IOException {
+ Artifact artifact, ArtifactFileMetadata data, @Nullable byte[] injectedDigest)
+ throws IOException {
if (!data.exists()) {
// Nonexistent files should only occur before executing an action.
throw new FileNotFoundException(artifact.prettyPrint() + " does not exist");
@@ -249,11 +254,12 @@
// and that is all that is needed for this file's metadata.
return FileArtifactValue.createNormalFile(data);
}
- // Unfortunately, the FileValue does not contain enough information for us to calculate the
- // corresponding FileArtifactValue -- either the metadata must use the modified time, which we
- // do not expose in the FileValue, or the FileValue didn't store the digest So we store the
- // metadata separately.
- // Use the FileValue's digest if no digest was injected, or if the file can't be digested.
+ // Unfortunately, the ArtifactFileMetadata does not contain enough information for us to
+ // calculate the corresponding FileArtifactValue -- either the metadata must use the modified
+ // time, which we do not expose in the ArtifactFileMetadata, or the ArtifactFileMetadata didn't
+ // store the digest So we store the metadata separately.
+ // Use the ArtifactFileMetadata's digest if no digest was injected, or if the file can't be
+ // digested.
injectedDigest = injectedDigest != null || !isFile ? injectedDigest : data.getDigest();
FileArtifactValue value = FileArtifactValue.create(artifact, artifactPathResolver, data,
injectedDigest);
@@ -346,14 +352,14 @@
for (TreeFileArtifact treeFileArtifact : contents) {
FileArtifactValue cachedValue = additionalOutputData.get(treeFileArtifact);
if (cachedValue == null) {
- FileValue fileValue = outputArtifactData.get(treeFileArtifact);
+ ArtifactFileMetadata fileMetadata = outputArtifactData.get(treeFileArtifact);
// This is similar to what's present in getRealMetadataForArtifact, except
- // we get back the FileValue, not the metadata.
+ // we get back the ArtifactFileMetadata, not the metadata.
// We do not cache exceptions besides nonexistence here, because it is unlikely that the
// file will be requested from this cache too many times.
- if (fileValue == null) {
+ if (fileMetadata == null) {
try {
- fileValue = constructFileValue(treeFileArtifact, /*statNoFollow=*/ null);
+ fileMetadata = constructArtifactFileMetadata(treeFileArtifact, /*statNoFollow=*/ null);
} catch (FileNotFoundException e) {
String errorMessage = String.format(
"Failed to resolve relative path %s inside TreeArtifact %s. "
@@ -366,7 +372,7 @@
// A minor hack: maybeStoreAdditionalData will force the data to be stored
// in additionalOutputData.
- maybeStoreAdditionalData(treeFileArtifact, fileValue, null);
+ maybeStoreAdditionalData(treeFileArtifact, fileMetadata, null);
cachedValue = Preconditions.checkNotNull(
additionalOutputData.get(treeFileArtifact), treeFileArtifact);
}
@@ -418,24 +424,36 @@
// Assumption: any non-Artifact output is 'virtual' and should be ignored here.
if (output instanceof Artifact) {
final Artifact artifact = (Artifact) output;
- // We have to add the artifact to injectedFiles before calling constructFileValue to avoid
+ // We have to add the artifact to injectedFiles before calling constructArtifactFileMetadata
+ // to avoid
// duplicate chmod calls.
injectedFiles.add(artifact);
- FileValue fileValue;
+ ArtifactFileMetadata fileMetadata;
try {
// This call may do an unnecessary call to Path#getFastDigest to see if the digest is
// readily available. We cannot pass the digest in, though, because if it is not available
- // from the filesystem, this FileValue will not compare equal to another one created for the
+ // from the filesystem, this ArtifactFileMetadata will not compare equal to another one
+ // created for the
// same file, because the other one will be missing its digest.
- fileValue = constructFileValue(artifact, FileStatusWithDigestAdapter.adapt(statNoFollow));
+ fileMetadata =
+ constructArtifactFileMetadata(
+ artifact, FileStatusWithDigestAdapter.adapt(statNoFollow));
// Ensure the digest supplied matches the actual digest if it exists.
- byte[] fileDigest = fileValue.getDigest();
+ byte[] fileDigest = fileMetadata.getDigest();
if (fileDigest != null && !Arrays.equals(digest, fileDigest)) {
BaseEncoding base16 = BaseEncoding.base16();
String digestString = (digest != null) ? base16.encode(digest) : "null";
String fileDigestString = base16.encode(fileDigest);
- throw new IllegalStateException("Expected digest " + digestString + " for artifact "
- + artifact + ", but got " + fileDigestString + " (" + fileValue + ")");
+ throw new IllegalStateException(
+ "Expected digest "
+ + digestString
+ + " for artifact "
+ + artifact
+ + ", but got "
+ + fileDigestString
+ + " ("
+ + fileMetadata
+ + ")");
}
} catch (IOException e) {
// Do nothing - we just failed to inject metadata. Real error handling will be done later,
@@ -446,16 +464,12 @@
// the filesystem does not support fast digests. Since we usually only inject digests when
// running with a filesystem that supports fast digests, this is fairly unlikely.
try {
- maybeStoreAdditionalData(artifact, fileValue, digest);
+ maybeStoreAdditionalData(artifact, fileMetadata, digest);
} catch (IOException e) {
- if (fileValue.getSize() != 0) {
- // Empty files currently have their mtimes examined, and so could throw. No other files
- // should throw, since all filesystem access has already been done.
- throw new IllegalStateException(
- "Filesystem should not have been accessed while injecting data for "
- + artifact.prettyPrint(), e);
- }
- // Ignore exceptions for empty files, as above.
+ throw new IllegalStateException(
+ "Filesystem should not have been accessed while injecting data for "
+ + artifact.prettyPrint(),
+ e);
}
}
}
@@ -486,7 +500,7 @@
// While `artifactValue` carries the important information, the control flow of `getMetadata`
// requires an entry in `outputArtifactData` to access `additionalOutputData`, so a
// `PLACEHOLDER` is added to `outputArtifactData`.
- outputArtifactData.put(output, FileValue.PLACEHOLDER);
+ outputArtifactData.put(output, ArtifactFileMetadata.PLACEHOLDER);
additionalOutputData.put(output, artifactValue);
}
@@ -521,7 +535,7 @@
}
/** @return data for output files that was computed during execution. */
- Map<Artifact, FileValue> getOutputArtifactData() {
+ Map<Artifact, ArtifactFileMetadata> getOutputArtifactData() {
return outputArtifactData;
}
@@ -537,28 +551,43 @@
* Returns data for any output files whose metadata was not computable from the corresponding
* entry in {@link #getOutputArtifactData}.
*
- * <p>There are three reasons why we might not be able to compute metadata for an artifact from
- * the FileValue. First, middleman artifacts have no corresponding FileValues. Second, if
- * computing a file's digest is not fast, the FileValue does not do so, so a file on a filesystem
- * without fast digests has to have its metadata stored separately. Third, some files' metadata
- * (directories, empty files) contain their mtimes, which the FileValue does not expose, so that
- * has to be stored separately.
+ * <p>There are two bits to consider: the filesystem possessing fast digests and the execution
+ * service injecting metadata via {@link #injectRemoteFile} or {@link #injectDigest}.
*
- * <p>Note that for files that need digests, we can't easily inject the digest in the FileValue
- * because it would complicate equality-checking on subsequent builds -- if our filesystem doesn't
- * do fast digests, the comparison value would not have a digest.
+ * <ol>
+ * <li>If the filesystem does not possess fast digests, then we will have additional output data
+ * for practically every artifact, since we will need to store their digests.
+ * <li>If we have a remote execution service injecting metadata, then we will just store that
+ * metadata here, and put {@link ArtifactFileMetadata#PLACEHOLDER} objects into {@link
+ * #outputArtifactData} if the filesystem supports fast digests, and the actual metadata if
+ * the filesystem does not support fast digests.
+ * <li>If the filesystem has fast digests <i>but</i> there is no remote execution injecting
+ * metadata, then we will not store additional metadata here.
+ * </ol>
+ *
+ * <p>Note that this means that in the vastly common cases (Google-internal, where we have fast
+ * digests and remote execution, and Bazel, where there is often neither), this map is always
+ * populated. Locally executed actions are the exception to this rule inside Google.
+ *
+ * <p>Moreover, there are some artifacts that are always stored here. First, middleman artifacts
+ * have no corresponding {@link ArtifactFileMetadata}. Second, directories' metadata contain their
+ * mtimes, which the {@link ArtifactFileMetadata} does not expose, so that has to be stored
+ * separately.
+ *
+ * <p>Note that for files that need digests, we can't easily inject the digest in the {@link
+ * ArtifactFileMetadata} because it would complicate equality-checking on subsequent builds -- if
+ * our filesystem doesn't do fast digests, the comparison value would not have a digest.
*/
Map<Artifact, FileArtifactValue> getAdditionalOutputData() {
return additionalOutputData;
}
/**
- * Constructs a new FileValue, saves it, and checks inconsistent data. This calls chmod on the
- * file if we're in executionMode.
+ * Constructs a new {@link ArtifactFileMetadata}, saves it, and checks inconsistent data. This
+ * calls chmod on the file if we're in executionMode.
*/
- private FileValue constructFileValue(
- Artifact artifact, @Nullable FileStatusWithDigest statNoFollow)
- throws IOException {
+ private ArtifactFileMetadata constructArtifactFileMetadata(
+ Artifact artifact, @Nullable FileStatusWithDigest statNoFollow) throws IOException {
// We first chmod the output files before we construct the FileContentsProxy. The proxy may use
// ctime, which is affected by chmod.
if (executionMode.get()) {
@@ -566,24 +595,30 @@
setPathReadOnlyAndExecutable(artifact);
}
- FileValue value = fileValueFromArtifact(artifact, artifactPathResolver, statNoFollow,
- getTimestampGranularityMonitor(artifact));
+ ArtifactFileMetadata value =
+ fileMetadataFromArtifact(
+ artifact, artifactPathResolver, statNoFollow, getTimestampGranularityMonitor(artifact));
outputArtifactData.put(artifact, value);
return value;
}
@VisibleForTesting
- static FileValue fileValueFromArtifact(Artifact artifact,
- @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm)
+ static ArtifactFileMetadata fileMetadataFromArtifact(
+ Artifact artifact,
+ @Nullable FileStatusWithDigest statNoFollow,
+ @Nullable TimestampGranularityMonitor tsgm)
throws IOException {
- return fileValueFromArtifact(artifact, ArtifactPathResolver.IDENTITY, statNoFollow, tsgm);
+ return fileMetadataFromArtifact(artifact, ArtifactPathResolver.IDENTITY, statNoFollow, tsgm);
}
- private static FileValue fileValueFromArtifact(Artifact artifact,
+ private static ArtifactFileMetadata fileMetadataFromArtifact(
+ Artifact artifact,
ArtifactPathResolver artifactPathResolver,
- @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm)
+ @Nullable FileStatusWithDigest statNoFollow,
+ @Nullable TimestampGranularityMonitor tsgm)
throws IOException {
Path path = artifactPathResolver.toPath(artifact);
+ PathFragment pathFragment = path.asFragment();
RootedPath rootedPath =
RootedPath.toRootedPath(
artifactPathResolver.transformRoot(artifact.getRoot().getRoot()),
@@ -591,8 +626,11 @@
if (statNoFollow == null) {
statNoFollow = FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW));
if (statNoFollow == null) {
- return FileValue.value(rootedPath, FileStateValue.NONEXISTENT_FILE_STATE_NODE,
- rootedPath, FileStateValue.NONEXISTENT_FILE_STATE_NODE);
+ return ArtifactFileMetadata.value(
+ pathFragment,
+ FileStateValue.NONEXISTENT_FILE_STATE_NODE,
+ pathFragment,
+ FileStateValue.NONEXISTENT_FILE_STATE_NODE);
}
}
Path realPath = path;
@@ -600,7 +638,8 @@
// done by the latter.
if (statNoFollow.isSymbolicLink()) {
realPath = path.resolveSymbolicLinks();
- // We need to protect against symlink cycles since FileValue#value assumes it's dealing with a
+ // We need to protect against symlink cycles since ArtifactFileMetadata#value assumes it's
+ // dealing with a
// file that's not in a symlink cycle.
if (realPath.equals(path)) {
throw new IOException("symlink cycle");
@@ -617,7 +656,8 @@
FileStateValue realFileStateValue = realPath.equals(path)
? fileStateValue
: FileStateValue.create(realRootedPath, tsgm);
- return FileValue.value(rootedPath, fileStateValue, realRootedPath, realFileStateValue);
+ return ArtifactFileMetadata.value(
+ pathFragment, fileStateValue, realPath.asFragment(), realFileStateValue);
}
private void setPathReadOnlyAndExecutable(Artifact artifact) throws IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index 51ba6f0..fd69118 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -271,7 +272,7 @@
// Middleman artifacts have no corresponding files, so their ArtifactValues should have already
// been constructed during execution of the action.
Preconditions.checkState(!artifact.isMiddlemanArtifact(), artifact);
- FileValue data =
+ ArtifactFileMetadata data =
Preconditions.checkNotNull(actionValue.getData(artifact), "%s %s", artifact, actionValue);
Preconditions.checkNotNull(
data.getDigest(), "Digest should already have been calculated for %s (%s)", artifact, data);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
index 31fa632..9efc2b6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesystemValueChecker.java
@@ -25,7 +25,7 @@
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.FileValue;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.concurrent.Sharder;
import com.google.devtools.build.lib.concurrent.ThrowableRecordingRunnableWrapper;
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.skyframe.Differencer;
import com.google.devtools.build.skyframe.FunctionHermeticity;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -302,9 +303,10 @@
Pair<SkyKey, ActionExecutionValue> keyAndValue = fileToKeyAndValue.get(artifact);
ActionExecutionValue actionValue = keyAndValue.getSecond();
SkyKey key = keyAndValue.getFirst();
- FileValue lastKnownData = actionValue.getAllFileValues().get(artifact);
+ ArtifactFileMetadata lastKnownData = actionValue.getAllFileValues().get(artifact);
try {
- FileValue newData = ActionMetadataHandler.fileValueFromArtifact(artifact, stat, tsgm);
+ ArtifactFileMetadata newData =
+ ActionMetadataHandler.fileMetadataFromArtifact(artifact, stat, tsgm);
if (!newData.equals(lastKnownData)) {
updateIntraBuildModifiedCounter(
stat != null ? stat.getLastChangeTime() : -1,
@@ -407,17 +409,19 @@
ImmutableSet<PathFragment> knownModifiedOutputFiles,
Supplier<NavigableSet<PathFragment>> sortedKnownModifiedOutputFiles) {
boolean isDirty = false;
- for (Map.Entry<Artifact, FileValue> entry : actionValue.getAllFileValues().entrySet()) {
+ for (Map.Entry<Artifact, ArtifactFileMetadata> entry :
+ actionValue.getAllFileValues().entrySet()) {
Artifact file = entry.getKey();
- FileValue lastKnownData = entry.getValue();
+ ArtifactFileMetadata lastKnownData = entry.getValue();
if (shouldCheckFile(knownModifiedOutputFiles, file)) {
try {
- FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(file, null,
- tsgm);
- if (!fileValue.equals(lastKnownData)) {
- updateIntraBuildModifiedCounter(fileValue.exists()
- ? fileValue.realRootedPath().asPath().getLastModifiedTime()
- : -1, lastKnownData.isSymlink(), fileValue.isSymlink());
+ ArtifactFileMetadata fileMetadata =
+ ActionMetadataHandler.fileMetadataFromArtifact(file, null, tsgm);
+ if (!fileMetadata.equals(lastKnownData)) {
+ updateIntraBuildModifiedCounter(
+ fileMetadata.exists() ? file.getPath().getLastModifiedTime(Symlinks.FOLLOW) : -1,
+ lastKnownData.isSymlink(),
+ fileMetadata.isSymlink());
modifiedOutputFilesCounter.getAndIncrement();
isDirty = true;
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index 6f8a519..4ddc79f 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -31,12 +31,12 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.actions.FileArtifactValue;
-import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -401,7 +401,7 @@
private static class SimpleActionExecutionFunction implements SkyFunction {
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
- Map<Artifact, FileValue> artifactData = new HashMap<>();
+ Map<Artifact, ArtifactFileMetadata> artifactData = new HashMap<>();
Map<Artifact, TreeArtifactValue> treeArtifactData = new HashMap<>();
Map<Artifact, FileArtifactValue> additionalOutputData = new HashMap<>();
ActionLookupData actionLookupData = (ActionLookupData) skyKey.argument();
@@ -421,7 +421,8 @@
treeFileArtifact2, FileArtifactValue.create(treeFileArtifact2)));
treeArtifactData.put(output, treeArtifactValue);
} else if (action.getActionType() == MiddlemanType.NORMAL) {
- FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(output, null, null);
+ ArtifactFileMetadata fileValue =
+ ActionMetadataHandler.fileMetadataFromArtifact(output, null, null);
artifactData.put(output, fileValue);
additionalOutputData.put(output, FileArtifactValue.create(output, fileValue));
} else {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 4e47c00..55cece8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -733,14 +734,14 @@
// Presently these appear to be untested.
private ActionExecutionValue actionValue(Action action, boolean forceDigest) {
- Map<Artifact, FileValue> artifactData = new HashMap<>();
+ Map<Artifact, ArtifactFileMetadata> artifactData = new HashMap<>();
for (Artifact output : action.getOutputs()) {
try {
Path path = output.getPath();
FileStatusWithDigest stat =
forceDigest ? statWithDigest(path, path.statIfFound(Symlinks.NOFOLLOW)) : null;
- artifactData.put(output,
- ActionMetadataHandler.fileValueFromArtifact(output, stat, null));
+ artifactData.put(
+ output, ActionMetadataHandler.fileMetadataFromArtifact(output, stat, null));
} catch (IOException e) {
throw new IllegalStateException(e);
}
@@ -759,7 +760,7 @@
(ImmutableMap.<TreeFileArtifact, FileArtifactValue>of());
return ActionExecutionValue.create(
- ImmutableMap.<Artifact, FileValue>of(),
+ ImmutableMap.of(),
ImmutableMap.of(emptyDir, emptyValue),
ImmutableMap.<Artifact, FileArtifactValue>of(),
/*outputSymlinks=*/ null,
@@ -768,7 +769,7 @@
}
private ActionExecutionValue actionValueWithTreeArtifacts(List<TreeFileArtifact> contents) {
- Map<Artifact, FileValue> fileData = new HashMap<>();
+ Map<Artifact, ArtifactFileMetadata> fileData = new HashMap<>();
Map<Artifact, Map<TreeFileArtifact, FileArtifactValue>> directoryData = new HashMap<>();
for (TreeFileArtifact output : contents) {
@@ -779,7 +780,8 @@
dirDatum = new HashMap<>();
directoryData.put(output.getParent(), dirDatum);
}
- FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(output, null, null);
+ ArtifactFileMetadata fileValue =
+ ActionMetadataHandler.fileMetadataFromArtifact(output, null, null);
dirDatum.put(output, FileArtifactValue.create(output, fileValue));
fileData.put(output, fileValue);
} catch (IOException e) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
index 3bb8353..992894a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -32,11 +32,11 @@
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.ArtifactFileMetadata;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.actions.FileArtifactValue;
-import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
@@ -251,7 +251,7 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- Map<Artifact, FileValue> fileData = new HashMap<>();
+ Map<Artifact, ArtifactFileMetadata> fileData = new HashMap<>();
Map<TreeFileArtifact, FileArtifactValue> treeArtifactData = new HashMap<>();
ActionLookupData actionLookupData = (ActionLookupData) skyKey.argument();
ActionLookupValue actionLookupValue =
@@ -261,8 +261,8 @@
for (PathFragment subpath : testTreeArtifactContents) {
try {
TreeFileArtifact suboutput = ActionInputHelper.treeFileArtifact(output, subpath);
- FileValue fileValue = ActionMetadataHandler.fileValueFromArtifact(
- suboutput, null, null);
+ ArtifactFileMetadata fileValue =
+ ActionMetadataHandler.fileMetadataFromArtifact(suboutput, null, null);
fileData.put(suboutput, fileValue);
treeArtifactData.put(suboutput, FileArtifactValue.create(suboutput, fileValue));
} catch (IOException e) {
