Introduce TreeFileArtifact, which represents files under TreeArtifacts.

Remove ArtifactFile, which is rendered obsolete by TreeFileArtifact.

--
MOS_MIGRATED_REVID=119789154

1 files changed, 71 insertions, 71 deletions

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 cf8f6680e7..28f7d0c731 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.common.io.BaseEncoding;
 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 @@ public class ActionMetadataHandler implements MetadataHandler {
    */
   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 @@ public class ActionMetadataHandler implements MetadataHandler {
    * 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 @@ public class ActionMetadataHandler implements MetadataHandler {
    * 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 @@ public class ActionMetadataHandler implements MetadataHandler {
       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 @@ public class ActionMetadataHandler implements MetadataHandler {
   }
 
   /**
-   * 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 @@ public class ActionMetadataHandler implements MetadataHandler {
    * 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 @@ public class ActionMetadataHandler implements MetadataHandler {
       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 @@ public class ActionMetadataHandler implements MetadataHandler {
         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 @@ public class ActionMetadataHandler implements MetadataHandler {
       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 @@ public class ActionMetadataHandler implements MetadataHandler {
       } 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 @@ public class ActionMetadataHandler implements MetadataHandler {
               + " 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 @@ public class ActionMetadataHandler implements MetadataHandler {
     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 @@ public class ActionMetadataHandler implements MetadataHandler {
     // 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 @@ public class ActionMetadataHandler implements MetadataHandler {
           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 @@ public class ActionMetadataHandler implements MetadataHandler {
       // 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 @@ public class ActionMetadataHandler implements MetadataHandler {
         "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 @@ public class ActionMetadataHandler implements MetadataHandler {
   }
 
   @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 @@ public class ActionMetadataHandler implements MetadataHandler {
 
   /**
    * 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 @@ public class ActionMetadataHandler implements MetadataHandler {
   }
 
   /** 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 @@ public class ActionMetadataHandler implements MetadataHandler {
       }
     }
     RootedPath realRootedPath = RootedPath.toRootedPathMaybeUnderRoot(realPath,
-        ImmutableList.of(file.getRoot().getPath()));
+        ImmutableList.of(artifact.getRoot().getPath()));
     FileStateValue fileStateValue;
     FileStateValue realFileStateValue;
     try {