Introduce ArtifactCache#isStoreSupported().
Summary:
This makes it possible to avoid creating zip files unnecessarily
when caching is disabled. This occurs frequently in unit tests,
and likely occurs often in external usage of Buck.
Test Plan: Sandcastle builds.
diff --git a/src/com/facebook/buck/rules/ArtifactCache.java b/src/com/facebook/buck/rules/ArtifactCache.java
index 2586c05..b5864f9 100644
--- a/src/com/facebook/buck/rules/ArtifactCache.java
+++ b/src/com/facebook/buck/rules/ArtifactCache.java
@@ -33,9 +33,17 @@
* Store the artifact at path specified by output to cache, such that it can later be fetched
* using ruleKey as the lookup key. If any internal errors occur, fail silently and continue
* execution.
+ * <p>
+ * This is a noop if {@link #isStoreSupported()} returns {@code false}.
*
* @param ruleKey cache store key
* @param output path to read artifact from
*/
public void store(RuleKey ruleKey, File output);
+
+ /**
+ * This method must return the same value over the lifetime of this object.
+ * @return whether this{@link ArtifactCache} supports storing artifacts.
+ */
+ public boolean isStoreSupported();
}
diff --git a/src/com/facebook/buck/rules/BuildInfoRecorder.java b/src/com/facebook/buck/rules/BuildInfoRecorder.java
index 67e948b..0bf8530 100644
--- a/src/com/facebook/buck/rules/BuildInfoRecorder.java
+++ b/src/com/facebook/buck/rules/BuildInfoRecorder.java
@@ -99,8 +99,12 @@
* Creates a zip file of the metadata and recorded artifacts and stores it in the artifact cache.
*/
public void performUploadToArtifactCache(ArtifactCache artifactCache, BuckEventBus eventBus) {
- // TODO(mbolin): Skip all of this if caching is disabled. Although artifactCache.store() will be
- // a noop, building up the zip is wasted I/O.
+ // Skip all of this if caching is disabled. Although artifactCache.store() will be a noop,
+ // building up the zip is wasted I/O.
+ if (!artifactCache.isStoreSupported()) {
+ return;
+ }
+
ImmutableSet<Path> pathsToIncludeInZip = ImmutableSet.<Path>builder()
.addAll(Iterables.transform(metadataToWrite.keySet(),
new Function<String, Path>() {
diff --git a/src/com/facebook/buck/rules/CassandraArtifactCache.java b/src/com/facebook/buck/rules/CassandraArtifactCache.java
index 8d38375..7a42f65 100644
--- a/src/com/facebook/buck/rules/CassandraArtifactCache.java
+++ b/src/com/facebook/buck/rules/CassandraArtifactCache.java
@@ -246,7 +246,7 @@
@Override
public void store(RuleKey ruleKey, File output) {
- if (!doStore) {
+ if (!isStoreSupported()) {
return;
}
@@ -273,6 +273,11 @@
}
}
+ @Override
+ public boolean isStoreSupported() {
+ return doStore;
+ }
+
private void reportConnectionFailure(String context, ConnectionException exception) {
if (numConnectionExceptionReports.incrementAndGet() < MAX_CONNECTION_FAILURE_REPORTS) {
buckEventBus.post(ThrowableLogEvent.create(exception,
diff --git a/src/com/facebook/buck/rules/DirArtifactCache.java b/src/com/facebook/buck/rules/DirArtifactCache.java
index 10aa1a0..f41a60a 100644
--- a/src/com/facebook/buck/rules/DirArtifactCache.java
+++ b/src/com/facebook/buck/rules/DirArtifactCache.java
@@ -81,4 +81,10 @@
}
}
}
+
+ /** @return {@code true}: storing artifacts is always supported by this class. */
+ @Override
+ public boolean isStoreSupported() {
+ return true;
+ }
}
diff --git a/src/com/facebook/buck/rules/LoggingArtifactCacheDecorator.java b/src/com/facebook/buck/rules/LoggingArtifactCacheDecorator.java
index fe39a88..794239e 100644
--- a/src/com/facebook/buck/rules/LoggingArtifactCacheDecorator.java
+++ b/src/com/facebook/buck/rules/LoggingArtifactCacheDecorator.java
@@ -47,6 +47,11 @@
delegate.store(ruleKey, output);
eventBus.post(ArtifactCacheEvent.finished(ArtifactCacheEvent.Operation.STORE));
}
+
+ @Override
+ public boolean isStoreSupported() {
+ return delegate.isStoreSupported();
+ }
};
}
}
diff --git a/src/com/facebook/buck/rules/MultiArtifactCache.java b/src/com/facebook/buck/rules/MultiArtifactCache.java
index bf8c73d..eb60e93 100644
--- a/src/com/facebook/buck/rules/MultiArtifactCache.java
+++ b/src/com/facebook/buck/rules/MultiArtifactCache.java
@@ -28,9 +28,19 @@
*/
public class MultiArtifactCache implements ArtifactCache {
private final ImmutableList<ArtifactCache> artifactCaches;
+ private final boolean isStoreSupported;
public MultiArtifactCache(ImmutableList<ArtifactCache> artifactCaches) {
this.artifactCaches = Preconditions.checkNotNull(artifactCaches);
+
+ boolean isStoreSupported = false;
+ for (ArtifactCache artifactCache : artifactCaches) {
+ if (artifactCache.isStoreSupported()) {
+ isStoreSupported = true;
+ break;
+ }
+ }
+ this.isStoreSupported = isStoreSupported;
}
/**
@@ -66,4 +76,10 @@
artifactCache.store(ruleKey, output);
}
}
+
+ /** @return {@code true} if there is at least one ArtifactCache that supports storing. */
+ @Override
+ public boolean isStoreSupported() {
+ return isStoreSupported;
+ }
}
diff --git a/src/com/facebook/buck/rules/NoopArtifactCache.java b/src/com/facebook/buck/rules/NoopArtifactCache.java
index 574da4a..9de778f 100644
--- a/src/com/facebook/buck/rules/NoopArtifactCache.java
+++ b/src/com/facebook/buck/rules/NoopArtifactCache.java
@@ -30,4 +30,10 @@
public void store(RuleKey ruleKey, File output) {
// Do nothing.
}
+
+ /** @return {@code false}: storing artifacts is never supported by this class. */
+ @Override
+ public boolean isStoreSupported() {
+ return false;
+ }
}
diff --git a/test/com/facebook/buck/rules/MultiArtifactCacheTest.java b/test/com/facebook/buck/rules/MultiArtifactCacheTest.java
index ac25196..250f1c7 100644
--- a/test/com/facebook/buck/rules/MultiArtifactCacheTest.java
+++ b/test/com/facebook/buck/rules/MultiArtifactCacheTest.java
@@ -47,6 +47,11 @@
public void store(RuleKey ruleKey, File output) {
storeKey = ruleKey;
}
+
+ @Override
+ public boolean isStoreSupported() {
+ return true;
+ }
}
@Test
