Introduce TreeFileArtifact, which represents files under TreeArtifacts.
Remove ArtifactFile, which is rendered obsolete by TreeFileArtifact.
--
MOS_MIGRATED_REVID=119789154
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 cf8f668..28f7d0c 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
@@ -22,7 +22,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactFile;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.cache.Digest;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.Metadata;
@@ -79,8 +79,8 @@
*/
private final Map<Artifact, FileArtifactValue> inputArtifactData;
- /** FileValues for each output ArtifactFile. */
- private final ConcurrentMap<ArtifactFile, FileValue> outputArtifactFileData =
+ /** FileValues for each output Artifact. */
+ private final ConcurrentMap<Artifact, FileValue> outputArtifactData =
new ConcurrentHashMap<>();
/**
@@ -89,7 +89,7 @@
* If the value is null, this means nothing was injected, and the output TreeArtifact
* is to have its values read from disk instead.
*/
- private final ConcurrentMap<Artifact, Set<ArtifactFile>> outputDirectoryListings =
+ private final ConcurrentMap<Artifact, Set<TreeFileArtifact>> outputDirectoryListings =
new ConcurrentHashMap<>();
/** Outputs that are to be omitted. */
@@ -109,18 +109,18 @@
* Unlike additionalOutputData, this map is discarded (the relevant FileArtifactValues
* are stored in outputTreeArtifactData's values instead).
*/
- private final ConcurrentMap<ArtifactFile, FileArtifactValue> cachedTreeArtifactFileData =
+ private final ConcurrentMap<TreeFileArtifact, FileArtifactValue> cachedTreeFileArtifactData =
new ConcurrentHashMap<>();
/**
- * Data for TreeArtifactValues, constructed from outputArtifactFileData and
+ * Data for TreeArtifactValues, constructed from outputArtifactData and
* additionalOutputFileData.
*/
private final ConcurrentMap<Artifact, TreeArtifactValue> outputTreeArtifactData =
new ConcurrentHashMap<>();
- /** Tracks which ArtifactFiles have had metadata injected. */
- private final Set<ArtifactFile> injectedFiles = Sets.newConcurrentHashSet();
+ /** Tracks which Artifacts have had metadata injected. */
+ private final Set<Artifact> injectedFiles = Sets.newConcurrentHashSet();
private final ImmutableSet<Artifact> outputs;
private final TimestampGranularityMonitor tsgm;
@@ -214,7 +214,7 @@
throw new FileNotFoundException(artifact + " not found");
}
// It's an ordinary artifact.
- FileValue fileValue = outputArtifactFileData.get(artifact);
+ FileValue fileValue = outputArtifactData.get(artifact);
if (fileValue != null) {
// Non-middleman artifacts should only have additionalOutputData if they have
// outputArtifactData. We don't assert this because of concurrency possibilities, but at least
@@ -237,16 +237,16 @@
}
/**
- * Check that the new {@code data} we just calculated for an {@link ArtifactFile} agrees with the
+ * Check that the new {@code data} we just calculated for an {@link Artifact} agrees with the
* {@code oldData} (presumably calculated concurrently), if it was present.
*/
// Not private only because used by SkyframeActionExecutor's metadata handler.
- static void checkInconsistentData(ArtifactFile file,
+ static void checkInconsistentData(Artifact artifact,
@Nullable Object oldData, Object data) throws IOException {
if (oldData != null && !oldData.equals(data)) {
// Another thread checked this file since we looked at the map, and got a different answer
// than we did. Presumably the user modified the file between reads.
- throw new IOException("Data for " + file.prettyPrint() + " changed to " + data
+ throw new IOException("Data for " + artifact.prettyPrint() + " changed to " + data
+ " after it was calculated as " + oldData);
}
}
@@ -256,13 +256,13 @@
* for normal (non-middleman) artifacts.
*/
@Nullable
- private Metadata maybeStoreAdditionalData(ArtifactFile file, FileValue data,
+ private Metadata maybeStoreAdditionalData(Artifact artifact, FileValue data,
@Nullable byte[] injectedDigest) throws IOException {
if (!data.exists()) {
// Nonexistent files should only occur before executing an action.
- throw new FileNotFoundException(file.prettyPrint() + " does not exist");
+ throw new FileNotFoundException(artifact.prettyPrint() + " does not exist");
}
- if (file instanceof Artifact) {
+ if (!artifact.hasParent()) {
// Artifacts may use either the "real" digest or the mtime, if the file is size 0.
boolean isFile = data.isFile();
boolean useDigest = DigestUtils.useFileDigest(isFile, isFile ? data.getSize() : 0);
@@ -279,17 +279,17 @@
injectedDigest = injectedDigest != null || !isFile ? injectedDigest : data.getDigest();
FileArtifactValue value =
FileArtifactValue.create(
- (Artifact) file, isFile, isFile ? data.getSize() : 0, injectedDigest);
- FileArtifactValue oldValue = additionalOutputData.putIfAbsent((Artifact) file, value);
- checkInconsistentData(file, oldValue, value);
+ (Artifact) artifact, isFile, isFile ? data.getSize() : 0, injectedDigest);
+ FileArtifactValue oldValue = additionalOutputData.putIfAbsent((Artifact) artifact, value);
+ checkInconsistentData(artifact, oldValue, value);
return metadataFromValue(value);
} else {
- // Non-Artifact ArtifactFiles are always "real" files, and always use the real digest.
- // When null, createWithDigest() will pull the digest from the filesystem.
+ // We are dealing with artifacts inside a tree artifact.
FileArtifactValue value =
- FileArtifactValue.createWithDigest(file.getPath(), injectedDigest, data.getSize());
- FileArtifactValue oldValue = cachedTreeArtifactFileData.putIfAbsent(file, value);
- checkInconsistentData(file, oldValue, value);
+ FileArtifactValue.createWithDigest(artifact.getPath(), injectedDigest, data.getSize());
+ FileArtifactValue oldValue = cachedTreeFileArtifactData.putIfAbsent(
+ (TreeFileArtifact) artifact, value);
+ checkInconsistentData(artifact, oldValue, value);
return new Metadata(value.getDigest());
}
}
@@ -302,13 +302,13 @@
FileArtifactValue.createProxy(digest.asMetadata().digest));
}
- private Set<ArtifactFile> getTreeArtifactContents(Artifact artifact) {
+ private Set<TreeFileArtifact> getTreeArtifactContents(Artifact artifact) {
Preconditions.checkArgument(artifact.isTreeArtifact(), artifact);
- Set<ArtifactFile> contents = outputDirectoryListings.get(artifact);
+ Set<TreeFileArtifact> contents = outputDirectoryListings.get(artifact);
if (contents == null) {
// Unfortunately, there is no such thing as a ConcurrentHashSet.
- contents = Collections.newSetFromMap(new ConcurrentHashMap<ArtifactFile, Boolean>());
- Set<ArtifactFile> oldContents = outputDirectoryListings.putIfAbsent(artifact, contents);
+ contents = Collections.newSetFromMap(new ConcurrentHashMap<TreeFileArtifact, Boolean>());
+ Set<TreeFileArtifact> oldContents = outputDirectoryListings.putIfAbsent(artifact, contents);
// Avoid a race condition.
if (oldContents != null) {
contents = oldContents;
@@ -323,7 +323,7 @@
return value;
}
- Set<ArtifactFile> registeredContents = outputDirectoryListings.get(artifact);
+ Set<TreeFileArtifact> registeredContents = outputDirectoryListings.get(artifact);
if (registeredContents != null) {
// Check that our registered outputs matches on-disk outputs. Only perform this check
// when contents were explicitly registered.
@@ -338,10 +338,10 @@
} catch (TreeArtifactException e) {
throw new IllegalStateException(e);
}
- Set<ArtifactFile> diskFiles = ActionInputHelper.asArtifactFiles(artifact, paths);
+ Set<TreeFileArtifact> diskFiles = ActionInputHelper.asTreeFileArtifacts(artifact, paths);
if (!diskFiles.equals(registeredContents)) {
// There might be more than one error here. We first look for missing output files.
- Set<ArtifactFile> missingFiles = Sets.difference(registeredContents, diskFiles);
+ Set<TreeFileArtifact> missingFiles = Sets.difference(registeredContents, diskFiles);
if (!missingFiles.isEmpty()) {
// Don't throw IOException--getMetadataMaybe() eats them.
// TODO(bazel-team): Report this error in a better way when called by checkOutputs()
@@ -352,7 +352,7 @@
+ " was registered, but not present on disk");
}
- Set<ArtifactFile> extraFiles = Sets.difference(diskFiles, registeredContents);
+ Set<TreeFileArtifact> extraFiles = Sets.difference(diskFiles, registeredContents);
// extraFiles cannot be empty
throw new IllegalStateException(
"File " + extraFiles.iterator().next().getParentRelativePath()
@@ -369,28 +369,30 @@
return value;
}
- private TreeArtifactValue constructTreeArtifactValue(Collection<ArtifactFile> contents)
+ private TreeArtifactValue constructTreeArtifactValue(Collection<TreeFileArtifact> contents)
throws IOException {
- Map<PathFragment, FileArtifactValue> values = Maps.newHashMapWithExpectedSize(contents.size());
+ Map<TreeFileArtifact, FileArtifactValue> values =
+ Maps.newHashMapWithExpectedSize(contents.size());
- for (ArtifactFile file : contents) {
- FileArtifactValue cachedValue = cachedTreeArtifactFileData.get(file);
+ for (TreeFileArtifact treeFileArtifact : contents) {
+ FileArtifactValue cachedValue = cachedTreeFileArtifactData.get(treeFileArtifact);
if (cachedValue == null) {
- FileValue fileValue = outputArtifactFileData.get(file);
- // This is similar to what's present in getRealMetadataForArtifactFile, except
+ FileValue fileValue = outputArtifactData.get(treeFileArtifact);
+ // This is similar to what's present in getRealMetadataForArtifact, except
// we get back the FileValue, 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) {
- fileValue = constructFileValue(file, /*statNoFollow=*/ null);
+ fileValue = constructFileValue(treeFileArtifact, /*statNoFollow=*/ null);
// A minor hack: maybeStoreAdditionalData will force the data to be stored
- // in cachedTreeArtifactFileData.
- maybeStoreAdditionalData(file, fileValue, null);
+ // in cachedTreeFileArtifactData.
+ maybeStoreAdditionalData(treeFileArtifact, fileValue, null);
}
- cachedValue = Preconditions.checkNotNull(cachedTreeArtifactFileData.get(file), file);
+ cachedValue = Preconditions.checkNotNull(
+ cachedTreeFileArtifactData.get(treeFileArtifact), treeFileArtifact);
}
- values.put(file.getParentRelativePath(), cachedValue);
+ values.put(treeFileArtifact, cachedValue);
}
return TreeArtifactValue.create(values);
@@ -414,34 +416,32 @@
// something has gone terribly wrong.
Object previousDirectoryListing =
outputDirectoryListings.put(artifact,
- Collections.newSetFromMap(new ConcurrentHashMap<ArtifactFile, Boolean>()));
+ Collections.newSetFromMap(new ConcurrentHashMap<TreeFileArtifact, Boolean>()));
Preconditions.checkState(previousDirectoryListing == null,
"Race condition while constructing TreArtifactValue: %s, %s",
artifact, previousDirectoryListing);
- return constructTreeArtifactValue(ActionInputHelper.asArtifactFiles(artifact, paths));
+ return constructTreeArtifactValue(ActionInputHelper.asTreeFileArtifacts(artifact, paths));
}
@Override
- public void addExpandedTreeOutput(ArtifactFile output) {
- Preconditions.checkArgument(output.getParent().isTreeArtifact(),
- "Expanded set output must belong to a TreeArtifact");
- Set<ArtifactFile> values = getTreeArtifactContents(output.getParent());
+ public void addExpandedTreeOutput(TreeFileArtifact output) {
+ Set<TreeFileArtifact> values = getTreeArtifactContents(output.getParent());
values.add(output);
}
@Override
public void injectDigest(ActionInput output, FileStatus statNoFollow, byte[] digest) {
- // Assumption: any non-ArtifactFile output is 'virtual' and should be ignored here.
- if (output instanceof ArtifactFile) {
- final ArtifactFile file = (ArtifactFile) output;
- Preconditions.checkState(injectedFiles.add(file), file);
+ // Assumption: any non-Artifact output is 'virtual' and should be ignored here.
+ if (output instanceof Artifact) {
+ final Artifact artifact = (Artifact) output;
+ Preconditions.checkState(injectedFiles.add(artifact), artifact);
FileValue fileValue;
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
// same file, because the other one will be missing its digest.
- fileValue = fileValueFromArtifactFile(file,
+ fileValue = fileValueFromArtifact(artifact,
FileStatusWithDigestAdapter.adapt(statNoFollow), tsgm);
// Ensure the digest supplied matches the actual digest if it exists.
byte[] fileDigest = fileValue.getDigest();
@@ -450,9 +450,9 @@
String digestString = (digest != null) ? base16.encode(digest) : "null";
String fileDigestString = base16.encode(fileDigest);
throw new IllegalStateException("Expected digest " + digestString + " for artifact "
- + file + ", but got " + fileDigestString + " (" + fileValue + ")");
+ + artifact + ", but got " + fileDigestString + " (" + fileValue + ")");
}
- outputArtifactFileData.put(file, fileValue);
+ outputArtifactData.put(artifact, fileValue);
} catch (IOException e) {
// Do nothing - we just failed to inject metadata. Real error handling will be done later,
// when somebody will try to access that file.
@@ -462,14 +462,14 @@
// 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(file, fileValue, digest);
+ maybeStoreAdditionalData(artifact, fileValue, 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 "
- + file.prettyPrint(), e);
+ + artifact.prettyPrint(), e);
}
// Ignore exceptions for empty files, as above.
}
@@ -496,11 +496,11 @@
"Files cannot be injected before action execution: %s", injectedFiles);
Preconditions.checkState(omittedOutputs.isEmpty(),
"Artifacts cannot be marked omitted before action execution: %s", omittedOutputs);
- outputArtifactFileData.clear();
+ outputArtifactData.clear();
outputDirectoryListings.clear();
outputTreeArtifactData.clear();
additionalOutputData.clear();
- cachedTreeArtifactFileData.clear();
+ cachedTreeFileArtifactData.clear();
}
@Override
@@ -521,13 +521,13 @@
}
@Override
- public boolean isInjected(ArtifactFile file) {
+ public boolean isInjected(Artifact file) {
return injectedFiles.contains(file);
}
/** @return data for output files that was computed during execution. */
- Map<ArtifactFile, FileValue> getOutputArtifactFileData() {
- return outputArtifactFileData;
+ Map<Artifact, FileValue> getOutputArtifactData() {
+ return outputArtifactData;
}
/**
@@ -540,7 +540,7 @@
/**
* Returns data for any output files whose metadata was not computable from the corresponding
- * entry in {@link #getOutputArtifactFileData}.
+ * 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
@@ -558,21 +558,21 @@
}
/** Constructs a new FileValue, saves it, and checks inconsistent data. */
- FileValue constructFileValue(ArtifactFile file, @Nullable FileStatusWithDigest statNoFollow)
+ FileValue constructFileValue(Artifact artifact, @Nullable FileStatusWithDigest statNoFollow)
throws IOException {
- FileValue value = fileValueFromArtifactFile(file, statNoFollow, tsgm);
- FileValue oldFsValue = outputArtifactFileData.putIfAbsent(file, value);
- checkInconsistentData(file, oldFsValue, null);
+ FileValue value = fileValueFromArtifact(artifact, statNoFollow, tsgm);
+ FileValue oldFsValue = outputArtifactData.putIfAbsent(artifact, value);
+ checkInconsistentData(artifact, oldFsValue, null);
return value;
}
@VisibleForTesting
- static FileValue fileValueFromArtifactFile(ArtifactFile file,
+ static FileValue fileValueFromArtifact(Artifact artifact,
@Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm)
throws IOException {
- Path path = file.getPath();
+ Path path = artifact.getPath();
RootedPath rootedPath =
- RootedPath.toRootedPath(file.getRoot().getPath(), file.getRootRelativePath());
+ RootedPath.toRootedPath(artifact.getRoot().getPath(), artifact.getRootRelativePath());
if (statNoFollow == null) {
statNoFollow = FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW));
if (statNoFollow == null) {
@@ -592,7 +592,7 @@
}
}
RootedPath realRootedPath = RootedPath.toRootedPathMaybeUnderRoot(realPath,
- ImmutableList.of(file.getRoot().getPath()));
+ ImmutableList.of(artifact.getRoot().getPath()));
FileStateValue fileStateValue;
FileStateValue realFileStateValue;
try {
