Make PackageFunction's strategy for handling unreadable BUILD files configurable. Add a test for the current behavior of treating an unreadable BUILD file as a package loading error.

RELNOTES: None
PiperOrigin-RevId: 158314187

1 files changed, 62 insertions, 12 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 1e8a955019..994472e787 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
@@ -101,6 +101,8 @@ public class PackageFunction implements SkyFunction {
   // Not final only for testing.
   @Nullable private SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining;
 
+  private final ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile;
+
   static final PathFragment DEFAULTS_PACKAGE_NAME = PathFragment.create("tools/defaults");
 
   public PackageFunction(
@@ -111,7 +113,8 @@ public class PackageFunction implements SkyFunction {
       Cache<PackageIdentifier, CacheEntryWithGlobDeps<AstAfterPreprocessing>> astCache,
       AtomicInteger numPackagesLoaded,
       @Nullable SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining,
-      @Nullable PackageProgressReceiver packageProgress) {
+      @Nullable PackageProgressReceiver packageProgress,
+      ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) {
     this.skylarkImportLookupFunctionForInlining = skylarkImportLookupFunctionForInlining;
     // Can be null in tests.
     this.preludeLabel = packageFactory == null
@@ -124,6 +127,7 @@ public class PackageFunction implements SkyFunction {
     this.astCache = astCache;
     this.numPackagesLoaded = numPackagesLoaded;
     this.packageProgress = packageProgress;
+    this.actionOnIOExceptionReadingBuildFile = actionOnIOExceptionReadingBuildFile;
   }
 
   public PackageFunction(
@@ -142,7 +146,8 @@ public class PackageFunction implements SkyFunction {
         astCache,
         numPackagesLoaded,
         skylarkImportLookupFunctionForInlining,
-        null);
+        null,
+        ActionOnIOExceptionReadingBuildFile.UseOriginalIOException.INSTANCE);
   }
 
   public void setSkylarkImportLookupFunctionForInliningForTesting(
@@ -150,6 +155,45 @@ public class PackageFunction implements SkyFunction {
     this.skylarkImportLookupFunctionForInlining = skylarkImportLookupFunctionForInlining;
   }
 
+  /**
+   * What to do when encountering an {@link IOException} trying to read the contents of a BUILD
+   * file.
+   *
+   * <p>Any choice besides
+   * {@link ActionOnIOExceptionReadingBuildFile.UseOriginalIOException#INSTANCE} is potentially
+   * incrementally unsound: if the initial {@link IOException} is transient, then Blaze will
+   * "incorrectly" not attempt to redo package loading for this BUILD file on incremental builds.
+   *
+   * <p>The fact that this behavior is configurable and potentially unsound is a concession to
+   * certain desired use cases with fancy filesystems.
+   */
+  public interface ActionOnIOExceptionReadingBuildFile {
+    /**
+     * Given the {@link IOException} encountered when reading the contents of a BUILD file,
+     * returns the contents that should be used, or {@code null} if the original {@link IOException}
+     * should be respected (that is, we should error-out with a package loading error).
+     */
+    @Nullable
+    byte[] maybeGetBuildFileContentsToUse(IOException originalExn);
+
+    /**
+     * A {@link ActionOnIOExceptionReadingBuildFile} whose {@link #maybeGetBuildFileContentsToUse}
+     * has the sensible behavior of always respecting the initial {@link IOException}.
+     */
+    public static class UseOriginalIOException implements ActionOnIOExceptionReadingBuildFile {
+      public static final UseOriginalIOException INSTANCE = new UseOriginalIOException();
+
+      private UseOriginalIOException() {
+      }
+
+      @Override
+      @Nullable
+      public byte[] maybeGetBuildFileContentsToUse(IOException originalExn) {
+        return null;
+      }
+    }
+  }
+
   /** An entry in {@link PackageFunction}'s internal caches. */
   public static class CacheEntryWithGlobDeps<T> {
     private final T value;
@@ -1142,23 +1186,29 @@ public class PackageFunction implements SkyFunction {
           ParserInputSource input;
           if (replacementContents == null) {
             Preconditions.checkNotNull(buildFileValue, packageId);
+            byte[] buildFileBytes = null;
             try {
-              byte[] buildFileBytes =
+              buildFileBytes =
                   buildFileValue.isSpecialFile()
                       ? FileSystemUtils.readContent(buildFilePath)
                       : FileSystemUtils.readWithKnownFileSize(
                           buildFilePath, buildFileValue.getSize());
-              input =
-                  ParserInputSource.create(
-                      FileSystemUtils.convertFromLatin1(buildFileBytes),
-                      buildFilePath.asFragment());
             } catch (IOException e) {
-              // Note that we did this work, so we should conservatively report this error as
-              // transient.
-              throw new PackageFunctionException(
-                  new BuildFileContainsErrorsException(packageId, e.getMessage(), e),
-                  Transience.TRANSIENT);
+              buildFileBytes =
+                  actionOnIOExceptionReadingBuildFile.maybeGetBuildFileContentsToUse(e);
+              if (buildFileBytes == null) {
+                // Note that we did the work that led to this IOException, so we should
+                // conservatively report this error as transient.
+                throw new PackageFunctionException(new BuildFileContainsErrorsException(
+                    packageId, e.getMessage(), e), Transience.TRANSIENT);
+              }
+              // If control flow reaches here, we're in territory that is deliberately unsound.
+              // See the javadoc for ActionOnIOExceptionReadingBuildFile.
             }
+            input =
+                ParserInputSource.create(
+                    FileSystemUtils.convertFromLatin1(buildFileBytes),
+                    buildFilePath.asFragment());
           } else {
             input = ParserInputSource.create(replacementContents, buildFilePath.asFragment());
           }