diff options
author | Harmen Stoppels <harmenstoppels@gmail.com> | 2023-01-25 16:02:41 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-25 15:02:41 +0000 |
commit | 5f8c09fd334d729cf2ff0cdd2398ead78d9c621c (patch) | |
tree | b76c03eb68f8495a37ff4bcd970fb41b815f4fd1 | |
parent | 8eb4807615610a987a558134247b8c3d41f04be8 (diff) | |
download | spack-5f8c09fd334d729cf2ff0cdd2398ead78d9c621c.tar.gz spack-5f8c09fd334d729cf2ff0cdd2398ead78d9c621c.tar.bz2 spack-5f8c09fd334d729cf2ff0cdd2398ead78d9c621c.tar.xz spack-5f8c09fd334d729cf2ff0cdd2398ead78d9c621c.zip |
Print file summary on checksum validation failure (#35161)
Currently we print "sha256 checksum failed for [file]. Expected X but
got Y".
This PR extends that message with file size and contents info:
"... but got Y. File size = 123456 bytes. Contents = b'abc...def'"
That way we can immediately see if the file was downloaded only
partially, or if we downloaded a text page instead of a binary, etc.
Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
-rw-r--r-- | lib/spack/spack/fetch_strategy.py | 25 | ||||
-rw-r--r-- | lib/spack/spack/test/fetch_strategy.py | 14 |
2 files changed, 35 insertions, 4 deletions
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 557044ca5b..720a28b9bd 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -95,6 +95,22 @@ def _ensure_one_stage_entry(stage_path): return os.path.join(stage_path, stage_entries[0]) +def _filesummary(path, print_bytes=16): + try: + n = print_bytes + with open(path, "rb") as f: + size = os.fstat(f.fileno()).st_size + if size <= 2 * n: + short_contents = f.read(2 * n) + else: + short_contents = f.read(n) + f.seek(-n, 2) + short_contents += b"..." + f.read(n) + return size, short_contents + except OSError: + return 0, b"" + + def fetcher(cls): """Decorator used to register fetch strategies.""" all_strategies.append(cls) @@ -500,9 +516,14 @@ class URLFetchStrategy(FetchStrategy): checker = crypto.Checker(self.digest) if not checker.check(self.archive_file): + # On failure, provide some information about the file size and + # contents, so that we can quickly see what the issue is (redirect + # was not followed, empty file, text instead of binary, ...) + size, contents = _filesummary(self.archive_file) raise ChecksumError( - "%s checksum failed for %s" % (checker.hash_name, self.archive_file), - "Expected %s but got %s" % (self.digest, checker.sum), + f"{checker.hash_name} checksum failed for {self.archive_file}", + f"Expected {self.digest} but got {checker.sum}. " + f"File size = {size} bytes. Contents = {contents!r}", ) @_needs_stage diff --git a/lib/spack/spack/test/fetch_strategy.py b/lib/spack/spack/test/fetch_strategy.py index ad484784b1..932a527a52 100644 --- a/lib/spack/spack/test/fetch_strategy.py +++ b/lib/spack/spack/test/fetch_strategy.py @@ -5,7 +5,7 @@ import pytest -from spack.fetch_strategy import from_url_scheme +from spack import fetch_strategy def test_fetchstrategy_bad_url_scheme(): @@ -13,4 +13,14 @@ def test_fetchstrategy_bad_url_scheme(): unsupported scheme fails as expected.""" with pytest.raises(ValueError): - fetcher = from_url_scheme("bogus-scheme://example.com/a/b/c") # noqa: F841 + fetcher = fetch_strategy.from_url_scheme("bogus-scheme://example.com/a/b/c") # noqa: F841 + + +def test_filesummary(tmpdir): + p = str(tmpdir.join("xyz")) + with open(p, "wb") as f: + f.write(b"abcdefghijklmnopqrstuvwxyz") + + assert fetch_strategy._filesummary(p, print_bytes=8) == (26, b"abcdefgh...stuvwxyz") + assert fetch_strategy._filesummary(p, print_bytes=13) == (26, b"abcdefghijklmnopqrstuvwxyz") + assert fetch_strategy._filesummary(p, print_bytes=100) == (26, b"abcdefghijklmnopqrstuvwxyz") |