From 498d8cf04b6c932d8c224c7b3b9f696fd7dd3e27 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Wed, 18 Jul 2018 20:10:46 -0700 Subject: [PATCH] core: fixes and tests for handling of fetcher attributes in packages - Packages can remove the top-level `url` attribute and still work - These are now legal: - Packages with *only* version-specific URLs (even with gaps) - Packages with a top-level git/hg/svn attribute and `version` directives for that. - If a package has both a top-level hg/git/svn attribute AND a top-level url attribute, the url attribute takes precedence. --- lib/spack/spack/fetch_strategy.py | 24 +++- lib/spack/spack/package.py | 76 ++++++++---- lib/spack/spack/test/packages.py | 114 ++++++++++++++++-- .../packages/git-and-url-top-level/package.py | 38 ++++++ .../packages/git-top-level/package.py | 36 ++++++ .../packages/hg-top-level/package.py | 36 ++++++ .../packages/svn-top-level/package.py | 35 ++++++ .../url-only-override-with-gaps/package.py | 37 ++++++ .../packages/url-only-override/package.py | 33 +++++ 9 files changed, 389 insertions(+), 40 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/git-and-url-top-level/package.py create mode 100644 var/spack/repos/builtin.mock/packages/git-top-level/package.py create mode 100644 var/spack/repos/builtin.mock/packages/hg-top-level/package.py create mode 100644 var/spack/repos/builtin.mock/packages/svn-top-level/package.py create mode 100644 var/spack/repos/builtin.mock/packages/url-only-override-with-gaps/package.py create mode 100644 var/spack/repos/builtin.mock/packages/url-only-override/package.py diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 8aa38a4c01..a497342379 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -971,9 +971,18 @@ def args_are_for(args, fetcher): def for_package_version(pkg, version): """Determine a fetch strategy based on the arguments supplied to version() in the package description.""" - # If it's not a known version, extrapolate one. + if not isinstance(version, Version): + version = Version(version) + + # If it's not a known version, extrapolate one by URL. if version not in pkg.versions: - url = pkg.url_for_version(version) + try: + url = pkg.url_for_version(version) + except spack.package.NoURLError: + msg = ("Can't extrapolate a URL for version %s " + "because package %s defines no URLs") + raise ExtrapolationError(msg % (version, pkg.name)) + if not url: raise InvalidArgsError(pkg, version) return URLFetchStrategy(url) @@ -987,10 +996,11 @@ def for_package_version(pkg, version): return fetcher(**args) # If nothing matched for a *specific* version, test all strategies - # against + # against attributes in the version directives and on the package for fetcher in all_strategies: - attrs = dict((attr, getattr(pkg, attr, None)) - for attr in fetcher.required_attributes) + attrs = dict((attr, getattr(pkg, attr)) + for attr in fetcher.required_attributes + if hasattr(pkg, attr)) if 'url' in attrs: attrs['url'] = pkg.url_for_version(version) attrs.update(args) @@ -1080,6 +1090,10 @@ class NoDigestError(FetchError): """Raised after attempt to checksum when URL has no digest.""" +class ExtrapolationError(FetchError): + """Raised when we can't extrapolate a version for a package.""" + + class InvalidArgsError(FetchError): def __init__(self, pkg, version): msg = ("Could not construct a fetch strategy for package %s at " diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 8d6b2b99f5..22d986a6d5 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -45,6 +45,7 @@ from six import StringIO from six import string_types from six import with_metaclass +from ordereddict_backport import OrderedDict import llnl.util.tty as tty @@ -487,13 +488,6 @@ def __init__(self, spec): except ValueError as e: raise ValueError("In package %s: %s" % (self.name, e.message)) - # stage used to build this package. - self._stage = None - - # Init fetch strategy and url to None - self._fetcher = None - self.url = getattr(self.__class__, 'url', None) - # Set a default list URL (place to find available versions) if not hasattr(self, 'list_url'): self.list_url = None @@ -501,15 +495,17 @@ def __init__(self, spec): if not hasattr(self, 'list_depth'): self.list_depth = 0 - # Set up some internal variables for timing. + # init internal variables + self._stage = None + self._fetcher = None + + # Set up timing variables self._fetch_time = 0.0 self._total_time = 0.0 if self.is_extension: spack.repo.get(self.extendee_spec)._check_extendable() - self.extra_args = {} - super(PackageBase, self).__init__() def possible_dependencies( @@ -577,17 +573,46 @@ def version(self): @memoized def version_urls(self): - """Return a list of URLs for different versions of this - package, sorted by version. A version's URL only appears - in this list if it has an explicitly defined URL.""" - version_urls = {} - for v in sorted(self.versions): - args = self.versions[v] + """OrderedDict of explicitly defined URLs for versions of this package. + + Return: + An OrderedDict (version -> URL) different versions of this + package, sorted by version. + + A version's URL only appears in the result if it has an an + explicitly defined ``url`` argument. So, this list may be empty + if a package only defines ``url`` at the top level. + """ + version_urls = OrderedDict() + for v, args in sorted(self.versions.items()): if 'url' in args: version_urls[v] = args['url'] return version_urls - # TODO: move this out of here and into some URL extrapolation module? + def nearest_url(self, version): + """Finds the URL with the "closest" version to ``version``. + + This uses the following precedence order: + + 1. Find the next lowest or equal version with a URL. + 2. If no lower URL, return the next *higher* URL. + 3. If no higher URL, return None. + + """ + version_urls = self.version_urls() + + if version in version_urls: + return version_urls[version] + + last_url = None + for v, u in self.version_urls().items(): + if v > version: + if last_url: + return last_url + last_url = u + + return last_url + def url_for_version(self, version): """Returns a URL from which the specified version of this package may be downloaded. @@ -600,17 +625,22 @@ def url_for_version(self, version): if not isinstance(version, Version): version = Version(version) - cls = self.__class__ - if not (hasattr(cls, 'url') or self.version_urls()): - raise NoURLError(cls) - # If we have a specific URL for this version, don't extrapolate. version_urls = self.version_urls() if version in version_urls: return version_urls[version] - # If we have no idea, substitute the version into the default URL. - default_url = getattr(self.__class__, 'url', None) + # If no specific URL, use the default, class-level URL + default_url = getattr(self, 'url', None) + + # if no exact match AND no class-level default, use the nearest URL + if not default_url: + default_url = self.nearest_url(version) + + # if there are NO URLs to go by, then we can't do anything + if not default_url: + raise NoURLError(self.__class__) + return spack.url.substitute_version( default_url, self.url_version(version)) diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index 8f9838ce78..da46283593 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -26,6 +26,7 @@ import pytest import spack.repo +import spack.fetch_strategy from spack.paths import mock_packages_path from spack.util.naming import mod_to_class from spack.spec import Spec @@ -166,17 +167,106 @@ def test_import_namespace_container_modules(self): import spack.pkg.builtin.mock as m # noqa from spack.pkg.builtin import mock # noqa - @pytest.mark.regression('2737') - def test_urls_for_versions(self): - # Checks that a version directive without a 'url' argument - # specified uses the default url - for spec_str in ('url_override@0.9.0', 'url_override@1.0.0'): - s = Spec(spec_str).concretized() - url = s.package.url_for_version('0.9.0') - assert url == 'http://www.anothersite.org/uo-0.9.0.tgz' - url = s.package.url_for_version('1.0.0') - assert url == 'http://www.doesnotexist.org/url_override-1.0.0.tar.gz' +@pytest.mark.regression('2737') +def test_urls_for_versions(mock_packages, config): + """Version directive without a 'url' argument should use default url.""" + for spec_str in ('url_override@0.9.0', 'url_override@1.0.0'): + s = Spec(spec_str).concretized() + url = s.package.url_for_version('0.9.0') + assert url == 'http://www.anothersite.org/uo-0.9.0.tgz' - url = s.package.url_for_version('0.8.1') - assert url == 'http://www.doesnotexist.org/url_override-0.8.1.tar.gz' + url = s.package.url_for_version('1.0.0') + assert url == 'http://www.doesnotexist.org/url_override-1.0.0.tar.gz' + + url = s.package.url_for_version('0.8.1') + assert url == 'http://www.doesnotexist.org/url_override-0.8.1.tar.gz' + + +def test_url_for_version_with_no_urls(): + pkg = spack.repo.get('git-test') + with pytest.raises(spack.package.NoURLError): + pkg.url_for_version('1.0') + + with pytest.raises(spack.package.NoURLError): + pkg.url_for_version('1.1') + + +def test_url_for_version_with_only_overrides(mock_packages, config): + spec = Spec('url-only-override') + spec.concretize() + + pkg = spack.repo.get(spec) + + # these exist and should just take the URL provided in the package + assert pkg.url_for_version('1.0.0') == 'http://a.example.com/url_override-1.0.0.tar.gz' + assert pkg.url_for_version('0.9.0') == 'http://b.example.com/url_override-0.9.0.tar.gz' + assert pkg.url_for_version('0.8.1') == 'http://c.example.com/url_override-0.8.1.tar.gz' + + # these don't exist but should still work, even if there are only overrides + assert pkg.url_for_version('1.0.5') == 'http://a.example.com/url_override-1.0.5.tar.gz' + assert pkg.url_for_version('0.9.5') == 'http://b.example.com/url_override-0.9.5.tar.gz' + assert pkg.url_for_version('0.8.5') == 'http://c.example.com/url_override-0.8.5.tar.gz' + assert pkg.url_for_version('0.7.0') == 'http://c.example.com/url_override-0.7.0.tar.gz' + + +def test_url_for_version_with_only_overrides_with_gaps(mock_packages, config): + spec = Spec('url-only-override-with-gaps') + spec.concretize() + + pkg = spack.repo.get(spec) + + # same as for url-only-override -- these are specific + assert pkg.url_for_version('1.0.0') == 'http://a.example.com/url_override-1.0.0.tar.gz' + assert pkg.url_for_version('0.9.0') == 'http://b.example.com/url_override-0.9.0.tar.gz' + assert pkg.url_for_version('0.8.1') == 'http://c.example.com/url_override-0.8.1.tar.gz' + + # these don't have specific URLs, but should still work by extrapolation + assert pkg.url_for_version('1.0.5') == 'http://a.example.com/url_override-1.0.5.tar.gz' + assert pkg.url_for_version('0.9.5') == 'http://b.example.com/url_override-0.9.5.tar.gz' + assert pkg.url_for_version('0.8.5') == 'http://c.example.com/url_override-0.8.5.tar.gz' + assert pkg.url_for_version('0.7.0') == 'http://c.example.com/url_override-0.7.0.tar.gz' + + +def test_git_top_level(mock_packages, config): + """Ensure that top-level git attribute can be used as a default.""" + pkg = spack.repo.get('git-top-level') + + fetcher = spack.fetch_strategy.for_package_version(pkg, '1.0') + assert isinstance(fetcher, spack.fetch_strategy.GitFetchStrategy) + assert fetcher.url == 'https://example.com/some/git/repo' + + +def test_svn_top_level(mock_packages, config): + """Ensure that top-level svn attribute can be used as a default.""" + pkg = spack.repo.get('svn-top-level') + + fetcher = spack.fetch_strategy.for_package_version(pkg, '1.0') + assert isinstance(fetcher, spack.fetch_strategy.SvnFetchStrategy) + assert fetcher.url == 'https://example.com/some/svn/repo' + + +def test_hg_top_level(mock_packages, config): + """Ensure that top-level hg attribute can be used as a default.""" + pkg = spack.repo.get('hg-top-level') + + fetcher = spack.fetch_strategy.for_package_version(pkg, '1.0') + assert isinstance(fetcher, spack.fetch_strategy.HgFetchStrategy) + assert fetcher.url == 'https://example.com/some/hg/repo' + + +def test_no_extrapolate_without_url(mock_packages, config): + """Verify that we can't extrapolate versions for non-URL packages.""" + pkg = spack.repo.get('git-top-level') + + with pytest.raises(spack.fetch_strategy.ExtrapolationError): + spack.fetch_strategy.for_package_version(pkg, '1.1') + + +def test_git_and_url_top_level(mock_packages, config): + """Verify that URL takes precedence over other top-level attributes.""" + pkg = spack.repo.get('git-and-url-top-level') + + fetcher = spack.fetch_strategy.for_package_version(pkg, '2.0') + assert isinstance(fetcher, spack.fetch_strategy.URLFetchStrategy) + assert fetcher.url == 'https://example.com/some/tarball-2.0.tar.gz' diff --git a/var/spack/repos/builtin.mock/packages/git-and-url-top-level/package.py b/var/spack/repos/builtin.mock/packages/git-and-url-top-level/package.py new file mode 100644 index 0000000000..d34c9d9b06 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/git-and-url-top-level/package.py @@ -0,0 +1,38 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class GitAndUrlTopLevel(Package): + """Mock package that uses git for fetching.""" + homepage = "http://www.git-fetch-example.com" + + git = 'https://example.com/some/git/repo' + url = 'https://example.com/some/tarball-1.0.tar.gz' + + version('2.0') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/git-top-level/package.py b/var/spack/repos/builtin.mock/packages/git-top-level/package.py new file mode 100644 index 0000000000..5c0be81101 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/git-top-level/package.py @@ -0,0 +1,36 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class GitTopLevel(Package): + """Mock package that uses git for fetching.""" + homepage = "http://www.git-fetch-example.com" + + git = 'https://example.com/some/git/repo' + version('1.0') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/hg-top-level/package.py b/var/spack/repos/builtin.mock/packages/hg-top-level/package.py new file mode 100644 index 0000000000..94a06cd38e --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/hg-top-level/package.py @@ -0,0 +1,36 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class HgTopLevel(Package): + """Test package that does fetching with mercurial.""" + homepage = "http://www.hg-fetch-example.com" + + hg = 'https://example.com/some/hg/repo' + version('1.0') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/svn-top-level/package.py b/var/spack/repos/builtin.mock/packages/svn-top-level/package.py new file mode 100644 index 0000000000..3198b16435 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/svn-top-level/package.py @@ -0,0 +1,35 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class SvnTopLevel(Package): + """Mock package that uses svn for fetching.""" + + svn = 'https://example.com/some/svn/repo' + version('1.0') + + def install(self, spec, prefix): + pass diff --git a/var/spack/repos/builtin.mock/packages/url-only-override-with-gaps/package.py b/var/spack/repos/builtin.mock/packages/url-only-override-with-gaps/package.py new file mode 100644 index 0000000000..8bcbb386bd --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/url-only-override-with-gaps/package.py @@ -0,0 +1,37 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class UrlOnlyOverrideWithGaps(Package): + homepage = 'http://www.example.com' + + version('1.0.5', 'abcdef0') + version('1.0.0', 'bcdef0a', url='http://a.example.com/url_override-1.0.0.tar.gz') + version('0.9.5', 'cdef0ab') + version('0.9.0', 'def0abc', url='http://b.example.com/url_override-0.9.0.tar.gz') + version('0.8.5', 'ef0abcd') + version('0.8.1', 'f0abcde', url='http://c.example.com/url_override-0.8.1.tar.gz') + version('0.7.0', '0abcdef') diff --git a/var/spack/repos/builtin.mock/packages/url-only-override/package.py b/var/spack/repos/builtin.mock/packages/url-only-override/package.py new file mode 100644 index 0000000000..76999fb21e --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/url-only-override/package.py @@ -0,0 +1,33 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +from spack import * + + +class UrlOnlyOverride(Package): + homepage = 'http://www.example.com' + + version('1.0.0', 'cxyzab', url='http://a.example.com/url_override-1.0.0.tar.gz') + version('0.9.0', 'bcxyza', url='http://b.example.com/url_override-0.9.0.tar.gz') + version('0.8.1', 'cxyzab', url='http://c.example.com/url_override-0.8.1.tar.gz')