From 57769fac7d57b55394b2ed8cc07f631904e7538b Mon Sep 17 00:00:00 2001
From: Harmen Stoppels <me@harmenstoppels.nl>
Date: Mon, 19 Aug 2024 11:34:13 +0200
Subject: Simplify URLFetchStrategy (#45741)
---
lib/spack/spack/fetch_strategy.py | 171 ++++++++++++--------------------------
lib/spack/spack/oci/oci.py | 2 +-
lib/spack/spack/patch.py | 2 +-
lib/spack/spack/stage.py | 31 +++----
lib/spack/spack/test/conftest.py | 2 +-
lib/spack/spack/test/gcs_fetch.py | 51 ++----------
lib/spack/spack/test/mirror.py | 2 +-
lib/spack/spack/test/packaging.py | 2 +-
lib/spack/spack/test/s3_fetch.py | 53 ++----------
lib/spack/spack/test/url_fetch.py | 10 +--
10 files changed, 93 insertions(+), 233 deletions(-)
(limited to 'lib')
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 589b341f5c..fd313e0463 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -54,7 +54,7 @@ import spack.util.web as web_util
import spack.version
import spack.version.git_ref_lookup
from spack.util.compression import decompressor_for
-from spack.util.executable import CommandNotFoundError, which
+from spack.util.executable import CommandNotFoundError, Executable, which
#: List of all fetch strategies, created by FetchStrategy metaclass.
all_strategies = []
@@ -246,33 +246,28 @@ class URLFetchStrategy(FetchStrategy):
# these are checksum types. The generic 'checksum' is deprecated for
# specific hash names, but we need it for backward compatibility
- optional_attrs = list(crypto.hashes.keys()) + ["checksum"]
+ optional_attrs = [*crypto.hashes.keys(), "checksum"]
- def __init__(self, url=None, checksum=None, **kwargs):
+ def __init__(self, *, url: str, checksum: Optional[str] = None, **kwargs) -> None:
super().__init__(**kwargs)
- # Prefer values in kwargs to the positionals.
- self.url = kwargs.get("url", url)
+ self.url = url
self.mirrors = kwargs.get("mirrors", [])
# digest can be set as the first argument, or from an explicit
# kwarg by the hash name.
- self.digest = kwargs.get("checksum", checksum)
+ self.digest: Optional[str] = checksum
for h in self.optional_attrs:
if h in kwargs:
self.digest = kwargs[h]
- self.expand_archive = kwargs.get("expand", True)
- self.extra_options = kwargs.get("fetch_options", {})
- self._curl = None
-
- self.extension = kwargs.get("extension", None)
-
- if not self.url:
- raise ValueError("URLFetchStrategy requires a url for fetching.")
+ self.expand_archive: bool = kwargs.get("expand", True)
+ self.extra_options: dict = kwargs.get("fetch_options", {})
+ self._curl: Optional[Executable] = None
+ self.extension: Optional[str] = kwargs.get("extension", None)
@property
- def curl(self):
+ def curl(self) -> Executable:
if not self._curl:
self._curl = web_util.require_curl()
return self._curl
@@ -348,8 +343,8 @@ class URLFetchStrategy(FetchStrategy):
if os.path.lexists(save_file):
os.remove(save_file)
- with open(save_file, "wb") as _open_file:
- shutil.copyfileobj(response, _open_file)
+ with open(save_file, "wb") as f:
+ shutil.copyfileobj(response, f)
self._check_headers(str(response.headers))
@@ -468,7 +463,7 @@ class URLFetchStrategy(FetchStrategy):
"""Check the downloaded archive against a checksum digest.
No-op if this stage checks code out of a repository."""
if not self.digest:
- raise NoDigestError("Attempt to check URLFetchStrategy with no digest.")
+ raise NoDigestError(f"Attempt to check {self.__class__.__name__} with no digest.")
verify_checksum(self.archive_file, self.digest)
@@ -479,8 +474,8 @@ class URLFetchStrategy(FetchStrategy):
"""
if not self.archive_file:
raise NoArchiveFileError(
- "Tried to reset URLFetchStrategy before fetching",
- "Failed on reset() for URL %s" % self.url,
+ f"Tried to reset {self.__class__.__name__} before fetching",
+ f"Failed on reset() for URL{self.url}",
)
# Remove everything but the archive from the stage
@@ -493,14 +488,10 @@ class URLFetchStrategy(FetchStrategy):
self.expand()
def __repr__(self):
- url = self.url if self.url else "no url"
- return "%s<%s>" % (self.__class__.__name__, url)
+ return f"{self.__class__.__name__}<{self.url}>"
def __str__(self):
- if self.url:
- return self.url
- else:
- return "[no url]"
+ return self.url
@fetcher
@@ -513,7 +504,7 @@ class CacheURLFetchStrategy(URLFetchStrategy):
# check whether the cache file exists.
if not os.path.isfile(path):
- raise NoCacheError("No cache of %s" % path)
+ raise NoCacheError(f"No cache of {path}")
# remove old symlink if one is there.
filename = self.stage.save_filename
@@ -523,8 +514,8 @@ class CacheURLFetchStrategy(URLFetchStrategy):
# Symlink to local cached archive.
symlink(path, filename)
- # Remove link if checksum fails, or subsequent fetchers
- # will assume they don't need to download.
+ # Remove link if checksum fails, or subsequent fetchers will assume they don't need to
+ # download.
if self.digest:
try:
self.check()
@@ -533,12 +524,12 @@ class CacheURLFetchStrategy(URLFetchStrategy):
raise
# Notify the user how we fetched.
- tty.msg("Using cached archive: {0}".format(path))
+ tty.msg(f"Using cached archive: {path}")
class OCIRegistryFetchStrategy(URLFetchStrategy):
- def __init__(self, url=None, checksum=None, **kwargs):
- super().__init__(url, checksum, **kwargs)
+ def __init__(self, *, url: str, checksum: Optional[str] = None, **kwargs):
+ super().__init__(url=url, checksum=checksum, **kwargs)
self._urlopen = kwargs.get("_urlopen", spack.oci.opener.urlopen)
@@ -1381,7 +1372,7 @@ class HgFetchStrategy(VCSFetchStrategy):
shutil.move(scrubbed, source_path)
def __str__(self):
- return "[hg] %s" % self.url
+ return f"[hg] {self.url}"
@fetcher
@@ -1390,47 +1381,16 @@ class S3FetchStrategy(URLFetchStrategy):
url_attr = "s3"
- def __init__(self, *args, **kwargs):
- try:
- super().__init__(*args, **kwargs)
- except ValueError:
- if not kwargs.get("url"):
- raise ValueError("S3FetchStrategy requires a url for fetching.")
-
@_needs_stage
def fetch(self):
+ if not self.url.startswith("s3://"):
+ raise spack.error.FetchError(
+ f"{self.__class__.__name__} can only fetch from s3:// urls."
+ )
if self.archive_file:
tty.debug(f"Already downloaded {self.archive_file}")
return
-
- parsed_url = urllib.parse.urlparse(self.url)
- if parsed_url.scheme != "s3":
- raise spack.error.FetchError("S3FetchStrategy can only fetch from s3:// urls.")
-
- basename = os.path.basename(parsed_url.path)
- request = urllib.request.Request(
- self.url, headers={"User-Agent": web_util.SPACK_USER_AGENT}
- )
-
- with working_dir(self.stage.path):
- try:
- response = web_util.urlopen(request)
- except (TimeoutError, urllib.error.URLError) as e:
- raise FailedDownloadError(e) from e
-
- tty.debug(f"Fetching {self.url}")
-
- with open(basename, "wb") as f:
- shutil.copyfileobj(response, f)
-
- content_type = web_util.get_header(response.headers, "Content-type")
-
- if content_type == "text/html":
- warn_content_type_mismatch(self.archive_file or "the archive")
-
- if self.stage.save_filename:
- fs.rename(os.path.join(self.stage.path, basename), self.stage.save_filename)
-
+ self._fetch_urllib(self.url)
if not self.archive_file:
raise FailedDownloadError(
RuntimeError(f"Missing archive {self.archive_file} after fetching")
@@ -1443,46 +1403,17 @@ class GCSFetchStrategy(URLFetchStrategy):
url_attr = "gs"
- def __init__(self, *args, **kwargs):
- try:
- super().__init__(*args, **kwargs)
- except ValueError:
- if not kwargs.get("url"):
- raise ValueError("GCSFetchStrategy requires a url for fetching.")
-
@_needs_stage
def fetch(self):
+ if not self.url.startswith("gs"):
+ raise spack.error.FetchError(
+ f"{self.__class__.__name__} can only fetch from gs:// urls."
+ )
if self.archive_file:
- tty.debug("Already downloaded {0}".format(self.archive_file))
+ tty.debug(f"Already downloaded {self.archive_file}")
return
- parsed_url = urllib.parse.urlparse(self.url)
- if parsed_url.scheme != "gs":
- raise spack.error.FetchError("GCSFetchStrategy can only fetch from gs:// urls.")
-
- basename = os.path.basename(parsed_url.path)
- request = urllib.request.Request(
- self.url, headers={"User-Agent": web_util.SPACK_USER_AGENT}
- )
-
- with working_dir(self.stage.path):
- try:
- response = web_util.urlopen(request)
- except (TimeoutError, urllib.error.URLError) as e:
- raise FailedDownloadError(e) from e
-
- tty.debug(f"Fetching {self.url}")
-
- with open(basename, "wb") as f:
- shutil.copyfileobj(response, f)
-
- content_type = web_util.get_header(response.headers, "Content-type")
-
- if content_type == "text/html":
- warn_content_type_mismatch(self.archive_file or "the archive")
-
- if self.stage.save_filename:
- os.rename(os.path.join(self.stage.path, basename), self.stage.save_filename)
+ self._fetch_urllib(self.url)
if not self.archive_file:
raise FailedDownloadError(
@@ -1496,7 +1427,7 @@ class FetchAndVerifyExpandedFile(URLFetchStrategy):
as well as after expanding it."""
def __init__(self, url, archive_sha256: str, expanded_sha256: str):
- super().__init__(url, archive_sha256)
+ super().__init__(url=url, checksum=archive_sha256)
self.expanded_sha256 = expanded_sha256
def expand(self):
@@ -1538,14 +1469,14 @@ def stable_target(fetcher):
return False
-def from_url(url):
+def from_url(url: str) -> URLFetchStrategy:
"""Given a URL, find an appropriate fetch strategy for it.
Currently just gives you a URLFetchStrategy that uses curl.
TODO: make this return appropriate fetch strategies for other
types of URLs.
"""
- return URLFetchStrategy(url)
+ return URLFetchStrategy(url=url)
def from_kwargs(**kwargs):
@@ -1614,10 +1545,12 @@ def _check_version_attributes(fetcher, pkg, version):
def _extrapolate(pkg, version):
"""Create a fetcher from an extrapolated URL for this version."""
try:
- return URLFetchStrategy(pkg.url_for_version(version), fetch_options=pkg.fetch_options)
+ return URLFetchStrategy(url=pkg.url_for_version(version), fetch_options=pkg.fetch_options)
except spack.package_base.NoURLError:
- msg = "Can't extrapolate a URL for version %s " "because package %s defines no URLs"
- raise ExtrapolationError(msg % (version, pkg.name))
+ raise ExtrapolationError(
+ f"Can't extrapolate a URL for version {version} because "
+ f"package {pkg.name} defines no URLs"
+ )
def _from_merged_attrs(fetcher, pkg, version):
@@ -1733,11 +1666,9 @@ def for_package_version(pkg, version=None):
raise InvalidArgsError(pkg, version, **args)
-def from_url_scheme(url, *args, **kwargs):
+def from_url_scheme(url: str, **kwargs):
"""Finds a suitable FetchStrategy by matching its url_attr with the scheme
in the given url."""
-
- url = kwargs.get("url", url)
parsed_url = urllib.parse.urlparse(url, scheme="file")
scheme_mapping = kwargs.get("scheme_mapping") or {
@@ -1754,11 +1685,9 @@ def from_url_scheme(url, *args, **kwargs):
for fetcher in all_strategies:
url_attr = getattr(fetcher, "url_attr", None)
if url_attr and url_attr == scheme:
- return fetcher(url, *args, **kwargs)
+ return fetcher(url=url, **kwargs)
- raise ValueError(
- 'No FetchStrategy found for url with scheme: "{SCHEME}"'.format(SCHEME=parsed_url.scheme)
- )
+ raise ValueError(f'No FetchStrategy found for url with scheme: "{parsed_url.scheme}"')
def from_list_url(pkg):
@@ -1783,7 +1712,9 @@ def from_list_url(pkg):
)
# construct a fetcher
- return URLFetchStrategy(url_from_list, checksum, fetch_options=pkg.fetch_options)
+ return URLFetchStrategy(
+ url=url_from_list, checksum=checksum, fetch_options=pkg.fetch_options
+ )
except KeyError as e:
tty.debug(e)
tty.msg("Cannot find version %s in url_list" % pkg.version)
@@ -1811,10 +1742,10 @@ class FsCache:
mkdirp(os.path.dirname(dst))
fetcher.archive(dst)
- def fetcher(self, target_path, digest, **kwargs):
+ def fetcher(self, target_path: str, digest: Optional[str], **kwargs) -> CacheURLFetchStrategy:
path = os.path.join(self.root, target_path)
url = url_util.path_to_file_url(path)
- return CacheURLFetchStrategy(url, digest, **kwargs)
+ return CacheURLFetchStrategy(url=url, checksum=digest, **kwargs)
def destroy(self):
shutil.rmtree(self.root, ignore_errors=True)
diff --git a/lib/spack/spack/oci/oci.py b/lib/spack/spack/oci/oci.py
index cd6ac1dad9..cacb53e08c 100644
--- a/lib/spack/spack/oci/oci.py
+++ b/lib/spack/spack/oci/oci.py
@@ -390,7 +390,7 @@ def make_stage(
) -> spack.stage.Stage:
_urlopen = _urlopen or spack.oci.opener.urlopen
fetch_strategy = spack.fetch_strategy.OCIRegistryFetchStrategy(
- url, checksum=digest.digest, _urlopen=_urlopen
+ url=url, checksum=digest.digest, _urlopen=_urlopen
)
# Use blobs/<alg>/<encoded> as the cache path, which follows
# the OCI Image Layout Specification. What's missing though,
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py
index 0c7fa45ff4..b2630f7c00 100644
--- a/lib/spack/spack/patch.py
+++ b/lib/spack/spack/patch.py
@@ -319,7 +319,7 @@ class UrlPatch(Patch):
self.url, archive_sha256=self.archive_sha256, expanded_sha256=self.sha256
)
else:
- fetcher = fs.URLFetchStrategy(self.url, sha256=self.sha256, expand=False)
+ fetcher = fs.URLFetchStrategy(url=self.url, sha256=self.sha256, expand=False)
# The same package can have multiple patches with the same name but
# with different contents, therefore apply a subset of the hash.
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index fd550fb0cc..26f29572b3 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -501,7 +501,7 @@ class Stage(LockableStagingDir):
fetchers[:0] = (
fs.from_url_scheme(
url_util.join(mirror.fetch_url, rel_path),
- digest,
+ checksum=digest,
expand=expand,
extension=extension,
)
@@ -525,13 +525,13 @@ class Stage(LockableStagingDir):
if self.search_fn and not mirror_only:
yield from self.search_fn()
- def fetch(self, mirror_only=False, err_msg=None):
+ def fetch(self, mirror_only: bool = False, err_msg: Optional[str] = None) -> 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
+ mirror_only: only fetch from a mirror
+ err_msg: the error message to display if all fetchers fail or ``None`` for the default
+ fetch failure message
"""
errors: List[str] = []
for fetcher in self._generate_fetchers(mirror_only):
@@ -593,16 +593,19 @@ class Stage(LockableStagingDir):
self.destroy()
def check(self):
- """Check the downloaded archive against a checksum digest.
- No-op if this stage checks code out of a repository."""
+ """Check the downloaded archive against a checksum digest."""
if self.fetcher is not self.default_fetcher and self.skip_checksum_for_mirror:
+ cache = isinstance(self.fetcher, fs.CacheURLFetchStrategy)
+ if cache:
+ secure_msg = "your download cache is in a secure location"
+ else:
+ secure_msg = "you trust this mirror and have a secure connection"
tty.warn(
- "Fetching from mirror without a checksum!",
- "This package is normally checked out from a version "
- "control system, but it has been archived on a spack "
- "mirror. This means we cannot know a checksum for the "
- "tarball in advance. Be sure that your connection to "
- "this mirror is secure!",
+ f"Using {'download cache' if cache else 'a mirror'} instead of version control",
+ "The required sources are normally checked out from a version control system, "
+ f"but have been archived {'in download cache' if cache else 'on a mirror'}: "
+ f"{self.fetcher}. Spack lacks a tree hash to verify the integrity of this "
+ f"archive. Make sure {secure_msg}.",
)
elif spack.config.get("config:checksum"):
self.fetcher.check()
@@ -1171,7 +1174,7 @@ def _fetch_and_checksum(url, options, keep_stage, action_fn=None):
try:
url_or_fs = url
if options:
- url_or_fs = fs.URLFetchStrategy(url, fetch_options=options)
+ url_or_fs = fs.URLFetchStrategy(url=url, fetch_options=options)
with Stage(url_or_fs, keep=keep_stage) as stage:
# Fetch the archive
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index c3926c67ab..2db0206bec 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -1003,7 +1003,7 @@ def temporary_store(tmpdir, request):
def mock_fetch(mock_archive, monkeypatch):
"""Fake the URL for a package so it downloads from a file."""
monkeypatch.setattr(
- spack.package_base.PackageBase, "fetcher", URLFetchStrategy(mock_archive.url)
+ spack.package_base.PackageBase, "fetcher", URLFetchStrategy(url=mock_archive.url)
)
diff --git a/lib/spack/spack/test/gcs_fetch.py b/lib/spack/spack/test/gcs_fetch.py
index 3689e5780c..ec53f0b633 100644
--- a/lib/spack/spack/test/gcs_fetch.py
+++ b/lib/spack/spack/test/gcs_fetch.py
@@ -3,54 +3,21 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import os
-
-import pytest
-
import spack.config
import spack.error
import spack.fetch_strategy
import spack.stage
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_gcsfetchstrategy_without_url(_fetch_method):
- """Ensure constructor with no URL fails."""
- with spack.config.override("config:url_fetch_method", _fetch_method):
- with pytest.raises(ValueError):
- spack.fetch_strategy.GCSFetchStrategy(None)
-
-
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_gcsfetchstrategy_bad_url(tmpdir, _fetch_method):
- """Ensure fetch with bad URL fails as expected."""
- testpath = str(tmpdir)
-
- with spack.config.override("config:url_fetch_method", _fetch_method):
- fetcher = spack.fetch_strategy.GCSFetchStrategy(url="file:///does-not-exist")
- assert fetcher is not None
-
- with spack.stage.Stage(fetcher, path=testpath) as stage:
- assert stage is not None
- assert fetcher.archive_file is None
- with pytest.raises(spack.error.FetchError):
- fetcher.fetch()
-
-
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_gcsfetchstrategy_downloaded(tmpdir, _fetch_method):
+def test_gcsfetchstrategy_downloaded(tmp_path):
"""Ensure fetch with archive file already downloaded is a noop."""
- testpath = str(tmpdir)
- archive = os.path.join(testpath, "gcs.tar.gz")
-
- with spack.config.override("config:url_fetch_method", _fetch_method):
+ archive = tmp_path / "gcs.tar.gz"
- class Archived_GCSFS(spack.fetch_strategy.GCSFetchStrategy):
- @property
- def archive_file(self):
- return archive
+ class Archived_GCSFS(spack.fetch_strategy.GCSFetchStrategy):
+ @property
+ def archive_file(self):
+ return str(archive)
- url = "gcs:///{0}".format(archive)
- fetcher = Archived_GCSFS(url=url)
- with spack.stage.Stage(fetcher, path=testpath):
- fetcher.fetch()
+ fetcher = Archived_GCSFS(url="gs://example/gcs.tar.gz")
+ with spack.stage.Stage(fetcher, path=str(tmp_path)):
+ fetcher.fetch()
diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py
index 518ade892d..5cb9282225 100644
--- a/lib/spack/spack/test/mirror.py
+++ b/lib/spack/spack/test/mirror.py
@@ -205,7 +205,7 @@ def test_invalid_json_mirror_collection(invalid_json, error_message):
def test_mirror_archive_paths_no_version(mock_packages, mock_archive):
spec = Spec("trivial-install-test-package@=nonexistingversion").concretized()
- fetcher = spack.fetch_strategy.URLFetchStrategy(mock_archive.url)
+ fetcher = spack.fetch_strategy.URLFetchStrategy(url=mock_archive.url)
spack.mirror.mirror_archive_paths(fetcher, "per-package-ref", spec)
diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py
index e79cdd91c8..2356515c05 100644
--- a/lib/spack/spack/test/packaging.py
+++ b/lib/spack/spack/test/packaging.py
@@ -48,7 +48,7 @@ pytestmark = pytest.mark.not_on_windows("does not run on windows")
def test_buildcache(mock_archive, tmp_path, monkeypatch, mutable_config):
# Install a test package
spec = Spec("trivial-install-test-package").concretized()
- monkeypatch.setattr(spec.package, "fetcher", URLFetchStrategy(mock_archive.url))
+ monkeypatch.setattr(spec.package, "fetcher", URLFetchStrategy(url=mock_archive.url))
spec.package.do_install()
pkghash = "/" + str(spec.dag_hash(7))
diff --git a/lib/spack/spack/test/s3_fetch.py b/lib/spack/spack/test/s3_fetch.py
index 66c4cd7bc4..5177c3f90d 100644
--- a/lib/spack/spack/test/s3_fetch.py
+++ b/lib/spack/spack/test/s3_fetch.py
@@ -3,54 +3,19 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import os
-
-import pytest
-
-import spack.config as spack_config
-import spack.error
import spack.fetch_strategy as spack_fs
import spack.stage as spack_stage
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_s3fetchstrategy_sans_url(_fetch_method):
- """Ensure constructor with no URL fails."""
- with spack_config.override("config:url_fetch_method", _fetch_method):
- with pytest.raises(ValueError):
- spack_fs.S3FetchStrategy(None)
-
-
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_s3fetchstrategy_bad_url(tmpdir, _fetch_method):
- """Ensure fetch with bad URL fails as expected."""
- testpath = str(tmpdir)
-
- with spack_config.override("config:url_fetch_method", _fetch_method):
- fetcher = spack_fs.S3FetchStrategy(url="file:///does-not-exist")
- assert fetcher is not None
-
- with spack_stage.Stage(fetcher, path=testpath) as stage:
- assert stage is not None
- assert fetcher.archive_file is None
- with pytest.raises(spack.error.FetchError):
- fetcher.fetch()
-
-
-@pytest.mark.parametrize("_fetch_method", ["curl", "urllib"])
-def test_s3fetchstrategy_downloaded(tmpdir, _fetch_method):
+def test_s3fetchstrategy_downloaded(tmp_path):
"""Ensure fetch with archive file already downloaded is a noop."""
- testpath = str(tmpdir)
- archive = os.path.join(testpath, "s3.tar.gz")
-
- with spack_config.override("config:url_fetch_method", _fetch_method):
+ archive = tmp_path / "s3.tar.gz"
- class Archived_S3FS(spack_fs.S3FetchStrategy):
- @property
- def archive_file(self):
- return archive
+ class Archived_S3FS(spack_fs.S3FetchStrategy):
+ @property
+ def archive_file(self):
+ return archive
- url = "s3:///{0}".format(archive)
- fetcher = Archived_S3FS(url=url)
- with spack_stage.Stage(fetcher, path=testpath):
- fetcher.fetch()
+ fetcher = Archived_S3FS(url="s3://example/s3.tar.gz")
+ with spack_stage.Stage(fetcher, path=str(tmp_path)):
+ fetcher.fetch()
diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py
index 96460cd9b3..b86a2f5cce 100644
--- a/lib/spack/spack/test/url_fetch.py
+++ b/lib/spack/spack/test/url_fetch.py
@@ -76,12 +76,6 @@ def pkg_factory():
return factory
-def test_urlfetchstrategy_sans_url():
- """Ensure constructor with no URL fails."""
- with pytest.raises(ValueError):
- fs.URLFetchStrategy(None)
-
-
@pytest.mark.parametrize("method", ["curl", "urllib"])
def test_urlfetchstrategy_bad_url(tmp_path, mutable_config, method):
"""Ensure fetch with bad URL fails as expected."""
@@ -267,7 +261,7 @@ def test_url_with_status_bar(tmpdir, mock_archive, monkeypatch, capfd):
monkeypatch.setattr(sys.stdout, "isatty", is_true)
monkeypatch.setattr(tty, "msg_enabled", is_true)
with spack.config.override("config:url_fetch_method", "curl"):
- fetcher = fs.URLFetchStrategy(mock_archive.url)
+ fetcher = fs.URLFetchStrategy(url=mock_archive.url)
with Stage(fetcher, path=testpath) as stage:
assert fetcher.archive_file is None
stage.fetch()
@@ -280,7 +274,7 @@ def test_url_with_status_bar(tmpdir, mock_archive, monkeypatch, capfd):
def test_url_extra_fetch(tmp_path, mutable_config, mock_archive, _fetch_method):
"""Ensure a fetch after downloading is effectively a no-op."""
mutable_config.set("config:url_fetch_method", _fetch_method)
- fetcher = fs.URLFetchStrategy(mock_archive.url)
+ fetcher = fs.URLFetchStrategy(url=mock_archive.url)
with Stage(fetcher, path=str(tmp_path)) as stage:
assert fetcher.archive_file is None
stage.fetch()
--
cgit v1.2.3-70-g09d2