Support non-fatal failures for downloads
Allow that downloads (as part of repository rules) may fail in a
way that may be handled by the rule. This is in line with the way
ctx.execute behaves. Moreover, it can be used to check if a file
is already in cache, e.g., if a download would require a credential
the user has to be asked for. To simplify pure cache probes, drop
the requirement of at least one URL being specified.
Another use case of trying downloads, while allowing failures is to
search for new versions in a free-floating definition of an archive
(to be then fixed in a resolved file).
Fixes #7635.
Change-Id: I2b3fc66b36c856077e5bd66dd5fee2af438999c9
PiperOrigin-RevId: 242089235
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 93842a9..e0fd9b3 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
@@ -95,7 +95,18 @@
throw new InterruptedException();
}
- Path destination = getDownloadDestination(urls.get(0), type, output);
+ URL mainUrl; // The "main" URL for this request
+ // Used for reporting only and determining the file name only.
+ if (urls.isEmpty()) {
+ if (type.isPresent() && !Strings.isNullOrEmpty(type.get())) {
+ mainUrl = new URL("http://nonexistent.example.org/cacheprobe." + type.get());
+ } else {
+ mainUrl = new URL("http://nonexistent.example.org/cacheprobe");
+ }
+ } else {
+ mainUrl = urls.get(0);
+ }
+ Path destination = getDownloadDestination(mainUrl, type, output);
// Is set to true if the value should be cached by the sha256 value provided
boolean isCachingByProvidedSha256 = false;
@@ -119,7 +130,7 @@
Path cachedDestination = repositoryCache.get(sha256, destination, KeyType.SHA256);
if (cachedDestination != null) {
// Cache hit!
- eventHandler.post(new RepositoryCacheHitEvent(repo, sha256, urls.get(0)));
+ eventHandler.post(new RepositoryCacheHitEvent(repo, sha256, mainUrl));
return cachedDestination;
}
} catch (IOException e) {
@@ -127,6 +138,10 @@
}
}
+ if (urls.isEmpty()) {
+ throw new IOException("Cache miss and no url specified");
+ }
+
for (Path dir : distdir) {
if (!dir.exists()) {
// This is not a warning (and probably we even should drop the message); it is
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 f130ab7..f7080c0 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
@@ -405,9 +405,14 @@
@Override
public StructImpl download(
- Object url, Object output, String sha256, Boolean executable, Location location)
+ Object url,
+ Object output,
+ String sha256,
+ Boolean executable,
+ Boolean allowFail,
+ Location location)
throws RepositoryFunctionException, EvalException, InterruptedException {
- List<URL> urls = getUrls(url);
+ List<URL> urls = getUrls(url, /* ensureNonEmpty= */ !allowFail);
RepositoryFunctionException sha256Validation = validateSha256(sha256, location);
if (sha256Validation != null) {
warnAboutSha256Error(urls, sha256);
@@ -438,7 +443,12 @@
throw new RepositoryFunctionException(
new IOException("thread interrupted"), Transience.TRANSIENT);
} catch (IOException e) {
- throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ if (allowFail) {
+ SkylarkDict<String, Object> dict = SkylarkDict.of(null, "success", false);
+ return StructProvider.STRUCT.createStruct(dict, null);
+ } else {
+ throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ }
}
if (sha256Validation != null) {
throw sha256Validation;
@@ -452,7 +462,8 @@
"Couldn't hash downloaded file (" + downloadedPath.getPathString() + ")", e),
Transience.PERSISTENT);
}
- SkylarkDict<String, Object> dict = SkylarkDict.of(null, "sha256", finalSha256);
+ SkylarkDict<String, Object> dict =
+ SkylarkDict.of(null, "sha256", finalSha256, "susccess", true);
return StructProvider.STRUCT.createStruct(dict, null);
}
@@ -493,9 +504,15 @@
@Override
public StructImpl downloadAndExtract(
- Object url, Object output, String sha256, String type, String stripPrefix, Location location)
+ Object url,
+ Object output,
+ String sha256,
+ String type,
+ String stripPrefix,
+ Boolean allowFail,
+ Location location)
throws RepositoryFunctionException, InterruptedException, EvalException {
- List<URL> urls = getUrls(url);
+ List<URL> urls = getUrls(url, /* ensureNonEmpty= */ !allowFail);
RepositoryFunctionException sha256Validation = validateSha256(sha256, location);
if (sha256Validation != null) {
warnAboutSha256Error(urls, sha256);
@@ -534,7 +551,12 @@
new IOException("thread interrupted"), Transience.TRANSIENT);
} catch (IOException e) {
env.getListener().post(w);
- throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ if (allowFail) {
+ SkylarkDict<String, Object> dict = SkylarkDict.of(null, "success", false);
+ return StructProvider.STRUCT.createStruct(dict, null);
+ } else {
+ throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ }
}
if (sha256Validation != null) {
throw sha256Validation;
@@ -567,7 +589,7 @@
"Couldn't delete temporary file (" + downloadedPath.getPathString() + ")", e),
Transience.TRANSIENT);
}
- SkylarkDict<String, Object> dict = SkylarkDict.of(null, "sha256", finalSha256);
+ SkylarkDict<String, Object> dict = SkylarkDict.of(null, "sha256", finalSha256, "success", true);
return StructProvider.STRUCT.createStruct(dict, null);
}
@@ -615,13 +637,18 @@
private static List<URL> getUrls(Object urlOrList)
throws RepositoryFunctionException, EvalException {
+ return getUrls(urlOrList, /* ensureNonEmpty= */ true);
+ }
+
+ private static List<URL> getUrls(Object urlOrList, boolean ensureNonEmpty)
+ throws RepositoryFunctionException, EvalException {
List<String> urlStrings;
if (urlOrList instanceof String) {
urlStrings = ImmutableList.of((String) urlOrList);
} else {
urlStrings = checkAllUrls((Iterable<?>) urlOrList);
}
- if (urlStrings.isEmpty()) {
+ if (ensureNonEmpty && urlStrings.isEmpty()) {
throw new RepositoryFunctionException(new IOException("urls not set"), Transience.PERSISTENT);
}
List<URL> urls = new ArrayList<>();
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
index 87a223b..493a397 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java
@@ -332,9 +332,22 @@
defaultValue = "False",
named = true,
doc = "set the executable flag on the created file, false by default."),
+ @Param(
+ name = "allow_fail",
+ type = Boolean.class,
+ defaultValue = "False",
+ named = true,
+ doc =
+ "If set, indicate the error in the return value"
+ + " instead of raising an error for failed downloads"),
})
public StructApi download(
- Object url, Object output, String sha256, Boolean executable, Location location)
+ Object url,
+ Object output,
+ String sha256,
+ Boolean executable,
+ Boolean allowFail,
+ Location location)
throws RepositoryFunctionExceptionT, EvalException, InterruptedException;
@SkylarkCallable(
@@ -445,8 +458,22 @@
+ " archive. Instead of needing to specify this prefix over and over in the"
+ " <code>build_file</code>, this field can be used to strip it from extracted"
+ " files."),
+ @Param(
+ name = "allow_fail",
+ type = Boolean.class,
+ defaultValue = "False",
+ named = true,
+ doc =
+ "If set, indicate the error in the return value"
+ + " instead of raising an error for failed downloads"),
})
public StructApi downloadAndExtract(
- Object url, Object output, String sha256, String type, String stripPrefix, Location location)
+ Object url,
+ Object output,
+ String sha256,
+ String type,
+ String stripPrefix,
+ Boolean allowFail,
+ Location location)
throws RepositoryFunctionExceptionT, InterruptedException, EvalException;
}
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index 93d3fba..9f64e81 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -1389,6 +1389,92 @@
expect_log '@ext//:foo'
}
+function test_cache_probe() {
+ # Verify that repository rules are able to probe for cache hits.
+ WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX")
+ cd "${WRKDIR}"
+
+ mkdir ext
+ cat > ext/BUILD <<'EOF'
+genrule(
+ name = "ext",
+ outs = ["ext.txt"],
+ cmd = "echo True external file > $@",
+ visibility = ["//visibility:public"],
+)
+EOF
+ zip ext.zip ext/*
+ rm -rf ext
+ sha256=$(sha256sum ext.zip | head -c 64)
+
+ mkdir main
+ cd main
+ cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+load("//:rule.bzl", "probe")
+
+http_archive(
+ name = "ext",
+ url = "file://${WRKDIR}/ext.zip",
+ strip_prefix="ext",
+)
+
+probe(
+ name = "probe",
+ sha256 = "${sha256}",
+)
+EOF
+ cat > BUILD <<'EOF'
+genrule(
+ name = "it",
+ outs = ["it.txt"],
+ srcs = ["@probe//:ext"],
+ cmd = "cp $< $@",
+)
+EOF
+ cat > rule.bzl <<'EOF'
+def _rule_impl(ctx):
+ result = ctx.download_and_extract(
+ url = [],
+ type = "zip",
+ stripPrefix="ext",
+ sha256 = ctx.attr.sha256,
+ allow_fail = True,
+ )
+ if not result.success:
+ # provide default implementation; a real probe
+ # should ask for credentials here and then download
+ # the actual (restricted) file.
+ ctx.file(
+ "BUILD",
+ "genrule(name='ext', outs = ['ext.txt'], cmd = 'echo no cache hit > $@', "
+ + "visibility = ['//visibility:public'],)" ,
+ )
+
+probe = repository_rule(
+ implementation = _rule_impl,
+ attrs = {"sha256" : attr.string()},
+ environ = ["ATTEMPT"],
+)
+EOF
+
+ echo; echo initial build; echo
+ # initially, no cache hit, should show default
+ env ATTEMPT=1 bazel build //:it
+ grep -q 'no cache hit' `bazel info bazel-genfiles`/it.txt \
+ || fail "didn't find the default implementation"
+
+ echo; echo build of ext; echo
+ # ensure the cache is filled
+ bazel build @ext//...
+
+ echo; echo build with cache hit; echo
+ # now we should get the real external dependency
+ env ATTEMPT=2 bazel build //:it
+ grep -q 'True external file' `bazel info bazel-genfiles`/it.txt \
+ || fail "didn't find the default implementation"
+}
+
function test_cache_hit_reported() {
# Verify that information about a chache hit is reported
# if an error happend in that repository. This information
