From bd3ffc7b76f4eda034765d1eadf507614f0ec098 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 22 Jul 2018 17:54:15 -0700 Subject: core: use sha256 instead of md5 for `spack checksum` and `spack create` - This changes `get_checksums_for_versions` to generate code that uses an explicit `sha256` argument instead if the bare `md5` hash we used to generate. - also use a generic digest parameter for the `version` directive, rather than a specific `md5` parameter. --- lib/spack/spack/directives.py | 15 ++--- lib/spack/spack/fetch_strategy.py | 41 ++++++++----- lib/spack/spack/patch.py | 2 +- lib/spack/spack/test/url_fetch.py | 69 ++++++++++++++++++---- lib/spack/spack/util/web.py | 4 +- .../builtin.mock/packages/url-list-test/package.py | 14 ++--- 6 files changed, 102 insertions(+), 43 deletions(-) diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 56490f8452..5137320e98 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -240,16 +240,17 @@ directive = DirectiveMeta.directive @directive('versions') def version(ver, checksum=None, **kwargs): - """Adds a version and metadata describing how to fetch it. - Metadata is just stored as a dict in the package's versions - dictionary. Package must turn it into a valid fetch strategy - later. + """Adds a version and metadata describing how to fetch its source code. + + Metadata is stored as a dict of ``kwargs`` in the package class's + ``versions`` dictionary. + + The ``dict`` of arguments is turned into a valid fetch strategy + later. See ``spack.fetch_strategy.for_package_version()``. """ def _execute_version(pkg): - # TODO: checksum vs md5 distinction is confusing -- fix this. - # special case checksum for backward compatibility if checksum: - kwargs['md5'] = checksum + kwargs['checksum'] = checksum # Store kwargs for the package to later with a fetch_strategy. pkg.versions[Version(ver)] = kwargs diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 3a08deb754..8aa38a4c01 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -181,19 +181,18 @@ class URLFetchStrategy(FetchStrategy): enabled = True required_attributes = ['url'] - def __init__(self, url=None, digest=None, **kwargs): + def __init__(self, url=None, checksum=None, **kwargs): super(URLFetchStrategy, self).__init__() - # If URL or digest are provided in the kwargs, then prefer - # those values. - self.url = kwargs.get('url', None) - if not self.url: - self.url = url + # Prefer values in kwargs to the positionals. + self.url = kwargs.get('url', url) - self.digest = next((kwargs[h] for h in crypto.hashes if h in kwargs), - None) - if not self.digest: - self.digest = digest + # digest can be set as the first argument, or from an explicit + # kwarg by the hash name. + self.digest = kwargs.get('checksum', checksum) + for h in crypto.hashes: + if h in kwargs: + self.digest = kwargs[h] self.expand_archive = kwargs.get('expand', True) self.extra_curl_options = kwargs.get('curl_options', []) @@ -1009,15 +1008,25 @@ def from_list_url(pkg): try: versions = pkg.fetch_remote_versions() try: + # get a URL, and a checksum if we have it url_from_list = versions[pkg.version] - digest = None - if pkg.version in pkg.versions: - digest = pkg.versions[pkg.version].get('md5', None) - return URLFetchStrategy(url=url_from_list, digest=digest) + checksum = None + + # try to find a known checksum for version, from the package + version = pkg.version + if version in pkg.versions: + args = pkg.versions[version] + checksum = next( + (v for k, v in args.items() if k in crypto.hashes), + args.get('checksum')) + + # construct a fetcher + return URLFetchStrategy(url_from_list, checksum) except KeyError: - tty.msg("Can not find version %s in url_list" % - pkg.version) + tty.msg("Cannot find version %s in url_list" % pkg.version) + except BaseException: + # TODO: Don't catch BaseException here! Be more specific. tty.msg("Could not determine url from list_url.") diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index ffa94d964f..f1b6b1a88c 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -153,7 +153,7 @@ class UrlPatch(Patch): if self.archive_sha256: fetch_digest = self.archive_sha256 - fetcher = fs.URLFetchStrategy(self.url, digest=fetch_digest) + fetcher = fs.URLFetchStrategy(self.url, fetch_digest) mirror = os.path.join( os.path.dirname(stage.mirror_path), os.path.basename(self.url)) diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py index 780b4bdf58..2e6e1912bf 100644 --- a/lib/spack/spack/test/url_fetch.py +++ b/lib/spack/spack/test/url_fetch.py @@ -83,16 +83,65 @@ def test_fetch( def test_from_list_url(mock_packages, config): pkg = spack.repo.get('url-list-test') - for ver_str in ['0.0.0', '1.0.0', '2.0.0', - '3.0', '4.5', '2.0.0b2', - '3.0a1', '4.5-rc5']: - spec = Spec('url-list-test@%s' % ver_str) - spec.concretize() - pkg.spec = spec - fetch_strategy = from_list_url(pkg) - assert isinstance(fetch_strategy, URLFetchStrategy) - assert (os.path.basename(fetch_strategy.url) == - ('foo-' + ver_str + '.tar.gz')) + + # These URLs are all in the url-list-test package and should have + # checksums taken from the package. + spec = Spec('url-list-test @0.0.0').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-0.0.0.tar.gz' + assert fetch_strategy.digest == 'abc000' + + spec = Spec('url-list-test @1.0.0').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-1.0.0.tar.gz' + assert fetch_strategy.digest == 'abc100' + + spec = Spec('url-list-test @3.0').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-3.0.tar.gz' + assert fetch_strategy.digest == 'abc30' + + spec = Spec('url-list-test @4.5').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-4.5.tar.gz' + assert fetch_strategy.digest == 'abc45' + + spec = Spec('url-list-test @2.0.0b2').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0b2.tar.gz' + assert fetch_strategy.digest == 'abc200b2' + + spec = Spec('url-list-test @3.0a1').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-3.0a1.tar.gz' + assert fetch_strategy.digest == 'abc30a1' + + spec = Spec('url-list-test @4.5-rc5').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-4.5-rc5.tar.gz' + assert fetch_strategy.digest == 'abc45rc5' + + # this one is not in the url-list-test package. + spec = Spec('url-list-test @2.0.0').concretized() + pkg = spack.repo.get(spec) + fetch_strategy = from_list_url(pkg) + assert isinstance(fetch_strategy, URLFetchStrategy) + assert os.path.basename(fetch_strategy.url) == 'foo-2.0.0.tar.gz' + assert fetch_strategy.digest is None def test_hash_detection(checksum_type): diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py index b9f94fcc35..9777859fa8 100644 --- a/lib/spack/spack/util/web.py +++ b/lib/spack/spack/util/web.py @@ -404,7 +404,7 @@ def get_checksums_for_versions( # Checksum the archive and add it to the list version_hashes.append((version, spack.util.crypto.checksum( - hashlib.md5, stage.archive_file))) + hashlib.sha256, stage.archive_file))) i += 1 except spack.stage.FailedDownloadError: tty.msg("Failed to fetch {0}".format(url)) @@ -420,7 +420,7 @@ def get_checksums_for_versions( # Generate the version directives to put in a package.py version_lines = "\n".join([ - " version('{0}', {1}'{2}')".format( + " version('{0}', {1}sha256='{2}')".format( v, ' ' * (max_len - len(str(v))), h) for v, h in version_hashes ]) diff --git a/var/spack/repos/builtin.mock/packages/url-list-test/package.py b/var/spack/repos/builtin.mock/packages/url-list-test/package.py index fff04d7a9e..2adb06272a 100644 --- a/var/spack/repos/builtin.mock/packages/url-list-test/package.py +++ b/var/spack/repos/builtin.mock/packages/url-list-test/package.py @@ -37,13 +37,13 @@ class UrlListTest(Package): list_url = 'file://' + web_data_path + '/index.html' list_depth = 3 - version('0.0.0') - version('1.0.0') - version('3.0') - version('4.5') - version('2.0.0b2') - version('3.0a1') - version('4.5-rc5') + version('0.0.0', 'abc000') + version('1.0.0', 'abc100') + version('3.0', 'abc30') + version('4.5', 'abc45') + version('2.0.0b2', 'abc200b2') + version('3.0a1', 'abc30a1') + version('4.5-rc5', 'abc45rc5') def install(self, spec, prefix): pass -- cgit v1.2.3-60-g2f50