diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index aece52f8436..c7a207d5d18 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -2589,3 +2589,28 @@ def temporary_dir(*args, **kwargs): yield tmp_dir finally: remove_directory_contents(tmp_dir) + + +def filesummary(path, print_bytes=16): + """Create a small summary of the given file. Does not error + when file does not exist. + + Args: + print_bytes (int): Number of bytes to print from start/end of file + + Returns: + Tuple of size and byte string containing first n .. last n bytes. + Size is 0 if file cannot be read.""" + 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"" diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 8ceeeea7386..03af286f0b1 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -36,6 +36,7 @@ import spack.relocate as relocate import spack.repo import spack.store +import spack.util.crypto import spack.util.file_cache as file_cache import spack.util.gpg import spack.util.spack_json as sjson @@ -603,7 +604,12 @@ class NoChecksumException(spack.error.SpackError): Raised if file fails checksum verification. """ - pass + def __init__(self, path, size, contents, algorithm, expected, computed): + super(NoChecksumException, self).__init__( + "{} checksum failed for {}".format(algorithm, path), + "Expected {} but got {}. " + "File size = {} bytes. Contents = {!r}".format(expected, computed, size, contents), + ) class NewLayoutException(spack.error.SpackError): @@ -1861,14 +1867,15 @@ def _extract_inner_tarball(spec, filename, extract_to, unsigned, remote_checksum raise UnsignedPackageException( "To install unsigned packages, use the --no-check-signature option." ) - # get the sha256 checksum of the tarball + + # compute the sha256 checksum of the tarball local_checksum = checksum_tarball(tarfile_path) + expected = remote_checksum["hash"] # if the checksums don't match don't install - if local_checksum != remote_checksum["hash"]: - raise NoChecksumException( - "Package tarball failed checksum verification.\n" "It cannot be installed." - ) + if local_checksum != expected: + size, contents = fsys.filesummary(tarfile_path) + raise NoChecksumException(tarfile_path, size, contents, "sha256", expected, local_checksum) return tarfile_path @@ -1928,12 +1935,14 @@ def extract_tarball(spec, download_result, allow_root=False, unsigned=False, for # compute the sha256 checksum of the tarball local_checksum = checksum_tarball(tarfile_path) + expected = bchecksum["hash"] # if the checksums don't match don't install - if local_checksum != bchecksum["hash"]: + if local_checksum != expected: + size, contents = fsys.filesummary(tarfile_path) _delete_staged_downloads(download_result) raise NoChecksumException( - "Package tarball failed checksum verification.\n" "It cannot be installed." + tarfile_path, size, contents, "sha256", expected, local_checksum ) new_relative_prefix = str(os.path.relpath(spec.prefix, spack.store.layout.root)) @@ -2024,8 +2033,11 @@ def install_root_node(spec, allow_root, unsigned=False, force=False, sha256=None tarball_path = download_result["tarball_stage"].save_filename msg = msg.format(tarball_path, sha256) if not checker.check(tarball_path): + size, contents = fsys.filesummary(tarball_path) _delete_staged_downloads(download_result) - raise spack.binary_distribution.NoChecksumException(msg) + raise NoChecksumException( + tarball_path, size, contents, checker.hash_name, sha256, checker.sum + ) tty.debug("Verified SHA256 checksum of the build cache") # don't print long padded paths while extracting/relocating binaries diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index d95abf3ac23..99a83079ebd 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -1773,14 +1773,16 @@ def install(self): raise except binary_distribution.NoChecksumException as exc: - if not task.cache_only: - # Checking hash on downloaded binary failed. - err = "Failed to install {0} from binary cache due to {1}:" - err += " Requeueing to install from source." - tty.error(err.format(pkg.name, str(exc))) - task.use_cache = False - self._requeue_task(task) - continue + if task.cache_only: + raise + + # Checking hash on downloaded binary failed. + err = "Failed to install {0} from binary cache due to {1}:" + err += " Requeueing to install from source." + tty.error(err.format(pkg.name, str(exc))) + task.use_cache = False + self._requeue_task(task) + continue except (Exception, SystemExit) as exc: self._update_failed(task, True, exc) diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 4950558db6e..24950b827e3 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -903,3 +903,13 @@ def test_remove_linked_tree_doesnt_change_file_permission(tmpdir, initial_mode): fs.remove_linked_tree(str(file_instead_of_dir)) final_stat = os.stat(str(file_instead_of_dir)) assert final_stat == initial_stat + + +def test_filesummary(tmpdir): + p = str(tmpdir.join("xyz")) + with open(p, "wb") as f: + f.write(b"abcdefghijklmnopqrstuvwxyz") + + assert fs.filesummary(p, print_bytes=8) == (26, b"abcdefgh...stuvwxyz") + assert fs.filesummary(p, print_bytes=13) == (26, b"abcdefghijklmnopqrstuvwxyz") + assert fs.filesummary(p, print_bytes=100) == (26, b"abcdefghijklmnopqrstuvwxyz")