Fix AndroidBinaryRule.Builder so that build() can be invoked multiple times.
Summary:
A previous diff introduced a change to `AndroidBinaryRule.Builder` such that invoking
`build()` would leave around some state (only if pre-dexing was applied) that
would cause a problem if `build()` were invoked again on the same `Builder` object.
When `buckd` is not running, `build()` is only invoked once, so this went
undetected in the original diff.
diff --git a/src/com/facebook/buck/android/AndroidBinaryRule.java b/src/com/facebook/buck/android/AndroidBinaryRule.java
index 826fabd..401dc5c 100644
--- a/src/com/facebook/buck/android/AndroidBinaryRule.java
+++ b/src/com/facebook/buck/android/AndroidBinaryRule.java
@@ -1235,29 +1235,44 @@
BuildRuleParams originalParams = createBuildRuleParams(ruleResolver);
ImmutableSortedSet<BuildRule> originalDeps = originalParams.getDeps();
+ ImmutableSet<BuildRule> preDexDepsAsBuildRules;
ImmutableSet<DexProducedFromJavaLibraryThatContainsClassFiles> preDexDeps;
if (!disablePreDex
&& PackageType.DEBUG.equals(packageType)
&& !dexSplitMode.isShouldSplitDex() // TODO(mbolin): Support predex for split dex.
&& !preprocessJavaClassesBash.isPresent() // TODO(mbolin): Support predex post-preprocess.
) {
- preDexDeps = enhanceGraphToLeveragePreDexing(originalDeps,
+ preDexDepsAsBuildRules = enhanceGraphToLeveragePreDexing(originalDeps,
buildRulesToExcludeFromDex,
ruleResolver,
originalParams.getPathRelativizer(),
originalParams.getRuleKeyBuilderFactory());
- for (DexProducedFromJavaLibraryThatContainsClassFiles preDexDep : preDexDeps) {
- addDep(preDexDep.getBuildTarget());
+ ImmutableSet.Builder<DexProducedFromJavaLibraryThatContainsClassFiles> preDexDepsBuilder =
+ ImmutableSet.builder();
+ for (BuildRule preDexDepBuildRule : preDexDepsAsBuildRules) {
+ preDexDepsBuilder.add(
+ (DexProducedFromJavaLibraryThatContainsClassFiles) preDexDepBuildRule.getBuildable());
}
+ preDexDeps = preDexDepsBuilder.build();
} else {
+ preDexDepsAsBuildRules = ImmutableSet.of();
preDexDeps = ImmutableSortedSet.of();
}
boolean allowNonExistentRule =
false;
- // Must invoke this a second time, as the preDexDeps may have been added to the builder.
- BuildRuleParams finalParams = createBuildRuleParams(ruleResolver);
+ // Must create a new BuildRuleParams to supersede the one built by
+ // createBuildRuleParams(ruleResolver).
+ ImmutableSortedSet<BuildRule> totalDeps = ImmutableSortedSet.<BuildRule>naturalOrder()
+ .addAll(originalDeps)
+ .addAll(preDexDepsAsBuildRules)
+ .build();
+ BuildRuleParams finalParams = new BuildRuleParams(getBuildTarget(),
+ totalDeps,
+ originalParams.getVisibilityPatterns(),
+ originalParams.getPathRelativizer(),
+ originalParams.getRuleKeyBuilderFactory());
return new AndroidBinaryRule(
finalParams,
@@ -1285,17 +1300,17 @@
/**
* @return The set of build rules that correspond to pre-dex'd artifacts that should be merged
- * to create the final classes.dex for the APK.
+ * to create the final classes.dex for the APK. For every element in the set, its
+ * {@link BuildRule#getBuildable()} method will return an instance of
+ * {@link DexProducedFromJavaLibraryThatContainsClassFiles}.
*/
- private ImmutableSet<DexProducedFromJavaLibraryThatContainsClassFiles>
- enhanceGraphToLeveragePreDexing(
+ private ImmutableSet<BuildRule> enhanceGraphToLeveragePreDexing(
ImmutableSortedSet<BuildRule> originalDeps,
Set<BuildTarget> buildRulesToExcludeFromDex,
BuildRuleResolver ruleResolver,
Function<String, Path> pathRelativizer,
RuleKeyBuilderFactory ruleKeyBuilderFactory) {
- ImmutableSet.Builder<DexProducedFromJavaLibraryThatContainsClassFiles> preDexDeps =
- ImmutableSet.builder();
+ ImmutableSet.Builder<BuildRule> preDexDeps = ImmutableSet.builder();
ImmutableSet<JavaLibraryRule> transitiveJavaDeps = Classpaths
.getClasspathEntries(originalDeps).keySet();
for (JavaLibraryRule javaLibraryRule : transitiveJavaDeps) {
@@ -1317,8 +1332,7 @@
"dex");
BuildRule preDexRule = ruleResolver.get(preDexTarget);
if (preDexRule != null) {
- preDexDeps.add(
- (DexProducedFromJavaLibraryThatContainsClassFiles) preDexRule.getBuildable());
+ preDexDeps.add(preDexRule);
continue;
}
@@ -1348,7 +1362,7 @@
preDexBuilder.addDep(accumulateClassNamesBuildTarget);
preDexBuilder.addVisibilityPattern(BuildTargetPattern.MATCH_ALL);
BuildRule preDex = ruleResolver.buildAndAddToIndex(preDexBuilder);
- preDexDeps.add((DexProducedFromJavaLibraryThatContainsClassFiles) preDex.getBuildable());
+ preDexDeps.add(preDex);
}
return preDexDeps.build();
}
diff --git a/src/com/facebook/buck/rules/AbstractBuildRuleBuilder.java b/src/com/facebook/buck/rules/AbstractBuildRuleBuilder.java
index f51f13e..f07c96a 100644
--- a/src/com/facebook/buck/rules/AbstractBuildRuleBuilder.java
+++ b/src/com/facebook/buck/rules/AbstractBuildRuleBuilder.java
@@ -46,6 +46,10 @@
this.ruleKeyBuilderFactory = params.getRuleKeyBuilderFactory();
}
+ /**
+ * This method must support being able to be invoked multiple times, as this builder will be
+ * reused when buckd is being used.
+ */
@Override
public abstract T build(BuildRuleResolver ruleResolver);
