`content_hash()`: make it work on abstract specs

Some test cases had to be modified in a kludgy way so that abstract specs made
concrete would have versions on them. We shouldn't *need* to do this, as the
only reason we care is because the content hash has to be able to get an archive
for a version.

This modifies the content hash so that it can be called on abstract specs,
including only relevant content.

This does NOT add a partial content hash to the DAG hash, as we do not really
want that -- we don't need in-memory spec hashes to need to load package files.
It just makes `Package.content_hash()` less prickly and tests easier to
understand.

4 files changed, 78 insertions, 40 deletions

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 55129d318e..9fc227baec 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1673,44 +1673,62 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
         return patches
 
     def content_hash(self, content=None):
-        """Create a hash based on the sources and logic used to build the
-        package. This includes the contents of all applied patches and the
-        contents of applicable functions in the package subclass."""
-        if not self.spec.concrete:
-            err_msg = ("Cannot invoke content_hash on a package"
-                       " if the associated spec is not concrete")
-            raise spack.error.SpackError(err_msg)
+        """Create a hash based on the artifacts and patches used to build this package.
 
-        hash_content = list()
-        try:
-            source_id = fs.for_package_version(self, self.version).source_id()
-        except (fs.ExtrapolationError, fs.InvalidArgsError):
-            # ExtrapolationError happens if the package has no fetchers defined.
-            # InvalidArgsError happens when there are version directives with args,
-            #     but none of them identifies an actual fetcher.
-            source_id = None
-
-        if not source_id:
-            # TODO? in cases where a digest or source_id isn't available,
-            # should this attempt to download the source and set one? This
-            # probably only happens for source repositories which are
-            # referenced by branch name rather than tag or commit ID.
-            env = spack.environment.active_environment()
-            from_local_sources = env and env.is_develop(self.spec)
-            if not self.spec.external and not from_local_sources:
-                message = 'Missing a source id for {s.name}@{s.version}'
-                tty.warn(message.format(s=self))
-            hash_content.append(''.encode('utf-8'))
-        else:
-            hash_content.append(source_id.encode('utf-8'))
+        This includes:
+            * source artifacts (tarballs, repositories) used to build;
+            * content hashes (``sha256``'s) of all patches applied by Spack; and
+            * canonicalized contents the ``package.py`` recipe used to build.
+
+        This hash is only included in Spack's DAG hash for concrete specs, but if it
+        happens to be called on a package with an abstract spec, only applicable (i.e.,
+        determinable) portions of the hash will be included.
+
+        """
+        # list of components to make up the hash
+        hash_content = []
+
+        # source artifacts/repositories
+        # TODO: resources
+        if self.spec.versions.concrete:
+            try:
+                source_id = fs.for_package_version(self, self.version).source_id()
+            except (fs.ExtrapolationError, fs.InvalidArgsError):
+                # ExtrapolationError happens if the package has no fetchers defined.
+                # InvalidArgsError happens when there are version directives with args,
+                #     but none of them identifies an actual fetcher.
+                source_id = None
+
+            if not source_id:
+                # TODO? in cases where a digest or source_id isn't available,
+                # should this attempt to download the source and set one? This
+                # probably only happens for source repositories which are
+                # referenced by branch name rather than tag or commit ID.
+                env = spack.environment.active_environment()
+                from_local_sources = env and env.is_develop(self.spec)
+                if not self.spec.external and not from_local_sources:
+                    message = 'Missing a source id for {s.name}@{s.version}'
+                    tty.warn(message.format(s=self))
+                hash_content.append(''.encode('utf-8'))
+            else:
+                hash_content.append(source_id.encode('utf-8'))
+
+        # patch sha256's
+        if self.spec.concrete:
+            hash_content.extend(
+                ':'.join((p.sha256, str(p.level))).encode('utf-8')
+                for p in self.spec.patches
+            )
 
-        hash_content.extend(':'.join((p.sha256, str(p.level))).encode('utf-8')
-                            for p in self.spec.patches)
+        # package.py contents
         hash_content.append(package_hash(self.spec, source=content).encode('utf-8'))
 
+        # put it all together and encode as base32
         b32_hash = base64.b32encode(
-            hashlib.sha256(bytes().join(
-                sorted(hash_content))).digest()).lower()
+            hashlib.sha256(
+                bytes().join(sorted(hash_content))
+            ).digest()
+        ).lower()
 
         # convert from bytes if running python 3
         if sys.version_info[0] >= 3:
