commit 9ad28cd6001feb5dd8afda5878ebacfe25efe254
author Rumou Duan <rduan@google.com> Wed Oct 19 19:28:06 2016 +0000
committer Philipp Wollermann <philwo@google.com> Wed Oct 19 19:44:52 2016 +0000
tree 7fd858ca933ef88504a5d4ded21bde7e655fa531
parent ca9a425a4825b5fd9892ec55051e7ffb2025bea2

Proper action output checks for TreeArtifacts. Instead of crashing Bazel, we now handle failed TreeArtifact output checks gracefully.

--
MOS_MIGRATED_REVID=136627086

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index e37be07..b523c92 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -180,29 +180,18 @@
     }
   };
 
-  /**
-   * Exception used when the contents of a directory do not form a valid SetArtifact.
-   * (We cannot use IOException because ActionMetadataHandler, in some code paths,
-   * interprets IOExceptions as missing files.)
-   */
-  static class TreeArtifactException extends Exception {
-    TreeArtifactException(String message) {
-      super(message);
-    }
-  }
-
-  private static void explodeDirectory(Artifact rootArtifact,
+  private static void explodeDirectory(Artifact treeArtifact,
       PathFragment pathToExplode, ImmutableSet.Builder<PathFragment> valuesBuilder)
-      throws IOException, TreeArtifactException {
-    for (Path subpath : rootArtifact.getPath().getRelative(pathToExplode).getDirectoryEntries()) {
+      throws IOException {
+    for (Path subpath : treeArtifact.getPath().getRelative(pathToExplode).getDirectoryEntries()) {
       PathFragment canonicalSubpathFragment =
           pathToExplode.getChild(subpath.getBaseName()).normalize();
       if (subpath.isDirectory()) {
-        explodeDirectory(rootArtifact,
+        explodeDirectory(treeArtifact,
             pathToExplode.getChild(subpath.getBaseName()), valuesBuilder);
       } else if (subpath.isSymbolicLink()) {
-        throw new TreeArtifactException(
-            "A SetArtifact may not contain a symlink, found " + subpath);
+        throw new IOException(
+            "A TreeArtifact may not contain a symlink, found " + subpath);
       } else if (subpath.isFile()) {
         valuesBuilder.add(canonicalSubpathFragment);
       } else {
@@ -215,13 +204,12 @@
   /**
    * Recursively get all child files in a directory
    * (excluding child directories themselves, but including all files in them).
-   * @throws IOException if one was thrown reading directory contents from disk.
-   * @throws TreeArtifactException if the on-disk directory is not a valid TreeArtifact.
+   * @throws IOException if there is any problem reading or validating outputs under the given
+   *     tree artifact.
    */
-  static Set<PathFragment> explodeDirectory(Artifact rootArtifact)
-      throws IOException, TreeArtifactException {
+  static Set<PathFragment> explodeDirectory(Artifact treeArtifact) throws IOException {
     ImmutableSet.Builder<PathFragment> explodedDirectory = ImmutableSet.builder();
-    explodeDirectory(rootArtifact, PathFragment.EMPTY_FRAGMENT, explodedDirectory);
+    explodeDirectory(treeArtifact, PathFragment.EMPTY_FRAGMENT, explodedDirectory);
     return explodedDirectory.build();
   }
 }