From 88749de5c946909dd7b159e87533ef0acf7d7b77 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Tue, 8 Sep 2020 17:15:48 -0700 Subject: Clarify manual download required if unable to fetch package (#18242) Clarify manual download required if unable to fetch (from mirror(s)); support (and tests) for package-specific download instructions --- lib/spack/spack/package.py | 20 ++++++++++++++-- lib/spack/spack/stage.py | 13 +++++++---- lib/spack/spack/test/packaging.py | 49 +++++++++++++++++++++++++++++++++++++++ lib/spack/spack/test/stage.py | 16 +++++++++---- 4 files changed, 87 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index d5cb3065a8..3a0c17da88 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -577,7 +577,8 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): archive_files = [] #: Boolean. Set to ``True`` for packages that require a manual download. - #: This is currently only used by package sanity tests. + #: This is currently used by package sanity tests and generation of a + #: more meaningful fetch failure error. manual_download = False #: Set of additional options used when fetching package versions. @@ -1210,6 +1211,20 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): """ spack.store.layout.remove_install_directory(self.spec) + @property + def download_instr(self): + """ + Defines the default manual download instructions. Packages can + override the property to provide more information. + + Returns: + (str): default manual download instructions + """ + required = ('Manual download is required for {0}. ' + .format(self.spec.name) if self.manual_download else '') + return ('{0}Refer to {1} for download instructions.' + .format(required, self.spec.package.homepage)) + def do_fetch(self, mirror_only=False): """ Creates a stage directory and downloads the tarball for this package. @@ -1244,7 +1259,8 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): self.spec.format('{name}{@version}'), ck_msg) self.stage.create() - self.stage.fetch(mirror_only) + err_msg = None if not self.manual_download else self.download_instr + self.stage.fetch(mirror_only, err_msg=err_msg) self._fetch_time = time.time() - start_time if checksum and self.version in self.versions: diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index 95e7dcc592..cf32bc92f8 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -400,8 +400,14 @@ class Stage(object): """Returns the well-known source directory path.""" return os.path.join(self.path, _source_path_subdir) - def fetch(self, mirror_only=False): - """Downloads an archive or checks out code from a repository.""" + def fetch(self, mirror_only=False, err_msg=None): + """Retrieves the code or archive + + Args: + mirror_only (bool): only fetch from a mirror + err_msg (str or None): the error message to display if all fetchers + fail or ``None`` for the default fetch failure message + """ fetchers = [] if not mirror_only: fetchers.append(self.default_fetcher) @@ -480,9 +486,8 @@ class Stage(object): else: print_errors(errors) - err_msg = 'All fetchers failed for {0}'.format(self.name) self.fetcher = self.default_fetcher - raise fs.FetchError(err_msg, None) + raise fs.FetchError(err_msg or 'All fetchers failed', None) print_errors(errors) diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index 75ead64955..bee0d01b16 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -560,3 +560,52 @@ def test_macho_make_paths(): '/Users/Shared/spack/pkgB/libB.dylib', '/usr/local/lib/libloco.dylib': '/usr/local/lib/libloco.dylib'} + + +@pytest.fixture() +def mock_download(): + """Mock a failing download strategy.""" + class FailedDownloadStrategy(spack.fetch_strategy.FetchStrategy): + def mirror_id(self): + return None + + def fetch(self): + raise spack.fetch_strategy.FailedDownloadError( + "", "This FetchStrategy always fails") + + fetcher = FetchStrategyComposite() + fetcher.append(FailedDownloadStrategy()) + + @property + def fake_fn(self): + return fetcher + + orig_fn = spack.package.PackageBase.fetcher + spack.package.PackageBase.fetcher = fake_fn + yield + spack.package.PackageBase.fetcher = orig_fn + + +@pytest.mark.parametrize("manual,instr", [(False, False), (False, True), + (True, False), (True, True)]) +@pytest.mark.disable_clean_stage_check +def test_manual_download(install_mockery, mock_download, monkeypatch, manual, + instr): + """ + Ensure expected fetcher fail message based on manual download and instr. + """ + @property + def _instr(pkg): + return 'Download instructions for {0}'.format(pkg.spec.name) + + spec = Spec('a').concretized() + pkg = spec.package + + pkg.manual_download = manual + if instr: + monkeypatch.setattr(spack.package.PackageBase, 'download_instr', + _instr) + + expected = pkg.download_instr if manual else 'All fetchers failed' + with pytest.raises(spack.fetch_strategy.FetchError, match=expected): + pkg.do_fetch() diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py index 9cf50b8da2..204a6e6a3e 100644 --- a/lib/spack/spack/test/stage.py +++ b/lib/spack/spack/test/stage.py @@ -529,15 +529,21 @@ class TestStage(object): pass check_destroy(stage, self.stage_name) - def test_search_if_default_fails(self, failing_fetch_strategy, search_fn): + @pytest.mark.parametrize( + "err_msg,expected", [('Fetch from fetch.test.com', + 'Fetch from fetch.test.com'), + (None, 'All fetchers failed')]) + def test_search_if_default_fails(self, failing_fetch_strategy, search_fn, + err_msg, expected): stage = Stage(failing_fetch_strategy, name=self.stage_name, search_fn=search_fn) + with stage: - try: - stage.fetch(mirror_only=False) - except spack.fetch_strategy.FetchError: - pass + with pytest.raises(spack.fetch_strategy.FetchError, + match=expected): + stage.fetch(mirror_only=False, err_msg=err_msg) + check_destroy(stage, self.stage_name) assert search_fn.performed_search -- cgit v1.2.3-70-g09d2