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>
This commit is contained in:
		@@ -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 @@ def check(self):
 | 
			
		||||
 | 
			
		||||
        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
 | 
			
		||||
 
 | 
			
		||||
@@ -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")
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user