Internal change
PiperOrigin-RevId: 234954493
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index b4dc6e1..211fa48 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -947,6 +947,7 @@
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/bazel/debug:workspace-rule-event",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache:events",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/shell",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
index ce795c6..f2829af 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
@@ -47,6 +47,7 @@
com.google.devtools.build.lib.bazel.coverage.BazelCoverageReportModule.class,
com.google.devtools.build.lib.skylarkdebug.module.SkylarkDebuggerModule.class,
com.google.devtools.build.lib.bazel.repository.RepositoryResolvedModule.class,
+ com.google.devtools.build.lib.bazel.repository.CacheHitReportingModule.class,
com.google.devtools.build.lib.bazel.SpawnLogModule.class,
com.google.devtools.build.lib.ssd.SsdModule.class,
com.google.devtools.build.lib.worker.WorkerModule.class,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java
new file mode 100644
index 0000000..bda4c58
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/CacheHitReportingModule.java
@@ -0,0 +1,81 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.bazel.repository;
+
+import com.google.common.eventbus.Subscribe;
+import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCacheHitEvent;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.repository.RepositoryFailedEvent;
+import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.util.Pair;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/** Module reporting about cache hits in external repositories in case of failures */
+public final class CacheHitReportingModule extends BlazeModule {
+ private Reporter reporter;
+ private Map<String, Set<Pair<String, URL>>> cacheHitsByRepo;
+
+ @Override
+ public void beforeCommand(CommandEnvironment env) {
+ env.getEventBus().register(this);
+ this.reporter = env.getReporter();
+ this.cacheHitsByRepo = new HashMap<String, Set<Pair<String, URL>>>();
+ }
+
+ @Override
+ public void afterCommand() {
+ this.reporter = null;
+ this.cacheHitsByRepo = null;
+ }
+
+ @Subscribe
+ public synchronized void cacheHit(RepositoryCacheHitEvent event) {
+ String repo = event.getRepo();
+ if (cacheHitsByRepo.get(repo) == null) {
+ cacheHitsByRepo.put(repo, new HashSet<Pair<String, URL>>());
+ }
+ cacheHitsByRepo.get(repo).add(Pair.of(event.getFileHash(), event.getUrl()));
+ }
+
+ @Subscribe
+ public void failed(RepositoryFailedEvent event) {
+ String repo = event.getRepo();
+ Set<Pair<String, URL>> cacheHits = cacheHitsByRepo.get(repo);
+ if (cacheHits != null && !cacheHits.isEmpty()) {
+ StringBuilder info = new StringBuilder();
+
+ info.append("Repository '")
+ .append(repo)
+ .append(
+ "' used the following cache hits instead of downloading the corresponding file.\n");
+ for (Pair<String, URL> hit : cacheHits) {
+ info.append(" * Hash '")
+ .append(hit.getFirst())
+ .append("' for ")
+ .append(hit.getSecond().toString())
+ .append("\n");
+ }
+ info.append("If the definition of '")
+ .append(repo)
+ .append("' was updated, verify that the hashes were also updated.");
+ reporter.handle(Event.info(info.toString()));
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/BUILD
index 388d708..0abca72 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/BUILD
@@ -15,3 +15,12 @@
"//third_party:jsr305",
],
)
+
+java_library(
+ name = "events",
+ srcs = ["RepositoryCacheHitEvent.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib:events",
+ "//third_party:guava",
+ ],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheHitEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheHitEvent.java
new file mode 100644
index 0000000..009b294
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheHitEvent.java
@@ -0,0 +1,43 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.bazel.repository.cache;
+
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+import java.net.URL;
+
+/** Event reporting about cache hits for download requests. */
+public class RepositoryCacheHitEvent implements ProgressLike {
+ private final String repo;
+ private final String hash;
+ private final URL url;
+
+ public RepositoryCacheHitEvent(String repo, String hash, URL url) {
+ this.repo = repo;
+ this.hash = hash;
+ this.url = url;
+ }
+
+ public String getRepo() {
+ return repo;
+ }
+
+ public URL getUrl() {
+ return url;
+ }
+
+ public String getFileHash() {
+ return hash;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
index de0562f..e1e9f21 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
@@ -16,6 +16,7 @@
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
+ "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache:events",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/vfs",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
index d039a8d..93842a9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
@@ -21,6 +21,7 @@
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType;
+import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCacheHitEvent;
import com.google.devtools.build.lib.buildeventstream.FetchEvent;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.clock.JavaClock;
@@ -75,6 +76,8 @@
* @param output destination filename if {@code type} is <i>absent</i>, otherwise output directory
* @param eventHandler CLI progress reporter
* @param clientEnv environment variables in shell issuing this command
+ * @param repo the name of the external repository for which the file was fetched; used only for
+ * reporting
* @throws IllegalArgumentException on parameter badness, which should be checked beforehand
* @throws IOException if download was attempted and ended up failing
* @throws InterruptedException if this thread is being cast into oblivion
@@ -85,7 +88,8 @@
Optional<String> type,
Path output,
ExtendedEventHandler eventHandler,
- Map<String, String> clientEnv)
+ Map<String, String> clientEnv,
+ String repo)
throws IOException, InterruptedException {
if (Thread.interrupted()) {
throw new InterruptedException();
@@ -115,6 +119,7 @@
Path cachedDestination = repositoryCache.get(sha256, destination, KeyType.SHA256);
if (cachedDestination != null) {
// Cache hit!
+ eventHandler.post(new RepositoryCacheHitEvent(repo, sha256, urls.get(0)));
return cachedDestination;
}
} catch (IOException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index 2457712..077f222 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -399,7 +399,8 @@
Optional.<String>absent(),
outputPath.getPath(),
env.getListener(),
- osObject.getEnvironmentVariables());
+ osObject.getEnvironmentVariables(),
+ getName());
if (executable) {
outputPath.getPath().setExecutable(true);
}
@@ -485,7 +486,8 @@
Optional.of(type),
outputPath.getPath(),
env.getListener(),
- osObject.getEnvironmentVariables());
+ osObject.getEnvironmentVariables(),
+ getName());
} catch (InterruptedException e) {
env.getListener().post(w);
throw new RepositoryFunctionException(
diff --git a/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java b/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java
new file mode 100644
index 0000000..61aec63
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java
@@ -0,0 +1,32 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.repository;
+
+import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
+
+/**
+ * Event indicating that a failure is related to a given external repository; this is in particular
+ * the case, if fetching that repository failed.
+ */
+public class RepositoryFailedEvent implements ProgressLike {
+ private final String repo;
+
+ public RepositoryFailedEvent(String repo) {
+ this.repo = repo;
+ }
+
+ public String getRepo() {
+ return repo;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
index c0b99c5..fd1e8e0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.repository.ExternalPackageException;
import com.google.devtools.build.lib.repository.ExternalPackageUtil;
import com.google.devtools.build.lib.repository.ExternalRuleNotFoundException;
+import com.google.devtools.build.lib.repository.RepositoryFailedEvent;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
@@ -219,6 +220,7 @@
} catch (SkyFunctionException e) {
// Upon an exceptional exit, the fetching of that repository is over as well.
env.getListener().post(new RepositoryFetching(repositoryName.getName(), true));
+ env.getListener().post(new RepositoryFailedEvent(repositoryName.strippedName()));
throw e;
}
if (env.valuesMissing()) {
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index 9dc16ee..52ab719 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -1410,6 +1410,99 @@
expect_log '@ext//:foo'
}
+function test_cache_hit_reported() {
+ # Verify that information about a chache hit is reported
+ # if an error happend in that repository. This information
+ # is useful as users sometimes change the URL but do not
+ # update the hash.
+ WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX")
+ cd "${WRKDIR}"
+ mkdir ext-1.1
+ cat > ext-1.1/BUILD <<'EOF'
+genrule(
+ name="foo",
+ outs=["foo.txt"],
+ cmd="echo Hello World > $@",
+ visibility = ["//visibility:public"],
+)
+EOF
+ zip ext-1.1.zip ext-1.1/*
+ rm -rf ext-1.1
+ sha256=$(sha256sum ext-1.1.zip | head -c 64)
+
+ rm -rf main
+ mkdir main
+ cd main
+ cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+ name="ext",
+ strip_prefix="ext-1.1",
+ urls=["file://${WRKDIR}/ext-1.1.zip"],
+ sha256="${sha256}",
+)
+EOF
+ cat > BUILD <<'EOF'
+genrule(
+ name = "it",
+ srcs = ["@ext//:foo"],
+ outs = ["it.txt"],
+ cmd = "cp $< $@"
+)
+EOF
+ # build to fill the cache
+ bazel build //:it || fail "Expected success"
+
+ # go offline and clean everything
+ bazel clean --expunge
+ rm "${WRKDIR}/ext-1.1.zip"
+
+ # We still should be able to build, as the file is in cache
+ bazel build //:it > "${TEST_log}" 2>&1 || fail "Expected success"
+ # As a cache hit is a perfectly normal thing, we don't expect it to be
+ # reported.
+ expect_not_log 'cache hit'
+ expect_not_log "${sha256}"
+ expect_not_log 'file:.*/ext-1.1.zip'
+
+ # Now update ext-1.1 to ext-1.2, while forgetting to update the checksum
+ ed WORKSPACE <<EOI
+%s/ext-1\.1/ext-1\.2/g
+w
+q
+EOI
+ # The build should fail, as the prefix is not found. The available prefix
+ # should be reported as well as the information that the file was taken
+ # from cache.
+ bazel build //:it > "${TEST_log}" 2>&1 && fail "Should not succeed" || :
+ expect_log 'ext-1.2.*not found'
+ expect_log 'prefixes.*ext-1.1'
+ expect_log 'cache hit'
+ expect_log "${sha256}"
+ expect_log 'file:.*/ext-1.2.zip'
+
+ # Now consider the case where no prefix is specified (and hence, the
+ # download_and_extract call will succeed), but a patch command has
+ # an assumption on a wrong path. As the fetching of the external
+ # repository will fail, we still expect being hinted at the
+ # cache hit.
+ ed WORKSPACE <<'EOI'
+/strip_prefix
+d
+a
+patch_cmds = ["cp ext-1.2/foo.txt ext-1.2/BUILD ."],
+.
+w
+q
+EOI
+ bazel build //:it > "${TEST_log}" 2>&1 && fail "Should not succeed" || :
+ expect_not_log 'prefix'
+ expect_log 'cp ext-1.2/foo.txt ext-1.2/BUILD'
+ expect_log 'cache hit'
+ expect_log "${sha256}"
+ expect_log 'file:.*/ext-1.2.zip'
+}
+
function test_distdir() {
WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX")
cd "${WRKDIR}"
