Allow the visitor to visit a subset of its children instead of all of them.
Test Plan: Unit tests.
diff --git a/src/com/facebook/buck/android/AndroidTransitiveDependencyGraph.java b/src/com/facebook/buck/android/AndroidTransitiveDependencyGraph.java
index 4ca46c3..f8127ab 100644
--- a/src/com/facebook/buck/android/AndroidTransitiveDependencyGraph.java
+++ b/src/com/facebook/buck/android/AndroidTransitiveDependencyGraph.java
@@ -99,7 +99,7 @@
// Visit all of the transitive dependencies to populate the above collections.
new AbstractDependencyVisitor(rulesToTraverseForTransitiveDeps.get()) {
@Override
- public boolean visit(BuildRule rule) {
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
// We need to include the transitive closure of the compiled .class files when dex'ing, as
// well as the third-party jars that they depend on.
// Update pathsToThirdPartyJars.
@@ -107,7 +107,7 @@
PrebuiltJarRule prebuiltJarRule = (PrebuiltJarRule) rule;
pathsToThirdPartyJarsBuilder.add(prebuiltJarRule.getBinaryJar());
}
- return rule.getProperties().is(LIBRARY);
+ return maybeVisitAllDeps(rule, rule.getProperties().is(LIBRARY));
}
}.start();
@@ -161,7 +161,7 @@
// Visit all of the transitive dependencies to populate the above collections.
new AbstractDependencyVisitor(rulesToTraverseForTransitiveDeps.get()) {
@Override
- public boolean visit(BuildRule rule) {
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
// We need to include the transitive closure of the compiled .class files when dex'ing, as
// well as the third-party jars that they depend on.
// Update pathsToThirdPartyJars.
@@ -176,7 +176,7 @@
// In the rare event that a PrebuiltNativeLibraryBuildRule has deps, it is likely another
// NativeLibraryRule that will need to be included in the final APK, so traverse the deps.
if (rule.getBuildable() instanceof PrebuiltNativeLibrary) {
- return true;
+ return rule.getDeps();
}
} else if (rule instanceof AndroidResourceRule) {
AndroidResourceRule androidRule = (AndroidResourceRule) rule;
@@ -197,7 +197,7 @@
Optionals.addIfPresent(androidLibraryRule.getManifestFile(), manifestFiles);
}
}
- return rule.getProperties().is(LIBRARY);
+ return maybeVisitAllDeps(rule, rule.getProperties().is(LIBRARY));
}
}.start();
diff --git a/src/com/facebook/buck/android/UberRDotJavaUtil.java b/src/com/facebook/buck/android/UberRDotJavaUtil.java
index 0d01ee2..7d82313 100644
--- a/src/com/facebook/buck/android/UberRDotJavaUtil.java
+++ b/src/com/facebook/buck/android/UberRDotJavaUtil.java
@@ -161,7 +161,7 @@
AbstractDependencyVisitor visitor = new AbstractDependencyVisitor(buildRule) {
@Override
- public boolean visit(BuildRule rule) {
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
if (rule instanceof HasAndroidResourceDeps) {
HasAndroidResourceDeps androidResourceRule = (HasAndroidResourceDeps)rule;
if (androidResourceRule.getRes() != null) {
@@ -171,7 +171,7 @@
// Only certain types of rules should be considered as part of this traversal.
BuildRuleType type = rule.getType();
- return TRAVERSABLE_TYPES.contains(type);
+ return maybeVisitAllDeps(rule, TRAVERSABLE_TYPES.contains(type));
}
};
diff --git a/src/com/facebook/buck/command/Project.java b/src/com/facebook/buck/command/Project.java
index bcdb904..1ae7754 100644
--- a/src/com/facebook/buck/command/Project.java
+++ b/src/com/facebook/buck/command/Project.java
@@ -736,7 +736,7 @@
private final LinkedHashSet<DependentModule> modulesToAdd = Sets.newLinkedHashSet();
@Override
- public boolean visit(BuildRule dep) {
+ public ImmutableSet<BuildRule> visit(BuildRule dep) {
boolean depShouldExportDeps = dep.getExportDeps() || rule.getProperties().is(PACKAGING);
boolean shouldVisitDeps = (dep.getProperties().is(LIBRARY) && (depShouldExportDeps))
|| (dep instanceof AndroidResourceRule)
@@ -786,7 +786,7 @@
String moduleName = getIntellijNameForRule(dep);
dependentModule = DependentModule.newModule(dep.getBuildTarget(), moduleName);
} else if (dep.getFullyQualifiedName().startsWith(ANDROID_GEN_BUILD_TARGET_PREFIX)) {
- return shouldVisitDeps;
+ return maybeVisitAllDeps(dep, shouldVisitDeps);
} else if (dep instanceof JavaLibraryRule) {
String moduleName = getIntellijNameForRule(dep);
dependentModule = DependentModule.newModule(dep.getBuildTarget(), moduleName);
@@ -795,9 +795,9 @@
dependentModule = DependentModule.newModule(dep.getBuildTarget(), moduleName);
} else if (dep.getBuildable() instanceof GenAidl) {
// This will likely be handled appropriately by the IDE's Android plugin.
- return shouldVisitDeps;
+ return maybeVisitAllDeps(dep, shouldVisitDeps);
} else {
- return shouldVisitDeps;
+ return maybeVisitAllDeps(dep, shouldVisitDeps);
}
if (isForTests) {
@@ -821,7 +821,7 @@
modulesToAdd.add(dependentModule);
}
- return shouldVisitDeps;
+ return maybeVisitAllDeps(dep, shouldVisitDeps);
}
@Override
diff --git a/src/com/facebook/buck/python/PythonBinaryRule.java b/src/com/facebook/buck/python/PythonBinaryRule.java
index a69972e..6a38113 100644
--- a/src/com/facebook/buck/python/PythonBinaryRule.java
+++ b/src/com/facebook/buck/python/PythonBinaryRule.java
@@ -88,19 +88,19 @@
new AbstractDependencyVisitor(this) {
@Override
- public boolean visit(BuildRule rule) {
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
Buildable buildable = rule.getBuildable();
if (buildable instanceof PythonLibrary) {
PythonLibrary pythonLibrary = (PythonLibrary) buildable;
Path pythonPathEntry = pythonLibrary.getPythonPathDirectory();
entries.add(pythonPathEntry);
- return true;
+ return getDeps();
}
// AbstractDependencyVisitor will start from this (PythonBinaryRule) so make sure it
// descends to its dependencies even though it is not a library rule.
- return rule == pythonBinaryRule;
+ return maybeVisitAllDeps(rule, rule == pythonBinaryRule);
}
}.start();
diff --git a/src/com/facebook/buck/rules/AbstractDependencyVisitor.java b/src/com/facebook/buck/rules/AbstractDependencyVisitor.java
index cc93661..95f744b 100644
--- a/src/com/facebook/buck/rules/AbstractDependencyVisitor.java
+++ b/src/com/facebook/buck/rules/AbstractDependencyVisitor.java
@@ -16,6 +16,7 @@
package com.facebook.buck.rules;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
@@ -53,14 +54,17 @@
continue;
}
- boolean shouldVisitDeps = visit(currentRule);
+ ImmutableSet<BuildRule> depsToVisit = visit(currentRule);
explored.add(currentRule);
- if (shouldVisitDeps) {
- for (BuildRule dep : currentRule.getDeps()) {
- if (!explored.contains(dep)) {
- toExplore.add(dep);
- }
+ for (BuildRule dep : depsToVisit) {
+ Preconditions.checkState(currentRule.getDeps().contains(dep),
+ "%s said that it should visit %s, but %s is not in its deps.",
+ currentRule,
+ dep,
+ dep);
+ if (!explored.contains(dep)) {
+ toExplore.add(dep);
}
}
}
@@ -74,8 +78,16 @@
}
/**
+ * To perform a full traversal of the {@code rule}'s transitive dependencies, this rule
+ * should return {@code rule.getDeps()}.
+ *
* @param rule Visited build rule
- * @return whether to traverse the deps of the current rule
+ * @return The set of children to visit after visiting this node. This set must be a subset of
+ * {@code rule.getDeps()}
*/
- public abstract boolean visit(BuildRule rule);
+ public abstract ImmutableSet<BuildRule> visit(BuildRule rule);
+
+ public static ImmutableSet<BuildRule> maybeVisitAllDeps(BuildRule rule, boolean visitDeps) {
+ return visitDeps ? rule.getDeps() : ImmutableSet.<BuildRule>of();
+ }
}
diff --git a/test/com/facebook/buck/rules/AbstractDependencyVisitorTest.java b/test/com/facebook/buck/rules/AbstractDependencyVisitorTest.java
index d8f2b29..09b9477 100644
--- a/test/com/facebook/buck/rules/AbstractDependencyVisitorTest.java
+++ b/test/com/facebook/buck/rules/AbstractDependencyVisitorTest.java
@@ -21,9 +21,11 @@
import com.facebook.buck.java.FakeJavaLibraryRule;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.model.BuildTargetPattern;
+import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import org.junit.Test;
@@ -60,9 +62,9 @@
final List<BuildRule> buildRuleTraversalOrder = Lists.newArrayList();
new AbstractDependencyVisitor(initialRule) {
@Override
- public boolean visit(BuildRule rule) {
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
buildRuleTraversalOrder.add(rule);
- return true;
+ return rule.getDeps();
}
}.start();
@@ -81,6 +83,56 @@
buildRuleTraversalOrder);
}
+ @Test
+ public void testSubsetWorks() {
+ // The dependency graph of build rules being built up is as follows:
+ //
+ // 10
+ // / / \ \
+ // 6 7 8 9
+ // / \ / \ \
+ // 5 4 3 2 1
+ //
+ BuildRule rule1 = createRule("1");
+ BuildRule rule2 = createRule("2");
+ BuildRule rule3 = createRule("3");
+ BuildRule rule4 = createRule("4");
+ BuildRule rule5 = createRule("5");
+
+ BuildRule rule6 = createRule("6", rule5, rule4);
+ BuildRule rule7 = createRule("7");
+ BuildRule rule8 = createRule("8", rule3, rule2);
+ BuildRule rule9 = createRule("9", rule1);
+
+ BuildRule initialRule = createRule("10", rule6, rule7, rule8, rule9);
+
+ final List<BuildRule> buildRuleTraversalOrder = Lists.newArrayList();
+
+ // This visitor only visits dependencies whose node names are even numbers.
+ new AbstractDependencyVisitor(initialRule) {
+ @Override
+ public ImmutableSet<BuildRule> visit(BuildRule rule) {
+ buildRuleTraversalOrder.add(rule);
+ return ImmutableSet.copyOf(Iterables.filter(rule.getDeps(), new Predicate<BuildRule>() {
+ @Override
+ public boolean apply(BuildRule input) {
+ return Integer.parseInt(input.getBuildTarget().getShortName()) % 2 == 0;
+ }
+ }));
+ }
+ }.start();
+
+ assertEquals(
+ "Dependencies should be explored depth-first, only containing rules whose rule name is " +
+ "an even number",
+ ImmutableList.of(initialRule,
+ rule6,
+ rule8,
+ rule4,
+ rule2),
+ buildRuleTraversalOrder);
+ }
+
private FakeJavaLibraryRule createRule(String name, BuildRule... deps) {
ImmutableSet<BuildTargetPattern> visibilityPatterns = ImmutableSet.of();
FakeJavaLibraryRule rule = new FakeJavaLibraryRule(
