diff options
Diffstat (limited to 'src/main/java/com/google')
7 files changed, 109 insertions, 26 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java b/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java index 9fb17e97d8..6b892a8319 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BazelSkyframeExecutorConstants.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; @@ -27,4 +28,8 @@ public class BazelSkyframeExecutorConstants { public static final ImmutableList<BuildFileName> BUILD_FILES_BY_PRIORITY = ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD); + + public static final ActionOnIOExceptionReadingBuildFile + ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE = + ActionOnIOExceptionReadingBuildFile.UseOriginalIOException.INSTANCE; } 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()); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 26e146b88b..845429ec3f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.UnionDirtine import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFilesKnowledge; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType; +import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.syntax.SkylarkSemanticsOptions; @@ -126,7 +127,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { PathFragment blacklistedPackagePrefixesFile, String productName, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, - List<BuildFileName> buildFilesByPriority) { + List<BuildFileName> buildFilesByPriority, + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { super( evaluatorSupplier, pkgFactory, @@ -141,7 +143,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { blacklistedPackagePrefixesFile, productName, crossRepositoryLabelViolationStrategy, - buildFilesByPriority); + buildFilesByPriority, + actionOnIOExceptionReadingBuildFile); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); this.customDirtinessCheckers = customDirtinessCheckers; } @@ -160,7 +163,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { PathFragment blacklistedPackagePrefixesFile, String productName, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, - List<BuildFileName> buildFilesByPriority) { + List<BuildFileName> buildFilesByPriority, + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { SequencedSkyframeExecutor skyframeExecutor = new SequencedSkyframeExecutor( InMemoryMemoizingEvaluator.SUPPLIER, @@ -177,7 +181,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { blacklistedPackagePrefixesFile, productName, crossRepositoryLabelViolationStrategy, - buildFilesByPriority); + buildFilesByPriority, + actionOnIOExceptionReadingBuildFile); skyframeExecutor.init(); return skyframeExecutor; } @@ -195,7 +200,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { Iterable<SkyValueDirtinessChecker> customDirtinessCheckers, String productName, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, - List<BuildFileName> buildFilesByPriority) { + List<BuildFileName> buildFilesByPriority, + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { return create( pkgFactory, directories, @@ -210,7 +216,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { /*blacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT, productName, crossRepositoryLabelViolationStrategy, - buildFilesByPriority); + buildFilesByPriority, + actionOnIOExceptionReadingBuildFile); } @VisibleForTesting @@ -239,7 +246,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { customDirtinessCheckers, productName, BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY, - BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY); + BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, + BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE); } @VisibleForTesting @@ -256,7 +264,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { Iterable<SkyValueDirtinessChecker> customDirtinessCheckers, String productName, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, - ImmutableList<BuildFileName> buildFilesByPriority) { + ImmutableList<BuildFileName> buildFilesByPriority, + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { return create( pkgFactory, directories, @@ -271,7 +280,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { /*blacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT, productName, crossRepositoryLabelViolationStrategy, - buildFilesByPriority); + buildFilesByPriority, + actionOnIOExceptionReadingBuildFile); } @VisibleForTesting @@ -298,7 +308,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { blacklistedPackagePrefixesFile, productName, BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY, - BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY); + BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, + BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index b6e91fec82..3617c13f13 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -56,6 +56,7 @@ public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory customDirtinessCheckers, productName, BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY, - BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY); + BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, + BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 77d8394679..69c4bbc0e5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -108,6 +108,7 @@ import com.google.devtools.build.lib.profiler.AutoProfiler; import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; +import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageFunction.CacheEntryWithGlobDeps; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; @@ -284,6 +285,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { private final List<BuildFileName> buildFilesByPriority; + private final ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile; + private PerBuildSyscallCache perBuildSyscallCache; private int lastConcurrencyLevel = -1; @@ -303,7 +306,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { PathFragment blacklistedPackagePrefixesFile, String productName, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, - List<BuildFileName> buildFilesByPriority) { + List<BuildFileName> buildFilesByPriority, + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { // Strictly speaking, these arguments are not required for initialization, but all current // callsites have them at hand, so we might as well set them during construction. this.evaluatorSupplier = evaluatorSupplier; @@ -335,6 +339,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { this.productName = productName; this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy; this.buildFilesByPriority = buildFilesByPriority; + this.actionOnIOExceptionReadingBuildFile = actionOnIOExceptionReadingBuildFile; this.removeActionsAfterEvaluation.set(false); } @@ -472,7 +477,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { astCache, numPackagesLoaded, null, - packageProgress); + packageProgress, + actionOnIOExceptionReadingBuildFile); } protected SkyFunction newSkylarkImportLookupFunction( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 9d5a1b0634..00a4a1257b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.skyframe.FileSymlinkCycleUniquenessFunction import com.google.devtools.build.lib.skyframe.FileSymlinkInfiniteExpansionUniquenessFunction; import com.google.devtools.build.lib.skyframe.GlobFunction; import com.google.devtools.build.lib.skyframe.PackageFunction; +import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageFunction.CacheEntryWithGlobDeps; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; @@ -310,6 +311,7 @@ public abstract class AbstractPackageLoader implements PackageLoader { protected abstract CrossRepositoryLabelViolationStrategy getCrossRepositoryLabelViolationStrategy(); protected abstract ImmutableList<BuildFileName> getBuildFilesByPriority(); + protected abstract ActionOnIOExceptionReadingBuildFile getActionOnIOExceptionReadingBuildFile(); protected abstract ImmutableMap<SkyFunctionName, SkyFunction> getExtraExtraSkyFunctions(); protected final ImmutableMap<SkyFunctionName, SkyFunction> makeFreshSkyFunctions() { @@ -368,7 +370,9 @@ public abstract class AbstractPackageLoader implements PackageLoader { packageFunctionCache, astCache, /*numPackagesLoaded=*/ new AtomicInteger(0), - null)) + /*skylarkImportLookupFunctionForInlining=*/ null, + /*packageProgress=*/ null, + getActionOnIOExceptionReadingBuildFile())) .putAll(extraSkyFunctions) .putAll(getExtraExtraSkyFunctions()); return builder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index aa88fd59dc..7c388e5536 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; import com.google.devtools.build.lib.skyframe.LocalRepositoryLookupFunction; +import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -86,6 +87,11 @@ public class BazelPackageLoader extends AbstractPackageLoader { } @Override + protected ActionOnIOExceptionReadingBuildFile getActionOnIOExceptionReadingBuildFile() { + return BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE; + } + + @Override protected ImmutableMap<SkyFunctionName, SkyFunction> getExtraExtraSkyFunctions() { return ImmutableMap.<SkyFunctionName, SkyFunction>of( SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction()); |