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

@@ -240,16 +240,17 @@ def remove_directives(arg):
 
 @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

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.")
 
 

lib/spack/spack/patch.py

@@ -153,7 +153,7 @@ def apply(self, stage):
         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))

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):

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
     ])
 

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