Fix reading externals from old databases (#11118)

* Update Spec.prefix to have special case for 'None' in database path; regression test
* Update in database reader rather than spec
* Change assertion to conditional + raise
* Added test for concrete check in Spec.prefix

4 files changed, 36 insertions, 2 deletions

diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index b76ba5722c..ded79cbd15 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -112,7 +112,7 @@ class InstallRecord(object):
             installation_time=None
     ):
         self.spec = spec
-        self.path = str(path)
+        self.path = str(path) if path else None
         self.installed = bool(installed)
         self.ref_count = ref_count
         self.explicit = explicit
@@ -132,6 +132,10 @@ class InstallRecord(object):
     def from_dict(cls, spec, dictionary):
         d = dict(dictionary.items())
         d.pop('spec', None)
+
+        # Old databases may have "None" for path for externals
+        if d['path'] == 'None':
+            d['path'] = None
         return InstallRecord(spec, **d)
 
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index b202d34473..67e0990a19 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1267,6 +1267,9 @@ class Spec(object):
 
     @property
     def prefix(self):
+        if not self._concrete:
+            raise SpecError("Spec is not concrete: " + str(self))
+
         if self._prefix is None:
             upstream, record = spack.store.db.query_by_spec_hash(
                 self.dag_hash())
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index d6eb2cc618..fc2549b574 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -12,6 +12,7 @@ import functools
 import multiprocessing
 import os
 import pytest
+import json
 
 from llnl.util.tty.colify import colify
 
@@ -625,3 +626,23 @@ def test_regression_issue_8036(mutable_database, usr_folder_exists):
     # Now install the external package and check again the `installed` property
     s.package.do_install(fake=True)
     assert s.package.installed
+
+
+@pytest.mark.regression('11118')
+def test_old_external_entries_prefix(mutable_database):
+    with open(spack.store.db._index_path, 'r') as f:
+        db_obj = json.loads(f.read())
+
+    s = spack.spec.Spec('externaltool')
+    s.concretize()
+
+    db_obj['database']['installs'][s.dag_hash()]['path'] = 'None'
+
+    with open(spack.store.db._index_path, 'w') as f:
+        f.write(json.dumps(db_obj))
+
+    record = spack.store.db.get_record(s)
+
+    assert record.path is None
+    assert record.spec._prefix is None
+    assert record.spec.prefix == record.spec.external_path
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 4ad9c5f9c7..bec16fc7ad 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -6,7 +6,7 @@
 import sys
 import pytest
 
-from spack.spec import Spec, UnsatisfiableSpecError
+from spack.spec import Spec, UnsatisfiableSpecError, SpecError
 from spack.spec import substitute_abstract_variants, parse_anonymous_spec
 from spack.variant import InvalidVariantValueError
 from spack.variant import MultipleValuesInExclusiveVariantError
@@ -835,3 +835,9 @@ class TestSpecSematics(object):
         with pytest.raises(spack.directives.DirectiveError) as exc_info:
             fn(Pkg())
         assert "the default cannot be an empty string" in str(exc_info.value)
+
+    def test_abstract_spec_prefix_error(self):
+        spec = Spec('libelf')
+
+        with pytest.raises(SpecError):
+            spec.prefix