package: Add fetch_options variable (#15317)

PR #15212 added a new connect_timeout option that can be overridden
using fetch_options but had to specified per-version. This adds a new
per-package variable that can be used to override fetch_options for
all versions in the package. This includes connect_timeout as well
as 'cookie' (e.g. for the jdk package).

Packages can combine package-level fetch_options with per-version
fetch_options, in which case the version fetch_options completely
override the package-level fetch_options.

This commit includes tests for the added behavior.

4 files changed, 49 insertions, 7 deletions

diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index 85e93164a8..d7613ae58a 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -256,7 +256,7 @@ class URLFetchStrategy(FetchStrategy):
                 self.digest = kwargs[h]
 
         self.expand_archive = kwargs.get('expand', True)
-        self.extra_options = kwargs.get('fetch_options', [])
+        self.extra_options = kwargs.get('fetch_options', {})
         self._curl = None
 
         self.extension = kwargs.get('extension', None)
@@ -1247,7 +1247,8 @@ 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))
+        return URLFetchStrategy(pkg.url_for_version(version),
+                                fetch_options=pkg.fetch_options)
     except spack.package.NoURLError:
         msg = ("Can't extrapolate a URL for version %s "
                "because package %s defines no URLs")
@@ -1267,6 +1268,7 @@ def _from_merged_attrs(fetcher, pkg, version):
         url = getattr(pkg, fetcher.url_attr)
         attrs = {fetcher.url_attr: url}
 
+    attrs['fetch_options'] = pkg.fetch_options
     attrs.update(pkg.versions[version])
     return fetcher(**attrs)
 
@@ -1289,8 +1291,10 @@ def for_package_version(pkg, version):
     if version not in pkg.versions:
         return _extrapolate(pkg, version)
 
+    # Set package args first so version args can override them
+    args = {'fetch_options': pkg.fetch_options}
     # Grab a dict of args out of the package version dict
-    args = pkg.versions[version]
+    args.update(pkg.versions[version])
 
     # If the version specifies a `url_attr` directly, use that.
     for fetcher in all_strategies:
@@ -1370,7 +1374,8 @@ def from_list_url(pkg):
                         args.get('checksum'))
 
                 # construct a fetcher
-                return URLFetchStrategy(url_from_list, checksum)
+                return URLFetchStrategy(url_from_list, checksum,
+                                        fetch_options=pkg.fetch_options)
             except KeyError as e:
                 tty.debug(e)
                 tty.msg("Cannot find version %s in url_list" % pkg.version)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index ff45355055..2c8917a9ac 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -477,6 +477,9 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
     #: This is currently only used by package sanity tests.
     manual_download = False
 
+    #: Set of additional options used when fetching package versions.
+    fetch_options = {}
+
     #
     # Set default licensing information
     #
diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py
index 299c56481e..ffaad396c1 100644
--- a/lib/spack/spack/test/packages.py
+++ b/lib/spack/spack/test/packages.py
@@ -402,3 +402,24 @@ def test_bundle_patch_directive(mock_directive_bundle,
                        match="Patches are not allowed"):
         patch = spack.directives.patch('mock/patch.txt')
         patch(mock_directive_bundle)
+
+
+def test_fetch_options(mock_packages, config):
+    """Test fetch options inference."""
+
+    pkg = spack.repo.get('fetch-options')
+
+    fetcher = spack.fetch_strategy.for_package_version(pkg, '1.0')
+    assert isinstance(fetcher, spack.fetch_strategy.URLFetchStrategy)
+    assert fetcher.digest == 'abc10'
+    assert fetcher.extra_options == {'timeout': 42, 'cookie': 'foobar'}
+
+    fetcher = spack.fetch_strategy.for_package_version(pkg, '1.1')
+    assert isinstance(fetcher, spack.fetch_strategy.URLFetchStrategy)
+    assert fetcher.digest == 'abc11'
+    assert fetcher.extra_options == {'timeout': 65}
+
+    fetcher = spack.fetch_strategy.for_package_version(pkg, '1.2')
+    assert isinstance(fetcher, spack.fetch_strategy.URLFetchStrategy)
+    assert fetcher.digest == 'abc12'
+    assert fetcher.extra_options == {'cookie': 'baz'}
diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py
index 679240049d..20648b4766 100644
--- a/lib/spack/spack/test/url_fetch.py
+++ b/lib/spack/spack/test/url_fetch.py
@@ -26,10 +26,10 @@ def checksum_type(request):
 @pytest.fixture
 def pkg_factory():
     Pkg = collections.namedtuple(
-        'Pkg', ['url_for_version', 'urls', 'url', 'versions']
+        'Pkg', ['url_for_version', 'urls', 'url', 'versions', 'fetch_options']
     )
 
-    def factory(url, urls):
+    def factory(url, urls, fetch_options={}):
 
         def fn(v):
             main_url = url or urls[0]
@@ -37,7 +37,8 @@ def pkg_factory():
 
         return Pkg(
             url_for_version=fn, url=url, urls=urls,
-            versions=collections.defaultdict(dict)
+            versions=collections.defaultdict(dict),
+            fetch_options=fetch_options
         )
 
     return factory
@@ -130,6 +131,10 @@ def test_from_list_url(mock_packages, config, spec, url, digest):
     assert isinstance(fetch_strategy, fs.URLFetchStrategy)
     assert os.path.basename(fetch_strategy.url) == url
     assert fetch_strategy.digest == digest
+    assert fetch_strategy.extra_options == {}
+    pkg.fetch_options = {'timeout': 60}
+    fetch_strategy = fs.from_list_url(pkg)
+    assert fetch_strategy.extra_options == {'timeout': 60}
 
 
 def test_from_list_url_unspecified(mock_packages, config):
@@ -142,6 +147,10 @@ def test_from_list_url_unspecified(mock_packages, config):
     assert isinstance(fetch_strategy, fs.URLFetchStrategy)
     assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0.tar.gz'
     assert fetch_strategy.digest is None
+    assert fetch_strategy.extra_options == {}
+    pkg.fetch_options = {'timeout': 60}
+    fetch_strategy = fs.from_list_url(pkg)
+    assert fetch_strategy.extra_options == {'timeout': 60}
 
 
 def test_nosource_from_list_url(mock_packages, config):
@@ -191,3 +200,7 @@ def test_candidate_urls(pkg_factory, url, urls, version, expected):
     pkg = pkg_factory(url, urls)
     f = fs._from_merged_attrs(fs.URLFetchStrategy, pkg, version)
     assert f.candidate_urls == expected
+    assert f.extra_options == {}
+    pkg = pkg_factory(url, urls, fetch_options={'timeout': 60})
+    f = fs._from_merged_attrs(fs.URLFetchStrategy, pkg, version)
+    assert f.extra_options == {'timeout': 60}