Include the transitive deps when computing the ABI key of a dep with export_deps=True.
Summary: When computing the ABI key of a dependency with export_deps=True,
the ABI keys for the transitive dependencies will be hashed in the final ABI key.
Test Plan: Added new test: DefaultJavaLibraryRuleTest.testGetAbiKeyInThePresenceOfExportDeps().
diff --git a/src/com/facebook/buck/java/DefaultJavaLibraryRule.java b/src/com/facebook/buck/java/DefaultJavaLibraryRule.java
index 5e87343..1e45507 100644
--- a/src/com/facebook/buck/java/DefaultJavaLibraryRule.java
+++ b/src/com/facebook/buck/java/DefaultJavaLibraryRule.java
@@ -309,15 +309,40 @@
setAbiKey(Suppliers.memoize(new Supplier<Sha1HashCode>() {
@Override
public Sha1HashCode get() {
- return javac.getAbiKey();
+ return createTotalAbiKey(javac.getAbiKey());
}
}));
} else {
// When there are no .java files to compile, the ABI key should be a constant.
- setAbiKey(Suppliers.ofInstance(new Sha1HashCode(AbiWriterProtocol.EMPTY_ABI_KEY)));
+ setAbiKey(Suppliers.ofInstance(createTotalAbiKey(
+ new Sha1HashCode(AbiWriterProtocol.EMPTY_ABI_KEY))));
}
}
+ /**
+ * Creates the total ABI key for this rule. If export_deps is true, the total key is computed by
+ * hashing the ABI keys of the dependencies together with the ABI key of this rule. If export_deps
+ * is false, the standalone ABI key for this rule is used as the total key.
+ * @param abiKey the standalone ABI key for this rule.
+ * @return total ABI key containing also the ABI keys of the dependencies.
+ */
+ protected Sha1HashCode createTotalAbiKey(Sha1HashCode abiKey) {
+ if (!getExportDeps()) {
+ return abiKey;
+ }
+
+ SortedSet<JavaLibraryRule> depsForAbiKey = getDepsForAbiKey();
+ // If there are no deps to consider, just return the ABI Key for this rule.
+ if (depsForAbiKey.isEmpty()) {
+ return abiKey;
+ }
+
+ // Hash the ABI keys of all dependencies together with ABI key for the current rule.
+ Hasher hasher = createHasherWithAbiKeyForDeps(depsForAbiKey);
+ hasher.putUnencodedChars(abiKey.getHash());
+ return new Sha1HashCode(hasher.hash().toString());
+ }
+
private String getPathToAbiOutputDir() {
BuildTarget target = getBuildTarget();
return String.format(
@@ -359,11 +384,18 @@
}
/**
- * Finds all deps that implement JavaLibraryRule and hash their ABI keys together. If any dep
- * lacks an ABI key, then returns {@link Optional#absent()}.
+ * Finds all deps that implement JavaLibraryRule and hash their ABI keys together.
*/
@Override
public Sha1HashCode getAbiKeyForDeps() {
+ return new Sha1HashCode(createHasherWithAbiKeyForDeps(getDepsForAbiKey()).hash().toString());
+ }
+
+ /**
+ * Returns a sorted set containing the dependencies which will be hashed in the final ABI key.
+ * @return the dependencies to be hashed in the final ABI key.
+ */
+ private SortedSet<JavaLibraryRule> getDepsForAbiKey() {
SortedSet<JavaLibraryRule> rulesWithAbiToConsider = Sets.newTreeSet();
for (BuildRule dep : getDeps()) {
if (dep instanceof JavaLibraryRule) {
@@ -371,7 +403,16 @@
rulesWithAbiToConsider.addAll(javaRule.getOutputClasspathEntries().keys());
}
}
+ return rulesWithAbiToConsider;
+ }
+ /**
+ * Creates a Hasher containing the ABI keys of the dependencies.
+ * @param rulesWithAbiToConsider a sorted set containing the dependencies whose ABI key will be
+ * added to the hasher.
+ * @return a Hasher containing the ABI keys of the dependencies.
+ */
+ private Hasher createHasherWithAbiKeyForDeps(SortedSet<JavaLibraryRule> rulesWithAbiToConsider) {
Hasher hasher = Hashing.sha1().newHasher();
for (JavaLibraryRule ruleWithAbiToConsider : rulesWithAbiToConsider) {
if (ruleWithAbiToConsider == this) {
@@ -382,7 +423,7 @@
hasher.putUnencodedChars(abiKey.getHash());
}
- return new Sha1HashCode(hasher.hash().toString());
+ return hasher;
}
@Override
diff --git a/test/com/facebook/buck/java/DefaultJavaLibraryRuleTest.java b/test/com/facebook/buck/java/DefaultJavaLibraryRuleTest.java
index e2414fe..ec2dee2 100644
--- a/test/com/facebook/buck/java/DefaultJavaLibraryRuleTest.java
+++ b/test/com/facebook/buck/java/DefaultJavaLibraryRuleTest.java
@@ -683,7 +683,7 @@
consumerNoExport.getAbiKeyForDeps(),
not(equalTo(consumerWithExport.getAbiKeyForDeps())));
String expectedAbiKeyNoDepsHashForConsumerWithExport = Hashing.sha1().newHasher()
- .putUnencodedChars(commonWithExportAbiKeyHash)
+ .putUnencodedChars(commonWithExport.getAbiKey().getHash())
.putUnencodedChars(tinyLibAbiKeyHash)
.hash()
.toString();
@@ -695,8 +695,201 @@
observedAbiKeyNoDepsHashForConsumerWithExport);
}
+ /**
+ * @see DefaultJavaLibraryRule#getAbiKey()
+ */
+ @Test
+ public void testGetAbiKeyInThePresenceOfExportDeps() throws IOException {
+ // Create a java_library named //:commonlib with a hardcoded ABI key.
+ String commonLibAbiKeyHash = Strings.repeat("a", 40);
+ JavaLibraryRule commonLibrary = createDefaultJavaLibaryRuleWithAbiKey(
+ commonLibAbiKeyHash,
+ BuildTargetFactory.newInstance("//:commonlib"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(),
+ /* exportDeps */ false);
+
+ // Create two java_library rules, each of which depends on //:commonlib, but only one of which
+ // exports its deps.
+ String libWithExportAbiKeyHash = Strings.repeat("b", 40);
+ DefaultJavaLibraryRule libWithExport = createDefaultJavaLibaryRuleWithAbiKey(
+ libWithExportAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_with_export"),
+ /* srcs */ ImmutableSet.<String>of(),
+ /* deps */ ImmutableSet.<BuildRule>of(commonLibrary),
+ /* exportDeps */ true);
+ String libNoExportAbiKeyHash = Strings.repeat("c", 40);
+ DefaultJavaLibraryRule libNoExport = createDefaultJavaLibaryRuleWithAbiKey(
+ /* abiHash */ libNoExportAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_no_export"),
+ /* srcs */ ImmutableSet.<String>of(),
+ /* deps */ ImmutableSet.<BuildRule>of(commonLibrary),
+ /* exportDeps */ false);
+
+ // Verify getAbiKey() for the two //:lib_XXX rules.
+ String expectedLibWithExportAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(commonLibAbiKeyHash)
+ .putUnencodedChars(libWithExportAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "getAbiKey() should include the dependencies' ABI keys for the rule with export_deps=true.",
+ expectedLibWithExportAbiKeyHash,
+ libWithExport.getAbiKey().getHash());
+
+ assertEquals(
+ "getAbiKey() should not include the dependencies' ABI keys for the rule with export_deps=false.",
+ libNoExportAbiKeyHash,
+ libNoExport.getAbiKey().getHash());
+ }
+
+ /**
+ * @see DefaultJavaLibraryRule#getAbiKey()
+ */
+ @Test
+ public void testGetAbiKeyRecursiveExportDeps() throws IOException {
+ String libAAbiKeyHash = Strings.repeat("a", 40);
+ String libBAbiKeyHash = Strings.repeat("b", 40);
+ String libCAbiKeyHash = Strings.repeat("c", 40);
+ String libDAbiKeyHash = Strings.repeat("d", 40);
+
+ // Test the following dependency graph:
+ // a(export_deps=true) -> b(export_deps=true) -> c(export_deps=true) -> d(export_deps=true)
+ JavaLibraryRule libD = createDefaultJavaLibaryRuleWithAbiKey(
+ libDAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_d"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(),
+ /* exportDeps */ true);
+ JavaLibraryRule libC = createDefaultJavaLibaryRuleWithAbiKey(
+ libCAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_c"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libD),
+ /* exportDeps */ true);
+ JavaLibraryRule libB = createDefaultJavaLibaryRuleWithAbiKey(
+ libBAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_b"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libC),
+ /* exportDeps */ true);
+ JavaLibraryRule libA = createDefaultJavaLibaryRuleWithAbiKey(
+ libAAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_a"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libB),
+ /* exportDeps */ true);
+
+ assertEquals(
+ "If a rule has no dependencies its final ABI key should be the rule's own ABI key.",
+ libDAbiKeyHash,
+ libD.getAbiKey().getHash());
+
+ String expectedLibCAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(libDAbiKeyHash)
+ .putUnencodedChars(libCAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "The ABI key for lib_c should contain lib_d's ABI key.",
+ expectedLibCAbiKeyHash,
+ libC.getAbiKey().getHash());
+
+ String expectedLibBAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(expectedLibCAbiKeyHash)
+ .putUnencodedChars(libDAbiKeyHash)
+ .putUnencodedChars(libBAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "The ABI key for lib_b should contain lib_c's and lib_d's ABI keys.",
+ expectedLibBAbiKeyHash,
+ libB.getAbiKey().getHash());
+
+ String expectedLibAAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(expectedLibBAbiKeyHash)
+ .putUnencodedChars(expectedLibCAbiKeyHash)
+ .putUnencodedChars(libDAbiKeyHash)
+ .putUnencodedChars(libAAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "The ABI key for lib_a should contain lib_b's, lib_c's and lib_d's ABI keys.",
+ expectedLibAAbiKeyHash,
+ libA.getAbiKey().getHash());
+
+ // Test the following dependency graph:
+ // a(export_deps=true) -> b(export_deps=true) -> c(export_deps=false) -> d(export_deps=false)
+ libD = createDefaultJavaLibaryRuleWithAbiKey(
+ libDAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_d2"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(),
+ /* exportDeps */ false);
+ libC = createDefaultJavaLibaryRuleWithAbiKey(
+ libCAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_c2"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libD),
+ /* exportDeps */ false);
+ libB = createDefaultJavaLibaryRuleWithAbiKey(
+ libBAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_b2"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libC),
+ /* exportDeps */ true);
+ libA = createDefaultJavaLibaryRuleWithAbiKey(
+ libAAbiKeyHash,
+ BuildTargetFactory.newInstance("//:lib_a2"),
+ // Must have a source file or else its ABI will be AbiWriterProtocol.EMPTY_ABI_KEY.
+ /* srcs */ ImmutableSet.of("foo/Bar.java"),
+ /* deps */ ImmutableSet.<BuildRule>of(libB),
+ /* exportDeps */ true);
+
+ assertEquals(
+ "If export_deps is false, the final ABI key should be the rule's own ABI key.",
+ libDAbiKeyHash,
+ libD.getAbiKey().getHash());
+
+ assertEquals(
+ "If export_deps is false, the final ABI key should be the rule's own ABI key.",
+ libCAbiKeyHash,
+ libC.getAbiKey().getHash());
+
+ expectedLibBAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(libCAbiKeyHash)
+ .putUnencodedChars(libBAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "The ABI key for lib_b should contain lib_c's ABI key.",
+ expectedLibBAbiKeyHash,
+ libB.getAbiKey().getHash());
+
+ expectedLibAAbiKeyHash = Hashing.sha1().newHasher()
+ .putUnencodedChars(expectedLibBAbiKeyHash)
+ .putUnencodedChars(libCAbiKeyHash)
+ .putUnencodedChars(libDAbiKeyHash)
+ .putUnencodedChars(libAAbiKeyHash)
+ .hash()
+ .toString();
+ assertEquals(
+ "The ABI key for lib_a should contain lib_b's, lib_c's and lib_d's ABI keys.",
+ expectedLibAAbiKeyHash,
+ libA.getAbiKey().getHash());
+
+ }
+
private static DefaultJavaLibraryRule createDefaultJavaLibaryRuleWithAbiKey(
- @Nullable final String abiHash,
+ @Nullable final String partialAbiHash,
BuildTarget buildTarget,
ImmutableSet<String> srcs,
ImmutableSet<BuildRule> deps,
@@ -711,10 +904,10 @@
) {
@Override
public Sha1HashCode getAbiKey() {
- if (abiHash == null) {
+ if (partialAbiHash == null) {
return super.getAbiKey();
} else {
- return new Sha1HashCode(abiHash);
+ return createTotalAbiKey(new Sha1HashCode(partialAbiHash));
}
}
};
