Packages must be added to DB to be considered installed (#8038)

Fixes #8036

Before this PR Package.installed was returning True if the spec prefix
existed, without checking the DB. This is wrong for external packages,
whose prefix exists before being registered into the DB. Now the property
checks for both the prefix and a DB entry.

lib/spack/spack/package.py

@@ -995,7 +995,23 @@ def provides(self, vpkg_name):
 
     @property
     def installed(self):
-        return os.path.isdir(self.prefix)
+        """Installation status of a package.
+
+        Returns:
+            True if the package has been installed, False otherwise.
+        """
+        has_prefix = os.path.isdir(self.prefix)
+        try:
+            # If the spec is in the DB, check the installed
+            # attribute of the record
+            rec = spack.store.db.get_record(self.spec)
+            db_says_installed = rec.installed
+        except KeyError:
+            # If the spec is not in the DB, the method
+            #  above raises a Key error
+            db_says_installed = False
+
+        return has_prefix and db_says_installed
 
     @property
     def prefix(self):
@@ -1650,11 +1666,18 @@ def unit_test_check(self):
     def check_for_unfinished_installation(
             self, keep_prefix=False, restage=False):
         """Check for leftover files from partially-completed prior install to
-           prepare for a new install attempt. Options control whether these
-           files are reused (vs. destroyed). This function considers a package
-           fully-installed if there is a DB entry for it (in that way, it is
-           more strict than Package.installed). The return value is used to
-           indicate when the prefix exists but the install is not complete.
+        prepare for a new install attempt.
+
+        Options control whether these files are reused (vs. destroyed).
+
+        Args:
+            keep_prefix (bool): True if the installation prefix needs to be
+                kept, False otherwise
+            restage (bool): False if the stage has to be kept, True otherwise
+
+        Returns:
+            True if the prefix exists but the install is not complete, False
+            otherwise.
         """
         if self.spec.external:
             raise ExternalPackageError("Attempted to repair external spec %s" %

lib/spack/spack/test/data/packages.yaml

@@ -3,6 +3,7 @@ packages:
     buildable: False
     paths:
       externaltool@1.0%gcc@4.5.0: /path/to/external_tool
+      externaltool@0.9%gcc@4.5.0: /usr
   externalvirtual:
     buildable: False
     paths:

lib/spack/spack/test/database.py

@@ -27,6 +27,7 @@
 both in memory and in its file
 """
 import datetime
+import functools
 import multiprocessing
 import os
 import pytest
@@ -42,6 +43,23 @@
 pytestmark = pytest.mark.db
 
 
+@pytest.fixture()
+def usr_folder_exists(monkeypatch):
+    """The ``/usr`` folder is assumed to be existing in some tests. This
+    fixture makes it such that its existence is mocked, so we have no
+    requirements on the system running tests.
+    """
+    isdir = os.path.isdir
+
+    @functools.wraps(os.path.isdir)
+    def mock_isdir(path):
+        if path == '/usr':
+            return True
+        return isdir(path)
+
+    monkeypatch.setattr(os.path, 'isdir', mock_isdir)
+
+
 def _print_ref_counts():
     """Print out all ref counts for the graph used here, for debugging"""
     recs = []
@@ -436,3 +454,18 @@ def test_external_entries_in_db(database):
     assert rec.spec.external_path == '/path/to/external_tool'
     assert rec.spec.external_module is None
     assert rec.explicit is True
+
+
+@pytest.mark.regression('8036')
+def test_regression_issue_8036(mutable_database, usr_folder_exists):
+    # The test ensures that the external package prefix is treated as
+    # existing. Even when the package prefix exists, the package should
+    # not be considered installed until it is added to the database with
+    # do_install.
+    s = spack.spec.Spec('externaltool@0.9')
+    s.concretize()
+    assert not s.package.installed
+
+    # Now install the external package and check again the `installed` property
+    s.package.do_install(fake=True)
+    assert s.package.installed

var/spack/repos/builtin.mock/packages/externaltool/package.py

@@ -30,6 +30,7 @@ class Externaltool(Package):
     url      = "http://somewhere.com/tool-1.0.tar.gz"
 
     version('1.0', '1234567890abcdef1234567890abcdef')
+    version('0.9', '1234567890abcdef1234567890abcdef')
 
     depends_on('externalprereq')