Update error handling for absolute symlink violations when globbing a…

… build file. PiperOrigin-RevId: 686177057 Change-Id: Id998df883ce6d8c977799194b6a9a3ad23f406fb
bazelbuild · Oct 15, 2024 · 986150a · 986150a
1 parent 48338d2
commit 986150a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 18 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -812,16 +812,23 @@ public void dump(PrintStream out) {
    * Returns an error {@link Event} with {@link Location} and {@link DetailedExitCode} properties.
    */
   public static Event error(Location location, String message, Code code) {
-    Event error = Event.error(location, message);
-    return error.withProperty(
-        DetailedExitCode.class,
+    return errorWithDetailedExitCode(
+        location,
+        message,
         DetailedExitCode.of(
             FailureDetail.newBuilder()
                 .setMessage(message)
                 .setPackageLoading(PackageLoading.newBuilder().setCode(code))
                 .build()));
   }
 
+  /** Similar to {@link #error} but with a custom {@link DetailedExitCode}. */
+  public static Event errorWithDetailedExitCode(
+      Location location, String message, DetailedExitCode detailedExitCode) {
+    Event error = Event.error(location, message);
+    return error.withProperty(DetailedExitCode.class, detailedExitCode);
+  }
+
   /**
    * If {@code pkg.containsErrors()}, sends an errorful "package contains errors" {@link Event}
    * (augmented with {@code pkg.getFailureDetail()}, if present) to the given {@link EventHandler}.

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
 import com.google.devtools.build.lib.starlarkbuildapi.StarlarkNativeModuleApi;
 import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.vfs.DetailedIOException;
 import java.io.IOException;
 import java.util.AbstractMap;
 import java.util.ArrayList;
@@ -863,15 +864,17 @@ private List<String> runGlobOperation(
               e.getMessage());
       Location loc = thread.getCallerLocation();
       Event error =
-          Package.error(
-              loc,
-              errorMessage,
-              // If there are other IOExceptions that can result from user error, they should be
-              // tested for here. Currently FileNotFoundException is not one of those, because globs
-              // only encounter that error in the presence of an inconsistent filesystem.
-              e instanceof FileSymlinkException
-                  ? Code.EVAL_GLOBS_SYMLINK_ERROR
-                  : Code.GLOB_IO_EXCEPTION);
+          switch (e) {
+            case DetailedIOException detailed ->
+                Package.errorWithDetailedExitCode(
+                    loc, errorMessage, detailed.getDetailedExitCode());
+            case FileSymlinkException symlink ->
+                Package.error(loc, errorMessage, Code.EVAL_GLOBS_SYMLINK_ERROR);
+            // If there are other IOExceptions that can result from user error, they should be
+            // tested for here. Currently, FileNotFoundException is not one of those, because globs
+            // only encounter that error in the presence of an inconsistent filesystem.
+            default -> Package.error(loc, errorMessage, Code.GLOB_IO_EXCEPTION);
+          };
       pkgBuilder.getLocalEventHandler().handle(error);
       pkgBuilder.setIOException(e, errorMessage, error.getProperty(DetailedExitCode.class));
       return ImmutableList.of();

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -61,6 +61,7 @@
 import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction.BuiltinsFailedException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.vfs.DetailedIOException;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -534,12 +535,13 @@ public SkyValue compute(SkyKey key, Environment env)
       // NoSuchPackageException. If that happens, we prefer throwing an exception derived from
       // Skyframe globbing. See the comments in #handleGlobDepsAndPropagateFilesystemExceptions.
       // Therefore we store the exception encountered here and maybe use it later.
-      pfeFromNonSkyframeGlobbing =
-          new PackageFunctionException(
-              e,
-              e.getCause() instanceof SkyframeGlobbingIOException
-                  ? Transience.PERSISTENT
-                  : Transience.TRANSIENT);
+      Transience transience =
+          switch (e.getCause()) {
+            case DetailedIOException detailed -> detailed.getTransience();
+            case SkyframeGlobbingIOException skyframeGlobbing -> Transience.PERSISTENT;
+            case null, default -> Transience.TRANSIENT;
+          };
+      pfeFromNonSkyframeGlobbing = new PackageFunctionException(e, transience);
     } catch (InternalInconsistentFilesystemException e) {
       throw e.throwPackageFunctionException();
     }