diff --git a/lib/spack/spack/test/test_activations.py b/lib/spack/spack/test/test_activations.py
index f8b3e15faa..a832236c49 100644
--- a/lib/spack/spack/test/test_activations.py
+++ b/lib/spack/spack/test/test_activations.py
@@ -215,9 +215,9 @@ def test_python_ignore_namespace_init_conflict(
     python_spec = spack.spec.Spec('python@2.7.12')
     python_spec._concrete = True
 
-    ext1_pkg = create_python_ext_pkg('py-extension1@1.0.0', ext1_prefix, python_spec,
+    ext1_pkg = create_python_ext_pkg('py-extension1', ext1_prefix, python_spec,
                                      monkeypatch, py_namespace)
-    ext2_pkg = create_python_ext_pkg('py-extension2@1.0.0', ext2_prefix, python_spec,
+    ext2_pkg = create_python_ext_pkg('py-extension2', ext2_prefix, python_spec,
                                      monkeypatch, py_namespace)
 
     view_dir = str(tmpdir.join('view'))
@@ -250,9 +250,9 @@ def test_python_keep_namespace_init(
     python_spec = spack.spec.Spec('python@2.7.12')
     python_spec._concrete = True
 
-    ext1_pkg = create_python_ext_pkg('py-extension1@1.0.0', ext1_prefix, python_spec,
+    ext1_pkg = create_python_ext_pkg('py-extension1', ext1_prefix, python_spec,
                                      monkeypatch, py_namespace)
-    ext2_pkg = create_python_ext_pkg('py-extension2@1.0.0', ext2_prefix, python_spec,
+    ext2_pkg = create_python_ext_pkg('py-extension2', ext2_prefix, python_spec,
                                      monkeypatch, py_namespace)
 
     view_dir = str(tmpdir.join('view'))
@@ -293,9 +293,9 @@ def test_python_namespace_conflict(tmpdir, namespace_extensions,
     python_spec = spack.spec.Spec('python@2.7.12')
     python_spec._concrete = True
 
-    ext1_pkg = create_python_ext_pkg('py-extension1@1.0.0', ext1_prefix, python_spec,
+    ext1_pkg = create_python_ext_pkg('py-extension1', ext1_prefix, python_spec,
                                      monkeypatch, py_namespace)
-    ext2_pkg = create_python_ext_pkg('py-extension2@1.0.0', ext2_prefix, python_spec,
+    ext2_pkg = create_python_ext_pkg('py-extension2', ext2_prefix, python_spec,
                                      monkeypatch, other_namespace)
 
     view_dir = str(tmpdir.join('view'))
diff --git a/lib/spack/spack/test/util/package_hash.py b/lib/spack/spack/test/util/package_hash.py
index dd674b01f9..3133cd08a7 100644
--- a/lib/spack/spack/test/util/package_hash.py
+++ b/lib/spack/spack/test/util/package_hash.py
@@ -96,6 +96,26 @@ def test_content_hash_all_same_but_patch_contents(mock_packages, config):
     compare_hash_sans_name(False, spec1, spec2)
 
 
+def test_content_hash_not_concretized(mock_packages, config):
+    """Check that Package.content_hash() works on abstract specs."""
+    # these are different due to the package hash
+    spec1 = Spec("hash-test1@1.1")
+    spec2 = Spec("hash-test2@1.3")
+    compare_hash_sans_name(False, spec1, spec2)
+
+    # at v1.1 these are actually the same package when @when's are removed
+    # and the name isn't considered
+    spec1 = Spec("hash-test1@1.1")
+    spec2 = Spec("hash-test2@1.1")
+    compare_hash_sans_name(True, spec1, spec2)
+
+    # these end up being different b/c we can't eliminate much of the package.py
+    # without a version.
+    spec1 = Spec("hash-test1")
+    spec2 = Spec("hash-test2")
+    compare_hash_sans_name(False, spec1, spec2)
+
+
 def test_content_hash_different_variants(mock_packages, config):
     spec1 = Spec("hash-test1@1.2 +variantx").concretized()
     spec2 = Spec("hash-test2@1.2 ~variantx").concretized()
diff --git a/lib/spack/spack/test/verification.py b/lib/spack/spack/test/verification.py
index ea60dc1824..ad7373a439 100644
--- a/lib/spack/spack/test/verification.py
+++ b/lib/spack/spack/test/verification.py
@@ -143,7 +143,7 @@ def test_check_prefix_manifest(tmpdir):
     prefix_path = tmpdir.join('prefix')
     prefix = str(prefix_path)
 
-    spec = spack.spec.Spec('libelf@0.8.13')
+    spec = spack.spec.Spec('libelf')
     spec._mark_concrete()
     spec.prefix = prefix