Make FileSymlinkException and InconsistentFSException IOExceptions

Most places handle them the same way as IOException, which seems like a safe
default. The places that do care can still throw or catch the more specific type.

PiperOrigin-RevId: 181719688

1 files changed, 35 insertions, 44 deletions

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
index c3e67c7c52..b10ab4b952 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -66,9 +66,9 @@ import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.ValueOrException;
 import com.google.devtools.build.skyframe.ValueOrException2;
 import com.google.devtools.build.skyframe.ValueOrException3;
-import com.google.devtools.build.skyframe.ValueOrException4;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -296,18 +296,17 @@ public class PackageFunction implements SkyFunction {
     Preconditions.checkState(
         Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.FILE)), depKeys);
     boolean packageShouldBeInError = packageWasInError;
-    for (Map.Entry<SkyKey, ValueOrException3<IOException, FileSymlinkException,
-        InconsistentFilesystemException>> entry : env.getValuesOrThrow(depKeys, IOException.class,
-            FileSymlinkException.class, InconsistentFilesystemException.class).entrySet()) {
+    for (Map.Entry<SkyKey, ValueOrException<IOException>> entry :
+        env.getValuesOrThrow(depKeys, IOException.class).entrySet()) {
       try {
         entry.getValue().get();
-      } catch (IOException e) {
-        maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
+      } catch (InconsistentFilesystemException e) {
+        throw new InternalInconsistentFilesystemException(packageIdentifier, e);
       } catch (FileSymlinkException e) {
         // Legacy doesn't detect symlink cycles.
         packageShouldBeInError = true;
-      } catch (InconsistentFilesystemException e) {
-        throw new InternalInconsistentFilesystemException(packageIdentifier, e);
+      } catch (IOException e) {
+        maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
       }
     }
     return packageShouldBeInError;
@@ -326,19 +325,18 @@ public class PackageFunction implements SkyFunction {
     Preconditions.checkState(
         Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
     boolean packageShouldBeInError = packageWasInError;
-    for (Map.Entry<SkyKey, ValueOrException4<IOException, BuildFileNotFoundException,
-        FileSymlinkException, InconsistentFilesystemException>> entry :
-        env.getValuesOrThrow(depKeys, IOException.class, BuildFileNotFoundException.class,
-            FileSymlinkException.class, InconsistentFilesystemException.class).entrySet()) {
+    for (Map.Entry<SkyKey, ValueOrException2<IOException, BuildFileNotFoundException>> entry :
+        env.getValuesOrThrow(
+            depKeys, IOException.class, BuildFileNotFoundException.class).entrySet()) {
       try {
         entry.getValue().get();
-      } catch (IOException | BuildFileNotFoundException e) {
-        maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
+      } catch (InconsistentFilesystemException e) {
+        throw new InternalInconsistentFilesystemException(packageIdentifier, e);
       } catch (FileSymlinkException e) {
         // Legacy doesn't detect symlink cycles.
         packageShouldBeInError = true;
-      } catch (InconsistentFilesystemException e) {
-        throw new InternalInconsistentFilesystemException(packageIdentifier, e);
+      } catch (IOException | BuildFileNotFoundException e) {
+        maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
       }
     }
     return packageShouldBeInError;
@@ -453,12 +451,9 @@ public class PackageFunction implements SkyFunction {
               env.getValueOrThrow(
                   workspaceKey,
                   IOException.class,
-                  FileSymlinkException.class,
-                  InconsistentFilesystemException.class,
                   EvalException.class,
                   SkylarkImportFailedException.class);
-    } catch (IOException | FileSymlinkException | InconsistentFilesystemException
-          | EvalException | SkylarkImportFailedException e) {
+    } catch (IOException | EvalException | SkylarkImportFailedException e) {
       throw new PackageFunctionException(
           new NoSuchPackageException(
               Label.EXTERNAL_PACKAGE_IDENTIFIER,
@@ -654,10 +649,9 @@ public class PackageFunction implements SkyFunction {
       throws InterruptedException {
     FileValue buildFileValue;
     try {
-      buildFileValue = (FileValue) env.getValueOrThrow(FileValue.key(buildFileRootedPath),
-          IOException.class, FileSymlinkException.class,
-          InconsistentFilesystemException.class);
-    } catch (IOException | FileSymlinkException | InconsistentFilesystemException e) {
+      buildFileValue =
+          (FileValue) env.getValueOrThrow(FileValue.key(buildFileRootedPath), IOException.class);
+    } catch (IOException e) {
       throw new IllegalStateException("Package lookup succeeded but encountered error when "
           + "getting FileValue for BUILD file directly.", e);
     }
@@ -1051,10 +1045,10 @@ public class PackageFunction implements SkyFunction {
       }
       globDepsRequested.addAll(globKeys);
 
-      Map<SkyKey, ValueOrException4<IOException, BuildFileNotFoundException,
-          FileSymlinkCycleException, InconsistentFilesystemException>> globValueMap =
+      Map<SkyKey, ValueOrException3<IOException, BuildFileNotFoundException,
+          FileSymlinkCycleException>> globValueMap =
           env.getValuesOrThrow(globKeys, IOException.class, BuildFileNotFoundException.class,
-              FileSymlinkCycleException.class, InconsistentFilesystemException.class);
+              FileSymlinkCycleException.class);
 
       // For each missing glob, evaluate it asychronously via the delegate.
       //
@@ -1087,12 +1081,12 @@ public class PackageFunction implements SkyFunction {
     }
 
     private Collection<SkyKey> getMissingKeys(Collection<SkyKey> globKeys,
-        Map<SkyKey, ValueOrException4<IOException, BuildFileNotFoundException,
-            FileSymlinkCycleException, InconsistentFilesystemException>> globValueMap) {
+        Map<SkyKey, ValueOrException3<IOException, BuildFileNotFoundException,
+            FileSymlinkCycleException>> globValueMap) {
       List<SkyKey> missingKeys = new ArrayList<>(globKeys.size());
       for (SkyKey globKey : globKeys) {
-        ValueOrException4<IOException, BuildFileNotFoundException, FileSymlinkCycleException,
-            InconsistentFilesystemException> valueOrException = globValueMap.get(globKey);
+        ValueOrException3<IOException, BuildFileNotFoundException, FileSymlinkCycleException>
+            valueOrException = globValueMap.get(globKey);
         if (valueOrException == null) {
           missingKeys.add(globKey);
         }
@@ -1100,8 +1094,7 @@ public class PackageFunction implements SkyFunction {
           if (valueOrException.get() == null) {
             missingKeys.add(globKey);
           }
-        } catch (IOException | BuildFileNotFoundException | FileSymlinkCycleException
-            | InconsistentFilesystemException doesntMatter) {
+        } catch (IOException | BuildFileNotFoundException doesntMatter) {
           continue;
         }
       }
@@ -1156,8 +1149,8 @@ public class PackageFunction implements SkyFunction {
      */
     private static class HybridToken extends Globber.Token {
       // The result of the Skyframe lookup for all the needed glob patterns.
-      private final Map<SkyKey, ValueOrException4<IOException, BuildFileNotFoundException,
-          FileSymlinkCycleException, InconsistentFilesystemException>> globValueMap;
+      private final Map<SkyKey, ValueOrException3<IOException, BuildFileNotFoundException,
+          FileSymlinkCycleException>> globValueMap;
       // The skyframe keys corresponding to the 'includes' patterns fetched from Skyframe
       // (this is includes_sky above).
       private final Iterable<SkyKey> includesGlobKeys;
@@ -1169,8 +1162,8 @@ public class PackageFunction implements SkyFunction {
       // A token for computing excludes_leg.
       private final Token legacyExcludesToken;
 
-      private HybridToken(Map<SkyKey, ValueOrException4<IOException, BuildFileNotFoundException,
-          FileSymlinkCycleException, InconsistentFilesystemException>> globValueMap,
+      private HybridToken(Map<SkyKey, ValueOrException3<IOException, BuildFileNotFoundException,
+          FileSymlinkCycleException>> globValueMap,
           Iterable<SkyKey> includesGlobKeys, Iterable<SkyKey> excludesGlobKeys,
           Token delegateIncludesToken, Token delegateExcludesToken) {
         this.globValueMap = globValueMap;
@@ -1208,20 +1201,18 @@ public class PackageFunction implements SkyFunction {
           SkyKey globKey,
           Map<
                   SkyKey,
-                  ValueOrException4<
-                      IOException, BuildFileNotFoundException, FileSymlinkCycleException,
-                      InconsistentFilesystemException>>
+                  ValueOrException3<
+                      IOException, BuildFileNotFoundException, FileSymlinkCycleException>>
               globValueMap)
           throws IOException {
-        ValueOrException4<IOException, BuildFileNotFoundException, FileSymlinkCycleException,
-            InconsistentFilesystemException> valueOrException =
+        ValueOrException3<IOException, BuildFileNotFoundException, FileSymlinkCycleException>
+            valueOrException =
                 Preconditions.checkNotNull(globValueMap.get(globKey), "%s should not be missing",
                     globKey);
         try {
           return Preconditions.checkNotNull((GlobValue) valueOrException.get(),
               "%s should not be missing", globKey).getMatches();
-        } catch (BuildFileNotFoundException | FileSymlinkCycleException
-            | InconsistentFilesystemException e) {
+        } catch (BuildFileNotFoundException e) {
           // Legacy package loading is only able to handle an IOException, so a rethrow here is the
           // best we can do. But after legacy package loading, PackageFunction will go through all
           // the skyframe deps and properly handle InconsistentFilesystemExceptions.