diff options
author | janakr <janakr@google.com> | 2017-04-27 17:43:16 +0200 |
---|---|---|
committer | Vladimir Moskva <vladmos@google.com> | 2017-04-28 01:01:13 +0200 |
commit | 67b992804f36e1e9bdb72285455fcc2f4b01307c (patch) | |
tree | 33f08ce9104f02205f42356d220828b35bbe71d6 /src/main/java/com/google/devtools/build/lib | |
parent | 999455030720c1502484aaa206e63afedc66f861 (diff) |
Simplify RecursiveDirectoryTraversalFunction: neither of its use cases were using the env variable passed into their visitors' methods, which means that we can stop passing them, and simplify the code a bit.
Just a cleanup I noticed.
PiperOrigin-RevId: 154426694
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
3 files changed, 50 insertions, 64 deletions
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 347cbf93e8..214daf54c1 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 @@ -19,7 +19,6 @@ 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; import com.google.devtools.build.lib.vfs.RootedPath; @@ -49,15 +48,16 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { } private class MyTraversalFunction - extends RecursiveDirectoryTraversalFunction<MyVisitor, CollectPackagesUnderDirectoryValue> { + extends RecursiveDirectoryTraversalFunction< + MyPackageDirectoryConsumer, CollectPackagesUnderDirectoryValue> { private MyTraversalFunction() { super(directories); } @Override - protected MyVisitor getInitialVisitor() { - return new MyVisitor(); + protected MyPackageDirectoryConsumer getInitialConsumer() { + return new MyPackageDirectoryConsumer(); } @Override @@ -71,7 +71,7 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { @Override protected CollectPackagesUnderDirectoryValue aggregateWithSubdirectorySkyValues( - MyVisitor visitor, Map<SkyKey, SkyValue> subdirectorySkyValues) { + MyPackageDirectoryConsumer consumer, Map<SkyKey, SkyValue> subdirectorySkyValues) { // Aggregate the child subdirectory package state. ImmutableMap.Builder<RootedPath, Boolean> builder = ImmutableMap.builder(); for (SkyKey key : subdirectorySkyValues.keySet()) { @@ -91,30 +91,30 @@ public class CollectPackagesUnderDirectoryFunction implements SkyFunction { builder.put(recursivePkgKey.getRootedPath(), packagesOrErrorsInSubdirectory); } ImmutableMap<RootedPath, Boolean> subdirectories = builder.build(); - String errorMessage = visitor.getErrorMessage(); + String errorMessage = consumer.getErrorMessage(); if (errorMessage != null) { return CollectPackagesUnderDirectoryValue.ofError(errorMessage, subdirectories); } return CollectPackagesUnderDirectoryValue.ofNoError( - visitor.isDirectoryPackage(), subdirectories); + consumer.isDirectoryPackage(), subdirectories); } } - private static class MyVisitor implements RecursiveDirectoryTraversalFunction.Visitor { + private static class MyPackageDirectoryConsumer + implements RecursiveDirectoryTraversalFunction.PackageDirectoryConsumer { private boolean isDirectoryPackage; @Nullable private String errorMessage; - private MyVisitor() {} + private MyPackageDirectoryConsumer() {} @Override - public void visitPackageValue(Package pkg, Environment env) { + public void notePackage(PathFragment pkgPath) { isDirectoryPackage = true; } @Override - public void visitPackageError(NoSuchPackageException e, Environment env) - throws InterruptedException { + public void notePackageError(NoSuchPackageException e) { errorMessage = e.getMessage(); } 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 fcdccbd04a..f72e436f02 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 @@ -40,7 +40,7 @@ import java.util.Map; * customized by classes that derive from it. */ abstract class RecursiveDirectoryTraversalFunction< - TVisitor extends RecursiveDirectoryTraversalFunction.Visitor, TReturn> { + TConsumer extends RecursiveDirectoryTraversalFunction.PackageDirectoryConsumer, TReturn> { private final ProcessPackageDirectory processPackageDirectory; @@ -61,12 +61,12 @@ abstract class RecursiveDirectoryTraversalFunction< } /** - * Called by {@link #visitDirectory}, which will next call {@link Visitor#visitPackageValue} if - * the {@code recursivePkgKey} specifies a directory with a package, and which will lastly be - * provided to {@link #aggregateWithSubdirectorySkyValues} to compute the {@code TReturn} value - * returned by {@link #visitDirectory}. + * Called by {@link #visitDirectory}, which will next call {@link + * PackageDirectoryConsumer#notePackage} if the {@code recursivePkgKey} specifies a directory with + * a package, and which will lastly be provided to {@link #aggregateWithSubdirectorySkyValues} to + * compute the {@code TReturn} value returned by {@link #visitDirectory}. */ - protected abstract TVisitor getInitialVisitor(); + protected abstract TConsumer getInitialConsumer(); /** * Called by {@link #visitDirectory} to get the {@link SkyKey}s associated with recursive @@ -80,29 +80,22 @@ abstract class RecursiveDirectoryTraversalFunction< /** * Called by {@link #visitDirectory} to compute the {@code TReturn} value it returns, as a - * function of {@code visitor} and the {@link SkyValue}s computed for subdirectories - * of the directory specified by {@code recursivePkgKey}, contained in - * {@code subdirectorySkyValues}. + * function of {@code consumer} and the {@link SkyValue}s computed for subdirectories of the + * directory specified by {@code recursivePkgKey}, contained in {@code subdirectorySkyValues}. */ protected abstract TReturn aggregateWithSubdirectorySkyValues( - TVisitor visitor, Map<SkyKey, SkyValue> subdirectorySkyValues); + TConsumer consumer, Map<SkyKey, SkyValue> subdirectorySkyValues); /** - * A type of value used by {@link #visitDirectory} as it checks for a package in the directory - * specified by {@code recursivePkgKey}; if such a package exists, {@link #visitPackageValue} - * is called. + * A type of consumer used by {@link #visitDirectory} as it checks for a package in the directory + * specified by {@code recursivePkgKey}; if such a package exists, {@link #notePackage} is called. * - * <p>The value is then provided to {@link #aggregateWithSubdirectorySkyValues} to compute the + * <p>The consumer is then provided to {@link #aggregateWithSubdirectorySkyValues} to compute the * value returned by {@link #visitDirectory}. */ - interface Visitor { - - /** - * Called iff the directory contains a package. Provides an {@link Environment} {@code env} so - * that the visitor may do additional lookups. {@link Environment#valuesMissing} will be checked - * afterwards. - */ - void visitPackageValue(Package pkg, Environment env) throws InterruptedException; + interface PackageDirectoryConsumer { + /** Called iff the directory contains a package. */ + void notePackage(PathFragment pkgPath) throws InterruptedException; /** * Called iff the directory contains a BUILD file but *not* a package, which can happen under @@ -114,19 +107,19 @@ abstract class RecursiveDirectoryTraversalFunction< * cycle * </ol> */ - void visitPackageError(NoSuchPackageException e, Environment env) throws InterruptedException; + void notePackageError(NoSuchPackageException e); } /** * Looks in the directory specified by {@code recursivePkgKey} for a package, does some work as - * specified by {@link Visitor} if such a package exists, then recursively does work in each - * non-excluded subdirectory as specified by {@link #getSkyKeyForSubdirectory}, and finally - * aggregates the {@link Visitor} value along with values from each subdirectory as specified by - * {@link #aggregateWithSubdirectorySkyValues}, and returns that aggregation. + * specified by {@link PackageDirectoryConsumer} if such a package exists, then recursively does + * work in each non-excluded subdirectory as specified by {@link #getSkyKeyForSubdirectory}, and + * finally aggregates the {@link PackageDirectoryConsumer} value along with values from each + * subdirectory as specified by {@link #aggregateWithSubdirectorySkyValues}, and returns that + * aggregation. * * <p>Returns null if {@code env.valuesMissing()} is true, checked after each call to one of * {@link RecursiveDirectoryTraversalFunction}'s abstract methods that were given {@code env}. - * (And after each of {@code visitDirectory}'s own uses of {@code env}, of course.) */ TReturn visitDirectory(RecursivePkgKey recursivePkgKey, Environment env) throws InterruptedException { @@ -140,7 +133,7 @@ abstract class RecursiveDirectoryTraversalFunction< Iterable<SkyKey> childDeps = packageExistenceAndSubdirDeps.getChildDeps(); - TVisitor visitor = getInitialVisitor(); + TConsumer consumer = getInitialConsumer(); Map<SkyKey, SkyValue> subdirectorySkyValues; if (packageExistenceAndSubdirDeps.packageExists()) { @@ -155,28 +148,22 @@ abstract class RecursiveDirectoryTraversalFunction< if (env.valuesMissing()) { return null; } - Package pkg = null; try { PackageValue pkgValue = (PackageValue) dependentSkyValues.get(packageKey).get(); if (pkgValue == null) { return null; } - pkg = pkgValue.getPackage(); + Package pkg = pkgValue.getPackage(); if (pkg.containsErrors()) { env.getListener() .handle(Event.error("package contains errors: " + rootRelativePath.getPathString())); } + consumer.notePackage(rootRelativePath); } 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(e.getMessage())); - visitor.visitPackageError(e, env); - if (env.valuesMissing()) { - return null; - } - } - if (pkg != null) { - visitor.visitPackageValue(pkg, env); + consumer.notePackageError(e); if (env.valuesMissing()) { return null; } @@ -198,6 +185,6 @@ abstract class RecursiveDirectoryTraversalFunction< if (env.valuesMissing()) { return null; } - return aggregateWithSubdirectorySkyValues(visitor, subdirectorySkyValues); + return aggregateWithSubdirectorySkyValues(consumer, subdirectorySkyValues); } } 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 58f79be753..8430c286a5 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 @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; 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; import com.google.devtools.build.lib.vfs.RootedPath; @@ -51,15 +50,15 @@ public class RecursivePkgFunction implements SkyFunction { } private class MyTraversalFunction - extends RecursiveDirectoryTraversalFunction<MyVisitor, RecursivePkgValue> { + extends RecursiveDirectoryTraversalFunction<MyPackageDirectoryConsumer, RecursivePkgValue> { private MyTraversalFunction() { super(directories); } @Override - protected MyVisitor getInitialVisitor() { - return new MyVisitor(); + protected MyPackageDirectoryConsumer getInitialConsumer() { + return new MyPackageDirectoryConsumer(); } @Override @@ -70,28 +69,28 @@ public class RecursivePkgFunction implements SkyFunction { } @Override - protected RecursivePkgValue aggregateWithSubdirectorySkyValues(MyVisitor visitor, - Map<SkyKey, SkyValue> subdirectorySkyValues) { + protected RecursivePkgValue aggregateWithSubdirectorySkyValues( + MyPackageDirectoryConsumer consumer, Map<SkyKey, SkyValue> subdirectorySkyValues) { // Aggregate the transitive subpackages. for (SkyValue childValue : subdirectorySkyValues.values()) { - visitor.addTransitivePackages(((RecursivePkgValue) childValue).getPackages()); + consumer.addTransitivePackages(((RecursivePkgValue) childValue).getPackages()); } - return visitor.createRecursivePkgValue(); + return consumer.createRecursivePkgValue(); } } - private static class MyVisitor implements RecursiveDirectoryTraversalFunction.Visitor { + private static class MyPackageDirectoryConsumer + implements RecursiveDirectoryTraversalFunction.PackageDirectoryConsumer { private final NestedSetBuilder<String> packages = new NestedSetBuilder<>(Order.STABLE_ORDER); @Override - public void visitPackageValue(Package pkg, Environment env) { - packages.add(pkg.getName()); + public void notePackage(PathFragment pkgPath) { + packages.add(pkgPath.getPathString()); } @Override - public void visitPackageError(NoSuchPackageException e, Environment env) - throws InterruptedException { + public void notePackageError(NoSuchPackageException e) { // Nothing to do because the RecursiveDirectoryTraversalFunction has already emitted an error // event. } |