summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMassimiliano Culpo <massimiliano.culpo@gmail.com>2018-07-17 20:43:30 +0200
committerscheibelp <scheibel1@llnl.gov>2018-07-17 11:43:30 -0700
commit373b3d2444b2a2058e904071042e08d2dfecfc5b (patch)
treebbd736b27e3238095ea1801b3b12f991d06cc1d8
parent8ce62ba51334f0f9e4b62f795923d81514229013 (diff)
downloadspack-373b3d2444b2a2058e904071042e08d2dfecfc5b.tar.gz
spack-373b3d2444b2a2058e904071042e08d2dfecfc5b.tar.bz2
spack-373b3d2444b2a2058e904071042e08d2dfecfc5b.tar.xz
spack-373b3d2444b2a2058e904071042e08d2dfecfc5b.zip
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.
-rw-r--r--lib/spack/spack/package.py35
-rw-r--r--lib/spack/spack/test/data/packages.yaml1
-rw-r--r--lib/spack/spack/test/database.py33
-rw-r--r--var/spack/repos/builtin.mock/packages/externaltool/package.py1
4 files changed, 64 insertions, 6 deletions
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index f0aa67fc8f..c79237dbef 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -995,7 +995,23 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
@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 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
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" %
diff --git a/lib/spack/spack/test/data/packages.yaml b/lib/spack/spack/test/data/packages.yaml
index 923d63173a..c7256ddb33 100644
--- a/lib/spack/spack/test/data/packages.yaml
+++ b/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:
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index 2e61853349..4c5db3089b 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -27,6 +27,7 @@ These tests check the database is functioning properly,
both in memory and in its file
"""
import datetime
+import functools
import multiprocessing
import os
import pytest
@@ -42,6 +43,23 @@ from spack.util.executable import Executable
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
diff --git a/var/spack/repos/builtin.mock/packages/externaltool/package.py b/var/spack/repos/builtin.mock/packages/externaltool/package.py
index c8867f3ef5..9f0e3f6f73 100644
--- a/var/spack/repos/builtin.mock/packages/externaltool/package.py
+++ b/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')