Adds the text output parameter and a partial manifest to the action key for ApkManifestAction, so that the key correctly changes when the structure of the manifest changes.
--
MOS_MIGRATED_REVID=121058425
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
index d78f976..a378988 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
@@ -16,10 +16,10 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
+import com.google.common.primitives.Ints;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.collect.CollectionUtils;
@@ -105,10 +105,15 @@
}
@Override
- public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) throws IOException {
+ public DeterministicWriter newDeterministicWriter(final ActionExecutionContext ctx)
+ throws IOException {
- ApkManifestCreator manifestCreator = new ApkManifestCreator(
- ctx.getMetadataHandler());
+ ApkManifestCreator manifestCreator = new ApkManifestCreator(new ArtifactDigester() {
+ @Override
+ public byte[] getDigest(Artifact artifact) throws IOException {
+ return ctx.getMetadataHandler().getMetadata(artifact).digest;
+ }
+ });
final ApkManifest manifest = manifestCreator.createManifest();
@@ -126,17 +131,45 @@
@Override
protected String computeKey() {
+
+ // Use fake hashes for the purposes of the action's key, because the hashes are retrieved from
+ // the MetadataHandler, which is available at only action-execution time. This should be ok
+ // because if an input artifact changes (and hence its hash changes), the action should be rerun
+ // anyway. This is more for the purpose of putting the structure of the output data into the
+ // key.
+ ApkManifestCreator manifestCreator = new ApkManifestCreator(new ArtifactDigester() {
+ @Override
+ public byte[] getDigest(Artifact artifact) {
+ return Ints.toByteArray(artifact.getExecPathString().hashCode());
+ }
+ });
+
+ ApkManifest manifest;
+ try {
+ manifest = manifestCreator.createManifest();
+ } catch (IOException e) {
+ // The ArtifactDigester shouldn't actually throw IOException, that's just for the
+ // ArtifactDigester that uses the MetadataHandler.
+ throw new IllegalStateException(e);
+ }
+
return new Fingerprint()
.addString(GUID)
+ .addBoolean(textOutput)
+ .addBytes(manifest.toByteArray())
.hexDigestAndReset();
}
+ private interface ArtifactDigester {
+ byte[] getDigest(Artifact artifact) throws IOException;
+ }
+
private class ApkManifestCreator {
- private final MetadataHandler metadataHandler;
+ private final ArtifactDigester artifactDigester;
- private ApkManifestCreator(MetadataHandler metadataHandler) {
- this.metadataHandler = metadataHandler;
+ private ApkManifestCreator(ArtifactDigester artifactDigester) {
+ this.artifactDigester = artifactDigester;
}
private ApkManifest createManifest() throws IOException {
@@ -171,9 +204,9 @@
}
return protoArtifacts.build();
}
-
+
private ApkManifestOuterClass.Artifact makeArtifactProto(Artifact artifact) throws IOException {
- byte[] digest = metadataHandler.getMetadata(artifact).digest;
+ byte[] digest = artifactDigester.getDigest(artifact);
return ApkManifestOuterClass.Artifact.newBuilder()
.setExecRootPath(artifact.getExecPathString())
.setHash(ByteString.copyFrom(digest))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
index dd0df22..3790799 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.android;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -108,9 +109,11 @@
// Map from architecture (CPU folder to place the library in) to libraries for that CPU
private final ImmutableMap<String, Iterable<Artifact>> nativeLibs;
- private final Artifact nativeLibsName;
+ @Nullable private final Artifact nativeLibsName;
- private NativeLibs(ImmutableMap<String, Iterable<Artifact>> nativeLibs, Artifact nativeLibsName) {
+ @VisibleForTesting
+ NativeLibs(ImmutableMap<String, Iterable<Artifact>> nativeLibs,
+ @Nullable Artifact nativeLibsName) {
this.nativeLibs = nativeLibs;
this.nativeLibsName = nativeLibsName;
}
