SkylarkRepositoryContext: change handling of invalid checksums
If a string is not a sequence of 64 Hex-Characters, then this property
is persisent: checking the same string again will not suddently make it
be a sequence of 64 Hex-Characters. Therefore, make the error persistent.
Also, a syntactically incorrect argument has nothing to do with IO,
so change the underlying exception class.
While there, also improve the error message and include the repository
name as well.
Moreover, as requested in issue #7353, do not abort early, but instead
still download from the specified URL, which will show the hash of the
file at the requested URL.
Related to #7353.
Change-Id: I1126ee55567f9b0753940423e8c80b811190fc8d
PiperOrigin-RevId: 233918098
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 8da9601..7ff9f27 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
@@ -361,12 +361,34 @@
return null;
}
+ private void warnAboutSha256Error(List<URL> urls, String sha256, Location location) {
+ // Inform the user immediately, even though the file will still be downloaded.
+ // This cannot be done by a regular error event, as all regular events are recorded
+ // and only shown once the execution of the repository rule is finished.
+ // So we have to provide the information as update on the progress
+ String url = "(unknown)";
+ if (urls.size() > 0) {
+ url = urls.get(0).toString();
+ }
+ reportProgress(
+ "Will fail after download of "
+ + url
+ + ". Invalid SHA256 '"
+ + sha256
+ + "' specified at "
+ + location);
+ }
+
@Override
public StructImpl download(
Object url, Object output, String sha256, Boolean executable, Location location)
throws RepositoryFunctionException, EvalException, InterruptedException {
- validateSha256(sha256);
List<URL> urls = getUrls(url);
+ RepositoryFunctionException sha256Validation = validateSha256(sha256, location);
+ if (sha256Validation != null) {
+ warnAboutSha256Error(urls, sha256, location);
+ sha256 = "";
+ }
SkylarkPath outputPath = getPath("download()", output);
WorkspaceRuleEvent w =
WorkspaceRuleEvent.newDownloadEvent(
@@ -393,6 +415,9 @@
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
+ if (sha256Validation != null) {
+ throw sha256Validation;
+ }
String finalSha256;
try {
finalSha256 = calculateSha256(sha256, downloadedPath);
@@ -435,8 +460,12 @@
public StructImpl downloadAndExtract(
Object url, Object output, String sha256, String type, String stripPrefix, Location location)
throws RepositoryFunctionException, InterruptedException, EvalException {
- validateSha256(sha256);
List<URL> urls = getUrls(url);
+ RepositoryFunctionException sha256Validation = validateSha256(sha256, location);
+ if (sha256Validation != null) {
+ warnAboutSha256Error(urls, sha256, location);
+ sha256 = "";
+ }
WorkspaceRuleEvent w =
WorkspaceRuleEvent.newDownloadAndExtractEvent(
@@ -447,7 +476,6 @@
stripPrefix,
rule.getLabel().toString(),
location);
- env.getListener().post(w);
// Download to outputDirectory and delete it after extraction
SkylarkPath outputPath = getPath("download_and_extract()", output);
@@ -465,11 +493,17 @@
env.getListener(),
osObject.getEnvironmentVariables());
} catch (InterruptedException e) {
+ env.getListener().post(w);
throw new RepositoryFunctionException(
new IOException("thread interrupted"), Transience.TRANSIENT);
} catch (IOException e) {
+ env.getListener().post(w);
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
+ if (sha256Validation != null) {
+ throw sha256Validation;
+ }
+ env.getListener().post(w);
DecompressorValue.decompress(
DecompressorDescriptor.builder()
.setTargetKind(rule.getTargetKind())
@@ -509,11 +543,19 @@
return RepositoryCache.getChecksum(KeyType.SHA256, path);
}
- private static void validateSha256(String sha256) throws RepositoryFunctionException {
+ private RepositoryFunctionException validateSha256(String sha256, Location loc) {
if (!sha256.isEmpty() && !KeyType.SHA256.isValid(sha256)) {
- throw new RepositoryFunctionException(
- new IOException("Invalid SHA256 checksum"), Transience.TRANSIENT);
+ return new RepositoryFunctionException(
+ new EvalException(
+ loc,
+ "Definition of repository "
+ + rule.getName()
+ + ": Syntactically invalid SHA256 checksum: '"
+ + sha256
+ + "'"),
+ Transience.PERSISTENT);
}
+ return null;
}
private static ImmutableList<String> checkAllUrls(Iterable<?> urlList) throws EvalException {
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index 9f9efa7..bb2b222 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -901,7 +901,7 @@
)
EOF
bazel build @repo//... &> $TEST_log && fail "Expected to fail"
- expect_log "Invalid SHA256 checksum"
+ expect_log "[Ii]nvalid SHA256 checksum"
shutdown_server
}
@@ -1708,4 +1708,37 @@
expect_log "foo.*Second action"
}
+function test_progress_reporting() {
+ # Isse 7353 requested that even in the case of a syntactically invalid
+ # checksum, the file still should be fetched and its checksum computed.
+ WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX")
+ cd "${WRKDIR}"
+
+ touch ext.tar
+
+ mkdir main
+ cd main
+ cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+ name = "ext",
+ urls = ["file://${WRKDIR}/ext.tar"],
+ sha256 = "badargument",
+)
+EOF
+ cat > BUILD <<'EOF'
+genrule(
+ name = "it",
+ srcs = ["@ext//:in.txt"],
+ outs = ["out.txt"],
+ cmd = "cp $< $@",
+)
+EOF
+
+ bazel build //:it > "${TEST_log}" 2>&1 && fail "Expected failure" || :
+
+ expect_log '@ext.*badargument'
+ expect_log 'SHA256 (.*/ext.tar) = e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'
+}
+
run_suite "external tests"
