Populate DetailedExitCode for BuildFileNotFoundException errors in PackageLookupFunction. Also switch the exit code for some package loading codes to "environmental error" since they come from disk problems/concurrent changes, not source state.
PiperOrigin-RevId: 363720546
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 2136d58..3e49f95 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -1795,11 +1795,13 @@
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
+ "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
],
@@ -2143,6 +2145,7 @@
],
deps = [
":action_execution_value",
+ ":detailed_exceptions",
":directory_listing_value",
":file_symlink_exception",
":file_symlink_infinite_expansion_exception",
@@ -2164,6 +2167,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
"//third_party:jsr305",
],
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index 14c8347..ec22abe 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -31,6 +31,8 @@
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
@@ -183,26 +185,52 @@
throws PackageLookupFunctionException, InterruptedException {
String basename = fileRootedPath.asPath().getBaseName();
SkyKey fileSkyKey = FileValue.key(fileRootedPath);
- FileValue fileValue = null;
+ FileValue fileValue;
try {
fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class);
} catch (InconsistentFilesystemException e) {
// This error is not transient from the perspective of the PackageLookupFunction.
throw new PackageLookupFunctionException(e, Transience.PERSISTENT);
} catch (FileSymlinkException e) {
+ String message =
+ e.getMessage()
+ + " detected while trying to find "
+ + basename
+ + " file "
+ + fileRootedPath.asPath();
throw new PackageLookupFunctionException(
new BuildFileNotFoundException(
packageIdentifier,
- "Symlink cycle detected while trying to find "
- + basename
- + " file "
- + fileRootedPath.asPath(),
- e),
+ message,
+ DetailedExitCode.of(
+ FailureDetails.FailureDetail.newBuilder()
+ .setMessage(message)
+ .setPackageLoading(
+ FailureDetails.PackageLoading.newBuilder()
+ .setCode(
+ FailureDetails.PackageLoading.Code
+ .SYMLINK_CYCLE_OR_INFINITE_EXPANSION))
+ .build())),
Transience.PERSISTENT);
} catch (IOException e) {
- throw new PackageLookupFunctionException(new BuildFileNotFoundException(packageIdentifier,
- "IO errors while looking for " + basename + " file reading "
- + fileRootedPath.asPath() + ": " + e.getMessage(), e),
+ String message =
+ "IO errors while looking for "
+ + basename
+ + " file reading "
+ + fileRootedPath.asPath()
+ + ": "
+ + e.getMessage();
+ throw new PackageLookupFunctionException(
+ new BuildFileNotFoundException(
+ packageIdentifier,
+ message,
+ DetailedExitCode.of(
+ FailureDetails.FailureDetail.newBuilder()
+ .setMessage(message)
+ .setPackageLoading(
+ FailureDetails.PackageLoading.newBuilder()
+ .setCode(FailureDetails.PackageLoading.Code.OTHER_IO_EXCEPTION))
+ .build())),
Transience.PERSISTENT);
}
return fileValue;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
index 8cfda43..457394e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.FileType;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFile;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFileFactory;
@@ -233,14 +234,28 @@
String.format(
"Error while traversing directory %s: %s",
traversal.root.getRelativePart(), e.getMessage());
+ // Trying to stat the starting point of this root may have failed with a symlink cycle or
+ // trying to get a package lookup value may have failed due to a symlink cycle.
+ RecursiveFilesystemTraversalException.Type exceptionType =
+ RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE;
+ if (e instanceof FileSymlinkException) {
+ exceptionType =
+ RecursiveFilesystemTraversalException.Type.SYMLINK_CYCLE_OR_INFINITE_EXPANSION;
+ }
+ if (e instanceof DetailedException) {
+ FailureDetails.PackageLoading.Code code =
+ ((DetailedException) e)
+ .getDetailedExitCode()
+ .getFailureDetail()
+ .getPackageLoading()
+ .getCode();
+ if (code == FailureDetails.PackageLoading.Code.SYMLINK_CYCLE_OR_INFINITE_EXPANSION) {
+ exceptionType =
+ RecursiveFilesystemTraversalException.Type.SYMLINK_CYCLE_OR_INFINITE_EXPANSION;
+ }
+ }
throw new RecursiveFilesystemTraversalFunctionException(
- new RecursiveFilesystemTraversalException(
- message,
- // Trying to stat the starting point of this root may have failed with a symlink cycle
- // or trying to get a package lookup value may have failed due to a symlink cycle.
- e instanceof FileSymlinkException || e.getCause() instanceof FileSymlinkException
- ? RecursiveFilesystemTraversalException.Type.SYMLINK_CYCLE_OR_INFINITE_EXPANSION
- : RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE));
+ new RecursiveFilesystemTraversalException(message, exceptionType));
}
}
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
index dead85a..7b1efbd 100644
--- a/src/main/protobuf/failure_details.proto
+++ b/src/main/protobuf/failure_details.proto
@@ -1166,8 +1166,9 @@
BUILD_FILE_MISSING = 3 [(metadata) = { exit_code: 1 }];
REPOSITORY_MISSING = 4 [(metadata) = { exit_code: 1 }];
PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR = 5
- [(metadata) = { exit_code: 1 }];
- TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR = 6 [(metadata) = { exit_code: 1 }];
+ [(metadata) = { exit_code: 36 }];
+ TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR = 6
+ [(metadata) = { exit_code: 36 }];
INVALID_NAME = 7 [(metadata) = { exit_code: 1 }];
// was: PRELUDE_FILE_READ_ERROR. Replaced by IMPORT_STARLARK_FILE_ERROR
// when the prelude was changed to be loaded as a Starlark module.
@@ -1196,6 +1197,8 @@
// builtins .bzl files are always packaged with Blaze in production, a
// failure here generally indicates a bug in Blaze.
BUILTINS_INJECTION_FAILURE = 29 [(metadata) = { exit_code: 1 }];
+ SYMLINK_CYCLE_OR_INFINITE_EXPANSION = 30 [(metadata) = { exit_code: 1 }];
+ OTHER_IO_EXCEPTION = 31 [(metadata) = { exit_code: 36 }];
}
Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index c1c8ab1..5e60185 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -345,7 +345,10 @@
.contains(
"according to stat, existing path /workspace/foo/BUILD is neither"
+ " a file nor directory nor symlink.");
- assertDetailedExitCode(ex, PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
+ assertDetailedExitCode(
+ ex,
+ PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR,
+ ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
}
@Test
@@ -376,7 +379,10 @@
String msg = ex.getMessage();
assertThat(msg).contains("Inconsistent filesystem operations");
assertThat(msg).contains("/workspace/foo/bar/baz is no longer an existing directory");
- assertDetailedExitCode(ex, PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR);
+ assertDetailedExitCode(
+ ex,
+ PackageLoading.Code.PERSISTENT_INCONSISTENT_FILESYSTEM_ERROR,
+ ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
}
/** Regression test for unexpected exception type from PackageValue. */
@@ -398,7 +404,10 @@
String msg = ex.getMessage();
assertThat(msg).contains("Inconsistent filesystem operations");
assertThat(msg).contains("Encountered error '/workspace/foo/bar (Permission denied)'");
- assertDetailedExitCode(ex, PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR);
+ assertDetailedExitCode(
+ ex,
+ PackageLoading.Code.TRANSIENT_INCONSISTENT_FILESYSTEM_ERROR,
+ ExitCode.LOCAL_ENVIRONMENTAL_ERROR);
}
@SuppressWarnings("unchecked") // Cast of srcs attribute to Iterable<Label>.
@@ -629,7 +638,8 @@
.isEqualTo(
"error loading package 'test/starlark': "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file");
- assertDetailedExitCode(ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
+ assertDetailedExitCode(
+ ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR, ExitCode.BUILD_FAILURE);
}
@Test
@@ -648,7 +658,8 @@
"error loading package 'test/starlark': "
+ "at /workspace/test/starlark/extension.bzl:1:6: "
+ "cannot load '//test/starlark:bad_extension.bzl': no such file");
- assertDetailedExitCode(ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
+ assertDetailedExitCode(
+ ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR, ExitCode.BUILD_FAILURE);
}
@Test
@@ -669,7 +680,8 @@
"error loading package 'pkg': Internal error while loading Starlark builtins: Failed"
+ " to load builtins sources: initialization of module 'exports.bzl' (internal)"
+ " failed");
- assertDetailedExitCode(ex, PackageLoading.Code.BUILTINS_INJECTION_FAILURE);
+ assertDetailedExitCode(
+ ex, PackageLoading.Code.BUILTINS_INJECTION_FAILURE, ExitCode.BUILD_FAILURE);
}
@Test
@@ -685,7 +697,8 @@
.isEqualTo(
"error loading package 'test/starlark': Encountered error while reading extension "
+ "file 'test/starlark/extension.bzl': Symlink cycle");
- assertDetailedExitCode(ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
+ assertDetailedExitCode(
+ ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR, ExitCode.BUILD_FAILURE);
}
@Test
@@ -860,7 +873,7 @@
assertThat(ex).hasMessageThat().contains("nope");
assertThat(ex).isInstanceOf(NoSuchPackageException.class);
assertThat(ex).hasCauseThat().isInstanceOf(IOException.class);
- assertDetailedExitCode(ex, PackageLoading.Code.BUILD_FILE_MISSING);
+ assertDetailedExitCode(ex, PackageLoading.Code.BUILD_FILE_MISSING, ExitCode.BUILD_FAILURE);
}
@Test
@@ -874,7 +887,8 @@
assertThat(ex).hasMessageThat().contains("nope");
assertThat(ex).isInstanceOf(NoSuchPackageException.class);
assertThat(ex).hasCauseThat().isInstanceOf(IOException.class);
- assertDetailedExitCode(ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR);
+ assertDetailedExitCode(
+ ex, PackageLoading.Code.IMPORT_STARLARK_FILE_ERROR, ExitCode.BUILD_FAILURE);
}
@Test
@@ -1325,12 +1339,14 @@
}
private static void assertDetailedExitCode(
- Exception exception, PackageLoading.Code expectedPackageLoadingCode) {
+ Exception exception, PackageLoading.Code expectedPackageLoadingCode, ExitCode exitCode) {
assertThat(exception).isInstanceOf(DetailedException.class);
DetailedExitCode detailedExitCode = ((DetailedException) exception).getDetailedExitCode();
- assertThat(detailedExitCode.getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
+ assertThat(detailedExitCode.getExitCode()).isEqualTo(exitCode);
assertThat(detailedExitCode.getFailureDetail().getPackageLoading().getCode())
.isEqualTo(expectedPackageLoadingCode);
+ assertThat(DetailedExitCode.getExitCode(detailedExitCode.getFailureDetail()))
+ .isEqualTo(exitCode);
}
/**
@@ -1644,7 +1660,7 @@
}
@Override
- protected InputStream getInputStream(PathFragment path) throws IOException {
+ protected synchronized InputStream getInputStream(PathFragment path) throws IOException {
IOException exnToThrow = pathsToErrorOnGetInputStream.get(path);
if (exnToThrow != null) {
throw exnToThrow;
