From 3bc943ae518dbc9380c0c065c4900c466d747d3b Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Wed, 18 Jan 2023 15:25:48 +0100 Subject: Remove verbose warning message from _try_install_from_binary_cache (#34994) In the past we checked remote binary mirrors for existence of a spec before attempting to download it. That changed to only checking local copies of index.jsons (if available) to prioritize certain mirrors where we expect to find a tarball. That was faster for CI since fetching index.json and loading it just to order mirrors takes more time than just attempting to fetch tarballs -- and also if we have a direct hit there's no point to look at other mirrors. Long story short: the info message only makes sense in the old version of Spack, so it's better to remove it. --- lib/spack/spack/installer.py | 44 +++++++++++++++------------------------ lib/spack/spack/test/installer.py | 12 +---------- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index eaf734a816..8415310c63 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -364,14 +364,13 @@ def _process_external_package(pkg, explicit): def _process_binary_cache_tarball( - pkg, binary_spec, explicit, unsigned, mirrors_for_spec=None, timer=timer.NULL_TIMER + pkg, explicit, unsigned, mirrors_for_spec=None, timer=timer.NULL_TIMER ): """ Process the binary cache tarball. Args: pkg (spack.package_base.PackageBase): the package being installed - binary_spec (spack.spec.Spec): the spec whose cache has been confirmed explicit (bool): the package was explicitly requested by the user unsigned (bool): ``True`` if binary package signatures to be checked, otherwise, ``False`` @@ -383,30 +382,24 @@ def _process_binary_cache_tarball( bool: ``True`` if the package was extracted from binary cache, else ``False`` """ - timer.start("fetch") - download_result = binary_distribution.download_tarball( - binary_spec, unsigned, mirrors_for_spec=mirrors_for_spec - ) - timer.stop("fetch") - # see #10063 : install from source if tarball doesn't exist - if download_result is None: - tty.msg("{0} exists in binary cache but with different hash".format(pkg.name)) - return False + with timer.measure("fetch"): + download_result = binary_distribution.download_tarball( + pkg.spec, unsigned, mirrors_for_spec + ) - pkg_id = package_id(pkg) - tty.msg("Extracting {0} from binary cache".format(pkg_id)) + if download_result is None: + return False - # don't print long padded paths while extracting/relocating binaries - timer.start("install") - with spack.util.path.filter_padding(): + tty.msg("Extracting {0} from binary cache".format(package_id(pkg))) + + with timer.measure("install"), spack.util.path.filter_padding(): binary_distribution.extract_tarball( - binary_spec, download_result, allow_root=False, unsigned=unsigned, force=False + pkg.spec, download_result, allow_root=False, unsigned=unsigned, force=False ) - pkg.installed_from_binary_cache = True - spack.store.db.add(pkg.spec, spack.store.layout, explicit=explicit) - timer.stop("install") - return True + pkg.installed_from_binary_cache = True + spack.store.db.add(pkg.spec, spack.store.layout, explicit=explicit) + return True def _try_install_from_binary_cache(pkg, explicit, unsigned=False, timer=timer.NULL_TIMER): @@ -424,16 +417,13 @@ def _try_install_from_binary_cache(pkg, explicit, unsigned=False, timer=timer.NU if not spack.mirror.MirrorCollection(): return False - pkg_id = package_id(pkg) - tty.debug("Searching for binary cache of {0}".format(pkg_id)) + tty.debug("Searching for binary cache of {0}".format(package_id(pkg))) - timer.start("search") - matches = binary_distribution.get_mirrors_for_spec(pkg.spec, index_only=True) - timer.stop("search") + with timer.measure("search"): + matches = binary_distribution.get_mirrors_for_spec(pkg.spec, index_only=True) return _process_binary_cache_tarball( pkg, - pkg.spec, explicit, unsigned, mirrors_for_spec=matches, diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 3b45646c6b..f0ecf7c3db 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -205,16 +205,6 @@ def test_process_external_package_module(install_mockery, monkeypatch, capfd): assert "has external module in {0}".format(spec.external_modules) in out -def test_process_binary_cache_tarball_none(install_mockery, monkeypatch, capfd): - """Tests of _process_binary_cache_tarball when no tarball.""" - monkeypatch.setattr(spack.binary_distribution, "download_tarball", _none) - - s = spack.spec.Spec("trivial-install-test-package").concretized() - assert not inst._process_binary_cache_tarball(s.package, None, False, False) - - assert "exists in binary cache but" in capfd.readouterr()[0] - - def test_process_binary_cache_tarball_tar(install_mockery, monkeypatch, capfd): """Tests of _process_binary_cache_tarball with a tar file.""" @@ -229,7 +219,7 @@ def test_process_binary_cache_tarball_tar(install_mockery, monkeypatch, capfd): monkeypatch.setattr(spack.database.Database, "add", _noop) spec = spack.spec.Spec("a").concretized() - assert inst._process_binary_cache_tarball(spec.package, spec, False, False) + assert inst._process_binary_cache_tarball(spec.package, explicit=False, unsigned=False) out = capfd.readouterr()[0] assert "Extracting a" in out -- cgit v1.2.3-70-g09d2