Proper action output checks for TreeArtifacts. Instead of crashing Bazel, we now handle failed TreeArtifact output checks gracefully.
--
MOS_MIGRATED_REVID=136627086
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 a7230a6..9a82af3 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,7 +26,6 @@
import com.google.devtools.build.lib.actions.cache.Md5Digest;
import com.google.devtools.build.lib.actions.cache.Metadata;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
-import com.google.devtools.build.lib.skyframe.TreeArtifactValue.TreeArtifactException;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileStatus;
@@ -307,11 +306,7 @@
// should be single threaded and there should be no race condition.
// The current design of ActionMetadataHandler makes this hard to enforce.
Set<PathFragment> paths = null;
- try {
- paths = TreeArtifactValue.explodeDirectory(artifact);
- } catch (TreeArtifactException e) {
- throw new IllegalStateException(e);
- }
+ paths = TreeArtifactValue.explodeDirectory(artifact);
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.
@@ -322,13 +317,13 @@
// Currently it's hard to report this error without refactoring, since checkOutputs()
// likes to substitute its own error messages upon catching IOException, and falls
// through to unrecoverable error behavior on any other exception.
- throw new IllegalStateException("Output file " + missingFiles.iterator().next()
+ throw new IOException("Output file " + missingFiles.iterator().next()
+ " was registered, but not present on disk");
}
Set<TreeFileArtifact> extraFiles = Sets.difference(diskFiles, registeredContents);
// extraFiles cannot be empty
- throw new IllegalStateException(
+ throw new IOException(
"File " + extraFiles.iterator().next().getParentRelativePath()
+ ", present in TreeArtifact " + artifact + ", was not registered");
}
@@ -381,11 +376,7 @@
}
Set<PathFragment> paths = null;
- try {
- paths = TreeArtifactValue.explodeDirectory(artifact);
- } catch (TreeArtifactException e) {
- throw new IllegalStateException(e);
- }
+ paths = TreeArtifactValue.explodeDirectory(artifact);
// If you're reading tree artifacts from disk while outputDirectoryListings are being injected,
// something has gone terribly wrong.
Object previousDirectoryListing =
@@ -482,12 +473,6 @@
}
@Override
- public boolean artifactExists(Artifact artifact) {
- Preconditions.checkState(!artifactOmitted(artifact), artifact);
- return getMetadataMaybe(artifact) != null;
- }
-
- @Override
public boolean isRegularFile(Artifact artifact) {
// Currently this method is used only for genrule input directory checks. If we need to call
// this on output artifacts too, this could be more efficient.
