From be3e6a0e9b8c243a4aec2a550b6512fe32553c13 Mon Sep 17 00:00:00 2001 From: loulawrence <72574166+loulawrence@users.noreply.github.com> Date: Mon, 2 Aug 2021 04:30:25 -0400 Subject: [PATCH] document config option "url_fetch_method" (#24638) - Change config from the undocumented `use_curl: true/false` to `url_fetch_method: urllib/curl`. - Documentation of `url_fetch_method` in `defaults/config.yaml` - Default fetch option explicitly set to `urllib` for users who may not have curl on their system To upgrade from `use_curl` to `url_fetch_method`, run `spack config update config` --- etc/spack/defaults/config.yaml | 4 +++ lib/spack/spack/fetch_strategy.py | 4 +-- lib/spack/spack/schema/config.py | 10 ++++++ lib/spack/spack/test/cache_fetch.py | 12 +++---- lib/spack/spack/test/s3_fetch.py | 18 +++++----- lib/spack/spack/test/url_fetch.py | 54 ++++++++++++++--------------- 6 files changed, 58 insertions(+), 44 deletions(-) diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml index 2400686e60..64a50cc805 100644 --- a/etc/spack/defaults/config.yaml +++ b/etc/spack/defaults/config.yaml @@ -134,6 +134,10 @@ config: # enabling locks. locks: true + # The default url fetch method to use. + # If set to 'curl', Spack will require curl on the user's system + # If set to 'urllib', Spack will use python built-in libs to fetch + url_fetch_method: urllib # The maximum number of jobs to use for the build system (e.g. `make`), when # the -j flag is not given on the command line. Defaults to 16 when not set. diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index f5eecdcb5d..136d786e3f 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -338,7 +338,7 @@ def fetch(self): def _existing_url(self, url): tty.debug('Checking existence of {0}'.format(url)) - if spack.config.get('config:use_curl'): + if spack.config.get('config:url_fetch_method') == 'curl': curl = self.curl # Telling curl to fetch the first byte (-r 0-0) is supposed to be # portable. @@ -357,7 +357,7 @@ def _existing_url(self, url): return (response.getcode() is None or response.getcode() == 200) def _fetch_from_url(self, url): - if spack.config.get('config:use_curl'): + if spack.config.get('config:url_fetch_method') == 'curl': return self._fetch_curl(url) else: return self._fetch_urllib(url) diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index 1df922b137..0cf533eb18 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -99,6 +99,10 @@ }, 'allow_sgid': {'type': 'boolean'}, 'binary_index_root': {'type': 'string'}, + 'url_fetch_method': { + 'type': 'string', + 'enum': ['urllib', 'curl'] + }, }, }, } @@ -153,4 +157,10 @@ def update(data): update_data = spack.config.merge_yaml(update_data, projections_data) data['install_tree'] = update_data changed = True + + use_curl = data.pop('use_curl', None) + if use_curl is not None: + data['url_fetch_method'] = 'curl' if use_curl else 'urllib' + changed = True + return changed diff --git a/lib/spack/spack/test/cache_fetch.py b/lib/spack/spack/test/cache_fetch.py index 0929bfd12c..a3b9cafed8 100644 --- a/lib/spack/spack/test/cache_fetch.py +++ b/lib/spack/spack/test/cache_fetch.py @@ -14,25 +14,25 @@ from spack.stage import Stage -@pytest.mark.parametrize('use_curl', [True, False]) -def test_fetch_missing_cache(tmpdir, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_fetch_missing_cache(tmpdir, _fetch_method): """Ensure raise a missing cache file.""" testpath = str(tmpdir) - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): fetcher = CacheURLFetchStrategy(url='file:///not-a-real-cache-file') with Stage(fetcher, path=testpath): with pytest.raises(NoCacheError, match=r'No cache'): fetcher.fetch() -@pytest.mark.parametrize('use_curl', [True, False]) -def test_fetch(tmpdir, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_fetch(tmpdir, _fetch_method): """Ensure a fetch after expanding is effectively a no-op.""" testpath = str(tmpdir) cache = os.path.join(testpath, 'cache.tar.gz') touch(cache) url = 'file:///{0}'.format(cache) - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): fetcher = CacheURLFetchStrategy(url=url) with Stage(fetcher, path=testpath) as stage: source_path = stage.source_path diff --git a/lib/spack/spack/test/s3_fetch.py b/lib/spack/spack/test/s3_fetch.py index 2fc9e3de84..fbff60033d 100644 --- a/lib/spack/spack/test/s3_fetch.py +++ b/lib/spack/spack/test/s3_fetch.py @@ -12,20 +12,20 @@ import spack.stage as spack_stage -@pytest.mark.parametrize('use_curl', [True, False]) -def test_s3fetchstrategy_sans_url(use_curl): +@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:use_curl', use_curl): + with spack_config.override('config:url_fetch_method', _fetch_method): with pytest.raises(ValueError): spack_fs.S3FetchStrategy(None) -@pytest.mark.parametrize('use_curl', [True, False]) -def test_s3fetchstrategy_bad_url(tmpdir, use_curl): +@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:use_curl', use_curl): + with spack_config.override('config:url_fetch_method', _fetch_method): fetcher = spack_fs.S3FetchStrategy(url='file:///does-not-exist') assert fetcher is not None @@ -36,13 +36,13 @@ def test_s3fetchstrategy_bad_url(tmpdir, use_curl): fetcher.fetch() -@pytest.mark.parametrize('use_curl', [True, False]) -def test_s3fetchstrategy_downloaded(tmpdir, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_s3fetchstrategy_downloaded(tmpdir, _fetch_method): """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:use_curl', use_curl): + with spack_config.override('config:url_fetch_method', _fetch_method): class Archived_S3FS(spack_fs.S3FetchStrategy): @property def archive_file(self): diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py index b466c40c06..b0bd53af72 100644 --- a/lib/spack/spack/test/url_fetch.py +++ b/lib/spack/spack/test/url_fetch.py @@ -49,20 +49,20 @@ def fn(v): return factory -@pytest.mark.parametrize('use_curl', [True, False]) -def test_urlfetchstrategy_sans_url(use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_urlfetchstrategy_sans_url(_fetch_method): """Ensure constructor with no URL fails.""" - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): with pytest.raises(ValueError): with fs.URLFetchStrategy(None): pass -@pytest.mark.parametrize('use_curl', [True, False]) -def test_urlfetchstrategy_bad_url(tmpdir, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_urlfetchstrategy_bad_url(tmpdir, _fetch_method): """Ensure fetch with bad URL fails as expected.""" testpath = str(tmpdir) - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): with pytest.raises(fs.FailedDownloadError): fetcher = fs.URLFetchStrategy(url='file:///does-not-exist') assert fetcher is not None @@ -75,7 +75,7 @@ def test_urlfetchstrategy_bad_url(tmpdir, use_curl): def test_fetch_options(tmpdir, mock_archive): testpath = str(tmpdir) - with spack.config.override('config:use_curl', True): + with spack.config.override('config:url_fetch_method', 'curl'): fetcher = fs.URLFetchStrategy(url=mock_archive.url, fetch_options={'cookie': 'True', 'timeout': 10}) @@ -87,11 +87,11 @@ def test_fetch_options(tmpdir, mock_archive): fetcher.fetch() -@pytest.mark.parametrize('use_curl', [True, False]) -def test_archive_file_errors(tmpdir, mock_archive, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_archive_file_errors(tmpdir, mock_archive, _fetch_method): """Ensure FetchStrategy commands may only be used as intended""" testpath = str(tmpdir) - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): fetcher = fs.URLFetchStrategy(url=mock_archive.url) assert fetcher is not None with pytest.raises(fs.FailedDownloadError): @@ -112,7 +112,7 @@ def test_archive_file_errors(tmpdir, mock_archive, use_curl): @pytest.mark.parametrize('secure', [True, False]) -@pytest.mark.parametrize('use_curl', [True, False]) +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) @pytest.mark.parametrize('mock_archive', [('.tar.gz', 'z'), ('.tgz', 'z'), ('.tar.bz2', 'j'), ('.tbz2', 'j'), @@ -121,7 +121,7 @@ def test_archive_file_errors(tmpdir, mock_archive, use_curl): def test_fetch( mock_archive, secure, - use_curl, + _fetch_method, checksum_type, config, mutable_mock_repo @@ -147,7 +147,7 @@ def test_fetch( # Enter the stage directory and check some properties with pkg.stage: with spack.config.override('config:verify_ssl', secure): - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): pkg.do_stage() with working_dir(pkg.stage.source_path): assert os.path.exists('configure') @@ -168,13 +168,13 @@ def test_fetch( ('url-list-test @3.0a1', 'foo-3.0a1.tar.gz', 'abc30a1'), ('url-list-test @4.5-rc5', 'foo-4.5-rc5.tar.gz', 'abc45rc5'), ]) -@pytest.mark.parametrize('use_curl', [True, False]) -def test_from_list_url(mock_packages, config, spec, url, digest, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_from_list_url(mock_packages, config, spec, url, digest, _fetch_method): """ Test URLs in the url-list-test package, which means they should have checksums in the package. """ - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): specification = Spec(spec).concretized() pkg = spack.repo.get(specification) fetch_strategy = fs.from_list_url(pkg) @@ -187,10 +187,10 @@ def test_from_list_url(mock_packages, config, spec, url, digest, use_curl): assert fetch_strategy.extra_options == {'timeout': 60} -@pytest.mark.parametrize('use_curl', [True, False]) -def test_from_list_url_unspecified(mock_packages, config, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_from_list_url_unspecified(mock_packages, config, _fetch_method): """Test non-specific URLs from the url-list-test package.""" - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): pkg = spack.repo.get('url-list-test') spec = Spec('url-list-test @2.0.0').concretized() @@ -236,7 +236,7 @@ def is_true(): monkeypatch.setattr(sys.stdout, 'isatty', is_true) monkeypatch.setattr(tty, 'msg_enabled', is_true) - with spack.config.override('config:use_curl', True): + with spack.config.override('config:url_fetch_method', 'curl'): fetcher = fs.URLFetchStrategy(mock_archive.url) with Stage(fetcher, path=testpath) as stage: assert fetcher.archive_file is None @@ -246,10 +246,10 @@ def is_true(): assert '##### 100' in status -@pytest.mark.parametrize('use_curl', [True, False]) -def test_url_extra_fetch(tmpdir, mock_archive, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_url_extra_fetch(tmpdir, mock_archive, _fetch_method): """Ensure a fetch after downloading is effectively a no-op.""" - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): testpath = str(tmpdir) fetcher = fs.URLFetchStrategy(mock_archive.url) with Stage(fetcher, path=testpath) as stage: @@ -267,12 +267,12 @@ def test_url_extra_fetch(tmpdir, mock_archive, use_curl): ['https://ftpmirror.gnu.org/autoconf/autoconf-2.62.tar.gz', 'https://ftp.gnu.org/gnu/autoconf/autoconf-2.62.tar.gz']) ]) -@pytest.mark.parametrize('use_curl', [True, False]) -def test_candidate_urls(pkg_factory, url, urls, version, expected, use_curl): +@pytest.mark.parametrize('_fetch_method', ['curl', 'urllib']) +def test_candidate_urls(pkg_factory, url, urls, version, expected, _fetch_method): """Tests that candidate urls include mirrors and that they go through pattern matching and substitution for versions. """ - with spack.config.override('config:use_curl', use_curl): + with spack.config.override('config:url_fetch_method', _fetch_method): pkg = pkg_factory(url, urls) f = fs._from_merged_attrs(fs.URLFetchStrategy, pkg, version) assert f.candidate_urls == expected @@ -298,7 +298,7 @@ def _which(*args, **kwargs): testpath = str(tmpdir) url = 'http://github.com/spack/spack' - with spack.config.override('config:use_curl', True): + with spack.config.override('config:url_fetch_method', 'curl'): fetcher = fs.URLFetchStrategy(url=url) assert fetcher is not None with pytest.raises(TypeError, match='object is not callable'):