diff options
| -rw-r--r-- | lib/spack/llnl/util/filesystem.py | 25 | ||||
| -rw-r--r-- | lib/spack/spack/binary_distribution.py | 30 | ||||
| -rw-r--r-- | lib/spack/spack/fetch_strategy.py | 18 | ||||
| -rw-r--r-- | lib/spack/spack/installer.py | 18 | ||||
| -rw-r--r-- | lib/spack/spack/test/fetch_strategy.py | 10 | ||||
| -rw-r--r-- | lib/spack/spack/test/llnl/util/filesystem.py | 10 |
6 files changed, 67 insertions, 44 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 330ee35911..4dfea7975d 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -2630,3 +2630,28 @@ def temporary_dir( yield tmp_dir finally: remove_directory_contents(tmp_dir) + + +def filesummary(path, print_bytes=16) -> Tuple[int, bytes]: + """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 01f428c633..e9e48b4010 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -41,6 +41,7 @@ import spack.relocate as relocate import spack.repo import spack.store import spack.traverse as traverse +import spack.util.crypto import spack.util.file_cache as file_cache import spack.util.gpg import spack.util.spack_json as sjson @@ -553,7 +554,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__( + f"{algorithm} checksum failed for {path}", + f"Expected {expected} but got {computed}. " + f"File size = {size} bytes. Contents = {contents!r}", + ) class NewLayoutException(spack.error.SpackError): @@ -1763,14 +1769,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 @@ -1828,12 +1835,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)) @@ -1924,8 +1933,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/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 19ad454ec5..0ea42ecb36 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -89,22 +89,6 @@ 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) @@ -513,7 +497,7 @@ class URLFetchStrategy(FetchStrategy): # 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) + size, contents = fs.filesummary(self.archive_file) raise ChecksumError( f"{checker.hash_name} checksum failed for {self.archive_file}", f"Expected {self.digest} but got {checker.sum}. " diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 7bbe8b2dd2..18eb00c2a9 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -1755,14 +1755,16 @@ class PackageInstaller(object): 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/fetch_strategy.py b/lib/spack/spack/test/fetch_strategy.py index 932a527a52..59c645d4e0 100644 --- a/lib/spack/spack/test/fetch_strategy.py +++ b/lib/spack/spack/test/fetch_strategy.py @@ -14,13 +14,3 @@ def test_fetchstrategy_bad_url_scheme(): with pytest.raises(ValueError): 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") diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index bbadfcbca9..5e81b52c53 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -861,3 +861,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") |