diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
9 files changed, 274 insertions, 117 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java index b44f6a138c..e04a7c6f5f 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java @@ -34,8 +34,8 @@ public interface PackageProvider extends TargetProvider { * pkg.containsErrors() == true</code>. Such packages may be missing some rules. Any rules that * are present may soundly be used for builds, though. * - * @param eventHandler the eventHandler on which to report warning and errors; if the package has - * been loaded by another thread, this eventHandler won't see any warnings or errors + * @param eventHandler the eventHandler on which to report warnings and errors associated with + * loading the package, but only if the package has not already been loaded * @param packageName a legal package name. * @throws NoSuchPackageException if the package could not be found. * @throws InterruptedException if the package loading was interrupted. @@ -56,7 +56,8 @@ public interface PackageProvider extends TargetProvider { * <p>If these don't hold, then attempting to read the package with {@link #getPackage} may fail * or may return a package containing errors. * - * @param eventHandler the eventHandler on which to report warnings and errors + * @param eventHandler if {@code packageName} specifies a package that could not be looked up + * because of a symlink cycle or IO error, the error is reported here * @param packageName the name of the package. */ boolean isPackage(ExtendedEventHandler eventHandler, PackageIdentifier packageName) diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java index 6366ebcc9d..9b857d9108 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java @@ -31,15 +31,17 @@ public interface RecursivePackageProvider extends PackageProvider { /** * Returns the names of all the packages under a given directory. * - * <p>Packages returned by this method and passed into {@link - * #bulkGetPackages(ExtendedEventHandler, Iterable)} are expected to return successful {@link - * Package} values. + * <p>Packages returned by this method and passed into {@link #bulkGetPackages(Iterable)} are + * expected to return successful {@link Package} values. * + * @param eventHandler any errors emitted during package lookup and loading for {@code directory} + * and non-excluded directories beneath it will be reported here * @param directory a {@link RootedPath} specifying the directory to search * @param excludedSubdirectories a set of {@link PathFragment}s, all of which are beneath {@code * directory}, specifying transitive subdirectories to exclude */ Iterable<PathFragment> getPackagesUnderDirectory( + ExtendedEventHandler eventHandler, RepositoryName repository, PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories) @@ -54,13 +56,10 @@ public interface RecursivePackageProvider extends PackageProvider { * pkg.containsErrors() == true</code>. Such packages may be missing some rules. Any rules that * are present may soundly be used for builds, though. * - * @param eventHandler the eventHandler on which to report warning and errors; if the package has - * been loaded by another thread, this eventHandler won't see any warnings or errors * @param pkgIds an Iterable of PackageIdentifier objects. * @throws NoSuchPackageException if any package could not be found. * @throws InterruptedException if the package loading was interrupted. */ - Map<PackageIdentifier, Package> bulkGetPackages( - ExtendedEventHandler eventHandler, Iterable<PackageIdentifier> pkgIds) + Map<PackageIdentifier, Package> bulkGetPackages(Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException, InterruptedException; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java index 9ac363df00..347cbf93e8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryFunction.java @@ -13,11 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.RecursivePkgValue.RecursivePkgKey; import com.google.devtools.build.lib.vfs.PathFragment; @@ -29,10 +30,11 @@ import java.util.Map; import javax.annotation.Nullable; /** - * <p>Computes {@link CollectPackagesUnderDirectoryValue} which describes whether the directory is a - * package and whether non-excluded packages exist below each of the directory's subdirectories. As - * a side effect, loads all of these packages, in order to interleave the disk-bound work of - * checking for directories and the CPU-bound work of package loading. + * Computes {@link CollectPackagesUnderDirectoryValue} which describes whether the directory is a + * package, or would have been a package but for a package loading error, and whether non-excluded + * packages (or errors) exist below each of the directory's subdirectories. As a side effect, loads + * all of these packages, in order to interleave the disk-bound work of checking for directories and + * the CPU-bound work of package loading. */ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { private final BlazeDirectories directories; @@ -76,28 +78,32 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { RecursivePkgKey recursivePkgKey = (RecursivePkgKey) key.argument(); CollectPackagesUnderDirectoryValue collectPackagesValue = (CollectPackagesUnderDirectoryValue) subdirectorySkyValues.get(key); - boolean packagesInSubdirectory = collectPackagesValue.isDirectoryPackage(); - // If the subdirectory isn't a package, check to see if any of its subdirectories - // transitively contain packages. - if (!packagesInSubdirectory) { - ImmutableCollection<Boolean> subdirectoryValues = - collectPackagesValue.getSubdirectoryTransitivelyContainsPackages().values(); - for (Boolean pkgsInSubSub : subdirectoryValues) { - if (pkgsInSubSub) { - packagesInSubdirectory = true; - break; - } - } - } - builder.put(recursivePkgKey.getRootedPath(), packagesInSubdirectory); + + boolean packagesOrErrorsInSubdirectory = + collectPackagesValue.isDirectoryPackage() + || collectPackagesValue.getErrorMessage() != null + || Iterables.contains( + collectPackagesValue + .getSubdirectoryTransitivelyContainsPackagesOrErrors() + .values(), + Boolean.TRUE); + + builder.put(recursivePkgKey.getRootedPath(), packagesOrErrorsInSubdirectory); } - return CollectPackagesUnderDirectoryValue.of(visitor.isDirectoryPackage(), builder.build()); + ImmutableMap<RootedPath, Boolean> subdirectories = builder.build(); + String errorMessage = visitor.getErrorMessage(); + if (errorMessage != null) { + return CollectPackagesUnderDirectoryValue.ofError(errorMessage, subdirectories); + } + return CollectPackagesUnderDirectoryValue.ofNoError( + visitor.isDirectoryPackage(), subdirectories); } } private static class MyVisitor implements RecursiveDirectoryTraversalFunction.Visitor { private boolean isDirectoryPackage; + @Nullable private String errorMessage; private MyVisitor() {} @@ -106,9 +112,20 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { isDirectoryPackage = true; } + @Override + public void visitPackageError(NoSuchPackageException e, Environment env) + throws InterruptedException { + errorMessage = e.getMessage(); + } + boolean isDirectoryPackage() { return isDirectoryPackage; } + + @Nullable + String getErrorMessage() { + return errorMessage; + } } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryValue.java index 1a431e2ca0..f7f48e59ad 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CollectPackagesUnderDirectoryValue.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -23,88 +24,201 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; - import java.util.Objects; +import javax.annotation.Nullable; /** * The value computed by {@link CollectPackagesUnderDirectoryFunction}. Contains a mapping for all - * its non-excluded directories to whether there are packages beneath them. + * its non-excluded directories to whether there are packages or error messages beneath them. * - * <p>This value is used by {@link GraphBackedRecursivePackageProvider#getPackagesUnderDirectory} - * to help it traverse the graph and find the set of packages under a directory, recursively by - * {@link CollectPackagesUnderDirectoryFunction} which computes a value for a directory by - * aggregating results calculated from its subdirectories, and by - * {@link PrepareDepsOfTargetsUnderDirectoryFunction} which uses this value to find transitive - * targets to load. + * <p>This value is used by {@link GraphBackedRecursivePackageProvider#getPackagesUnderDirectory} to + * help it traverse the graph and find the set of packages under a directory, recursively by {@link + * CollectPackagesUnderDirectoryFunction} which computes a value for a directory by aggregating + * results calculated from its subdirectories, and by {@link + * PrepareDepsOfTargetsUnderDirectoryFunction} which uses this value to find transitive targets to + * load. * - * <p>Note that even though the {@link CollectPackagesUnderDirectoryFunction} is evaluated in - * part because of its side-effects (i.e. loading transitive dependencies of targets), this value + * <p>Note that even though the {@link CollectPackagesUnderDirectoryFunction} is evaluated in part + * because of its side-effects (i.e. loading transitive dependencies of targets), this value * interacts safely with change pruning, despite the fact that this value is a lossy representation * of the packages beneath a directory (i.e. it doesn't care <b>which</b> packages are under a - * directory, just whether there are any). When the targets in a package change, the - * {@link PackageValue} that {@link CollectPackagesUnderDirectoryFunction} depends on will be - * invalidated, and the PrepareDeps function for that package's directory will be reevaluated, - * loading any new transitive dependencies. Change pruning may prevent the reevaluation of - * PrepareDeps for directories above that one, but they don't need to be re-run. + * directory, just whether there are any). When the targets in a package change, the {@link + * PackageValue} that {@link CollectPackagesUnderDirectoryFunction} depends on will be invalidated, + * and the PrepareDeps function for that package's directory will be reevaluated, loading any new + * transitive dependencies. Change pruning may prevent the reevaluation of PrepareDeps for + * directories above that one, but they don't need to be re-run. */ -public class CollectPackagesUnderDirectoryValue implements SkyValue { - public static final CollectPackagesUnderDirectoryValue EMPTY = - new CollectPackagesUnderDirectoryValue(false, ImmutableMap.<RootedPath, Boolean>of()); +public abstract class CollectPackagesUnderDirectoryValue implements SkyValue { - private final boolean isDirectoryPackage; - private final ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages; + private final ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors; - private CollectPackagesUnderDirectoryValue( - boolean isDirectoryPackage, - ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages) { - this.subdirectoryTransitivelyContainsPackages = subdirectoryTransitivelyContainsPackages; - this.isDirectoryPackage = isDirectoryPackage; + CollectPackagesUnderDirectoryValue( + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors) { + this.subdirectoryTransitivelyContainsPackagesOrErrors = + Preconditions.checkNotNull(subdirectoryTransitivelyContainsPackagesOrErrors); } - public static CollectPackagesUnderDirectoryValue of( - boolean isDirectoryPackage, - ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages) { - if (!isDirectoryPackage && subdirectoryTransitivelyContainsPackages.isEmpty()) { - return EMPTY; + /** Represents a successfully loaded package or a directory without a BUILD file. */ + public static class NoErrorCollectPackagesUnderDirectoryValue + extends CollectPackagesUnderDirectoryValue { + public static final NoErrorCollectPackagesUnderDirectoryValue EMPTY = + new NoErrorCollectPackagesUnderDirectoryValue(false, ImmutableMap.of()); + + private final boolean isDirectoryPackage; + + private NoErrorCollectPackagesUnderDirectoryValue( + boolean isDirectoryPackage, + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors) { + super(subdirectoryTransitivelyContainsPackagesOrErrors); + this.isDirectoryPackage = isDirectoryPackage; } - return new CollectPackagesUnderDirectoryValue( - isDirectoryPackage, subdirectoryTransitivelyContainsPackages); - } - public boolean isDirectoryPackage() { - return isDirectoryPackage; - } + @Override + public boolean isDirectoryPackage() { + return isDirectoryPackage; + } - public ImmutableMap<RootedPath, Boolean> getSubdirectoryTransitivelyContainsPackages() { - return subdirectoryTransitivelyContainsPackages; - } + @Nullable + @Override + public String getErrorMessage() { + return null; + } + + @Override + public int hashCode() { + return Objects.hash( + isDirectoryPackage, getSubdirectoryTransitivelyContainsPackagesOrErrors()); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof NoErrorCollectPackagesUnderDirectoryValue)) { + return false; + } + NoErrorCollectPackagesUnderDirectoryValue that = + (NoErrorCollectPackagesUnderDirectoryValue) o; + return this.isDirectoryPackage == that.isDirectoryPackage + && Objects.equals( + this.getSubdirectoryTransitivelyContainsPackagesOrErrors(), + that.getSubdirectoryTransitivelyContainsPackagesOrErrors()); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("isDirectoryPackage", isDirectoryPackage) + .add( + "subdirectoryTransitivelyContainsPackagesOrErrors", + getSubdirectoryTransitivelyContainsPackagesOrErrors()) + .toString(); + } - @Override - public int hashCode() { - return Objects.hash(isDirectoryPackage, subdirectoryTransitivelyContainsPackages); } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; + /** Represents a directory with a BUILD file that failed to load. */ + public static class ErrorCollectPackagesUnderDirectoryValue + extends CollectPackagesUnderDirectoryValue { + private final String errorMessage; + + private ErrorCollectPackagesUnderDirectoryValue( + String errorMessage, + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors) { + super(subdirectoryTransitivelyContainsPackagesOrErrors); + this.errorMessage = Preconditions.checkNotNull(errorMessage); } - if (!(o instanceof CollectPackagesUnderDirectoryValue)) { + + @Override + public boolean isDirectoryPackage() { return false; } - CollectPackagesUnderDirectoryValue that = (CollectPackagesUnderDirectoryValue) o; - return this.isDirectoryPackage == that.isDirectoryPackage - && this - .subdirectoryTransitivelyContainsPackages.equals( - that.subdirectoryTransitivelyContainsPackages); + + @Override + public String getErrorMessage() { + return errorMessage; + } + + @Override + public int hashCode() { + return Objects.hash(errorMessage, getSubdirectoryTransitivelyContainsPackagesOrErrors()); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ErrorCollectPackagesUnderDirectoryValue)) { + return false; + } + ErrorCollectPackagesUnderDirectoryValue that = (ErrorCollectPackagesUnderDirectoryValue) o; + return Objects.equals(this.errorMessage, that.errorMessage) + && Objects.equals( + this.getSubdirectoryTransitivelyContainsPackagesOrErrors(), + that.getSubdirectoryTransitivelyContainsPackagesOrErrors()); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("errorMessage", errorMessage) + .add( + "subdirectoryTransitivelyContainsPackagesOrErrors", + getSubdirectoryTransitivelyContainsPackagesOrErrors()) + .toString(); + } } - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("isDirectoryPackage", isDirectoryPackage) - .add("subdirectoryTransitivelyContainsPackages", subdirectoryTransitivelyContainsPackages) - .toString(); + /** + * Constructs a {@link CollectPackagesUnderDirectoryValue} for a directory with a BUILD file that + * failed to load as a package. + */ + public static CollectPackagesUnderDirectoryValue ofError( + String errorMessage, + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors) { + Preconditions.checkNotNull(errorMessage, "errorMessage"); + return new ErrorCollectPackagesUnderDirectoryValue( + errorMessage, subdirectoryTransitivelyContainsPackagesOrErrors); + } + + /** + * Constructs a {@link CollectPackagesUnderDirectoryValue} for a directory without a BUILD file or + * that has a BUILD file that successfully loads as a package. + */ + public static CollectPackagesUnderDirectoryValue ofNoError( + boolean isDirectoryPackage, + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackagesOrErrors) { + if (!isDirectoryPackage && subdirectoryTransitivelyContainsPackagesOrErrors.isEmpty()) { + return NoErrorCollectPackagesUnderDirectoryValue.EMPTY; + } + return new NoErrorCollectPackagesUnderDirectoryValue( + isDirectoryPackage, subdirectoryTransitivelyContainsPackagesOrErrors); + } + + /** + * Returns whether there is a BUILD file in this directory that can be loaded as a package. If + * this returns {@code true}, then {@link #getErrorMessage()} returns {@code null}. + */ + public abstract boolean isDirectoryPackage(); + + /** + * Returns an error describing why the BUILD file in this directory cannot be loaded as a package, + * if there is one and it can't be. Otherwise returns {@code null}. If this returns non-{@code + * null}, then {@link #isDirectoryPackage()} returns {@code false}. + */ + @Nullable + public abstract String getErrorMessage(); + + /** + * Returns an {@link ImmutableMap} describing each immediate subdirectory of this directory and + * whether there are any packages, or BUILD files that couldn't be loaded, in or beneath that + * subdirectory. + */ + public final ImmutableMap<RootedPath, Boolean> + getSubdirectoryTransitivelyContainsPackagesOrErrors() { + return subdirectoryTransitivelyContainsPackagesOrErrors; } /** Create a collect packages under directory request. */ diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java index e254c58fd8..0b7eaf9d7d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java @@ -41,9 +41,12 @@ import java.util.List; import java.util.Map; /** - * A {@link RecursivePackageProvider} backed by an {@link Environment}. Its methods - * may throw {@link MissingDepException} if the package values this depends on haven't been - * calculated and added to its environment. + * A {@link RecursivePackageProvider} backed by an {@link Environment}. Its methods may throw {@link + * MissingDepException} if the package values this depends on haven't been calculated and added to + * its environment. + * + * <p>This implementation never emits events through the {@link ExtendedEventHandler}s passed to its + * methods. Instead, it emits events through its environment's {@link Environment#getListener()}. */ public final class EnvironmentBackedRecursivePackageProvider implements RecursivePackageProvider { @@ -80,12 +83,11 @@ public final class EnvironmentBackedRecursivePackageProvider implements Recursiv } @Override - public Map<PackageIdentifier, Package> bulkGetPackages( - ExtendedEventHandler eventHandler, Iterable<PackageIdentifier> pkgIds) + public Map<PackageIdentifier, Package> bulkGetPackages(Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException, InterruptedException { ImmutableMap.Builder<PackageIdentifier, Package> builder = ImmutableMap.builder(); for (PackageIdentifier pkgId : pkgIds) { - builder.put(pkgId, getPackage(eventHandler, pkgId)); + builder.put(pkgId, getPackage(env.getListener(), pkgId)); } return builder.build(); } @@ -103,13 +105,14 @@ public final class EnvironmentBackedRecursivePackageProvider implements Recursiv } return packageLookupValue.packageExists(); } catch (NoSuchPackageException | InconsistentFilesystemException e) { - eventHandler.handle(Event.error(e.getMessage())); + env.getListener().handle(Event.error(e.getMessage())); return false; } } @Override public Iterable<PathFragment> getPackagesUnderDirectory( + ExtendedEventHandler eventHandler, RepositoryName repository, PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java index 2140b069a3..a64372d6b1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java @@ -99,8 +99,7 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka } @Override - public Map<PackageIdentifier, Package> bulkGetPackages( - ExtendedEventHandler eventHandler, Iterable<PackageIdentifier> pkgIds) + public Map<PackageIdentifier, Package> bulkGetPackages(Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException, InterruptedException { Set<SkyKey> pkgKeys = ImmutableSet.copyOf(PackageValue.keys(pkgIds)); @@ -164,6 +163,7 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka @Override public Iterable<PathFragment> getPackagesUnderDirectory( + ExtendedEventHandler eventHandler, RepositoryName repository, PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories) @@ -175,11 +175,8 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka for (TargetPatternKey patternKey : universeTargetPatternKeys) { TargetPattern pattern = patternKey.getParsedPattern(); boolean isTBD = pattern.getType().equals(Type.TARGETS_BELOW_DIRECTORY); - PackageIdentifier packageIdentifier = PackageIdentifier.create( - repository, directory); - if (isTBD - && pattern.containsAllTransitiveSubdirectoriesForTBD( - packageIdentifier)) { + PackageIdentifier packageIdentifier = PackageIdentifier.create(repository, directory); + if (isTBD && pattern.containsAllTransitiveSubdirectoriesForTBD(packageIdentifier)) { inUniverse = true; break; } @@ -210,12 +207,13 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka for (Path root : roots) { RootedPath rootedDir = RootedPath.toRootedPath(root, directory); TraversalInfo info = new TraversalInfo(rootedDir, excludedSubdirectories); - collectPackagesUnder(repository, ImmutableSet.of(info), builder); + collectPackagesUnder(eventHandler, repository, ImmutableSet.of(info), builder); } return builder.build(); } private void collectPackagesUnder( + ExtendedEventHandler eventHandler, final RepositoryName repository, Set<TraversalInfo> traversals, ImmutableList.Builder<PathFragment> builder) @@ -244,8 +242,12 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka builder.add(info.rootedDir.getRelativePath()); } + if (collectPackagesValue.getErrorMessage() != null) { + eventHandler.handle(Event.error(collectPackagesValue.getErrorMessage())); + } + ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages = - collectPackagesValue.getSubdirectoryTransitivelyContainsPackages(); + collectPackagesValue.getSubdirectoryTransitivelyContainsPackagesOrErrors(); for (RootedPath subdirectory : subdirectoryTransitivelyContainsPackages.keySet()) { if (subdirectoryTransitivelyContainsPackages.get(subdirectory)) { PathFragment subdirectoryRelativePath = subdirectory.getRelativePath(); @@ -261,7 +263,7 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka ImmutableSet<TraversalInfo> subdirTraversals = subdirTraversalBuilder.build(); if (!subdirTraversals.isEmpty()) { - collectPackagesUnder(repository, subdirTraversals, builder); + collectPackagesUnder(eventHandler, repository, subdirTraversals, builder); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java index a7491d5fa1..fcdccbd04a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveDirectoryTraversalFunction.java @@ -35,14 +35,12 @@ import com.google.devtools.build.skyframe.ValueOrException; import java.util.Map; /** - * RecursiveDirectoryTraversalFunction traverses the subdirectories of a directory, looking for - * and loading packages, and builds up a value from these packages in a manner customized by - * classes that derive from it. + * RecursiveDirectoryTraversalFunction traverses the subdirectories of a directory, looking for and + * loading packages, and builds up a value from the packages and package loading errors in a manner + * customized by classes that derive from it. */ -abstract class RecursiveDirectoryTraversalFunction - <TVisitor extends RecursiveDirectoryTraversalFunction.Visitor, TReturn> { - private static final String SENTINEL_FILE_NAME_FOR_NOT_TRAVERSING_SYMLINKS = - "DONT_FOLLOW_SYMLINKS_WHEN_TRAVERSING_THIS_DIRECTORY_VIA_A_RECURSIVE_TARGET_PATTERN"; +abstract class RecursiveDirectoryTraversalFunction< + TVisitor extends RecursiveDirectoryTraversalFunction.Visitor, TReturn> { private final ProcessPackageDirectory processPackageDirectory; @@ -105,6 +103,18 @@ abstract class RecursiveDirectoryTraversalFunction * afterwards. */ void visitPackageValue(Package pkg, Environment env) throws InterruptedException; + + /** + * Called iff the directory contains a BUILD file but *not* a package, which can happen under + * the following circumstances: + * + * <ol> + * <li>The BUILD file contains a Skylark load statement that is in error + * <li>TODO(mschaller), not yet implemented: The BUILD file is a symlink that points into a + * cycle + * </ol> + */ + void visitPackageError(NoSuchPackageException e, Environment env) throws InterruptedException; } /** @@ -159,8 +169,11 @@ abstract class RecursiveDirectoryTraversalFunction } catch (NoSuchPackageException e) { // The package had errors, but don't fail-fast as there might be subpackages below the // current directory. - env.getListener() - .handle(Event.error("package contains errors: " + rootRelativePath.getPathString())); + env.getListener().handle(Event.error(e.getMessage())); + visitor.visitPackageError(e, env); + if (env.valuesMissing()) { + return null; + } } if (pkg != null) { visitor.visitPackageValue(pkg, env); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java index dc84f5b6a1..b68ef437f8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java @@ -97,7 +97,7 @@ public class RecursivePackageProviderBackedTargetPatternResolver private Map<PackageIdentifier, Package> bulkGetPackages(Iterable<PackageIdentifier> pkgIds) throws NoSuchPackageException, InterruptedException { - return recursivePackageProvider.bulkGetPackages(eventHandler, pkgIds); + return recursivePackageProvider.bulkGetPackages(pkgIds); } @Override @@ -244,8 +244,9 @@ public class RecursivePackageProviderBackedTargetPatternResolver Iterable<PathFragment> packagesUnderDirectory; try { pathFragment = TargetPatternResolverUtil.getPathFragment(directory); - packagesUnderDirectory = recursivePackageProvider.getPackagesUnderDirectory( - repository, pathFragment, excludedSubdirectories); + packagesUnderDirectory = + recursivePackageProvider.getPackagesUnderDirectory( + eventHandler, repository, pathFragment, excludedSubdirectories); } catch (TargetParsingException e) { return Futures.immediateFailedFuture(e); } catch (InterruptedException e) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java index 95cc59b5e7..58f79be753 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgFunction.java @@ -89,6 +89,13 @@ public class RecursivePkgFunction implements SkyFunction { packages.add(pkg.getName()); } + @Override + public void visitPackageError(NoSuchPackageException e, Environment env) + throws InterruptedException { + // Nothing to do because the RecursiveDirectoryTraversalFunction has already emitted an error + // event. + } + void addTransitivePackages(NestedSet<String> transitivePackages) { packages.addTransitive(transitivePackages); } |