diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/database.py | 68 | ||||
| -rw-r--r-- | lib/spack/spack/directory_layout.py | 15 | ||||
| -rw-r--r-- | lib/spack/spack/test/conftest.py | 4 | ||||
| -rw-r--r-- | lib/spack/spack/test/database.py | 49 |
4 files changed, 94 insertions, 42 deletions
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index b7294f3ecb..2b99e33bb0 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -937,22 +937,15 @@ class Database(object): tty.debug( 'RECONSTRUCTING FROM OLD DB: {0}'.format(entry.spec)) try: - layout = spack.store.layout - if entry.spec.external: - layout = None - install_check = True - else: - install_check = layout.check_installed(entry.spec) - - if install_check: - kwargs = { - 'spec': entry.spec, - 'directory_layout': layout, - 'explicit': entry.explicit, - 'installation_time': entry.installation_time # noqa: E501 - } - self._add(**kwargs) - processed_specs.add(entry.spec) + layout = None if entry.spec.external else spack.store.layout + kwargs = { + 'spec': entry.spec, + 'directory_layout': layout, + 'explicit': entry.explicit, + 'installation_time': entry.installation_time # noqa: E501 + } + self._add(**kwargs) + processed_specs.add(entry.spec) except Exception as e: # Something went wrong, so the spec was not restored # from old data @@ -1096,24 +1089,28 @@ class Database(object): } self._add(dep, directory_layout, **extra_args) - if key not in self._data: - installed = bool(spec.external) - path = None - if not spec.external and directory_layout: - path = directory_layout.path_for_spec(spec) - if path in self._installed_prefixes: - raise Exception("Install prefix collision.") - try: - directory_layout.check_installed(spec) - installed = True - except DirectoryLayoutError as e: - tty.warn( - 'Dependency missing: may be deprecated or corrupted:', - path, str(e)) + # Make sure the directory layout agrees whether the spec is installed + if not spec.external and directory_layout: + path = directory_layout.path_for_spec(spec) + installed = False + try: + directory_layout.ensure_installed(spec) + installed = True self._installed_prefixes.add(path) - elif spec.external_path: - path = spec.external_path + except DirectoryLayoutError as e: + msg = ("{0} is being {1} in the database with prefix {2}, " + "but this directory does not contain an installation of " + "the spec, due to: {3}") + action = "updated" if key in self._data else "registered" + tty.warn(msg.format(spec.short_spec, action, path, str(e))) + elif spec.external_path: + path = spec.external_path + installed = True + else: + path = None + installed = True + if key not in self._data: # Create a new install record with no deps initially. new_spec = spec.copy(deps=False) extra_args = { @@ -1141,9 +1138,8 @@ class Database(object): new_spec._full_hash = spec._full_hash else: - # If it is already there, mark it as installed and update - # installation time - self._data[key].installed = True + # It is already in the database + self._data[key].installed = installed self._data[key].installation_time = _now() self._data[key].explicit = explicit @@ -1210,7 +1206,7 @@ class Database(object): # This install prefix is now free for other specs to use, even if the # spec is only marked uninstalled. - if not rec.spec.external: + if not rec.spec.external and rec.installed: self._installed_prefixes.remove(rec.path) if rec.ref_count > 0: diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index 762b591827..8193b328b9 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -233,13 +233,20 @@ class DirectoryLayout(object): self.write_spec(spec, self.spec_file_path(spec)) - def check_installed(self, spec): + def ensure_installed(self, spec): + """ + Throws DirectoryLayoutError if: + 1. spec prefix does not exist + 2. spec prefix does not contain a spec file + 3. the spec file does not correspond to the spec + """ _check_concrete(spec) path = self.path_for_spec(spec) spec_file_path = self.spec_file_path(spec) if not os.path.isdir(path): - return None + raise InconsistentInstallDirectoryError( + "Install prefix {0} does not exist.".format(path)) if not os.path.isfile(spec_file_path): raise InconsistentInstallDirectoryError( @@ -248,7 +255,7 @@ class DirectoryLayout(object): installed_spec = self.read_spec(spec_file_path) if installed_spec == spec: - return path + return # DAG hashes currently do not include build dependencies. # @@ -261,7 +268,7 @@ class DirectoryLayout(object): # may be installed. This means for example that for two instances # that differ only in CMake version used to build, only one will # be installed. - return path + return if spec.dag_hash() == installed_spec.dag_hash(): raise SpecHashCollisionError(spec, installed_spec) diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 9a6dc0b13b..00ab62e020 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -876,8 +876,8 @@ class MockLayout(object): def path_for_spec(self, spec): return '/'.join([self.root, spec.name + '-' + spec.dag_hash()]) - def check_installed(self, spec): - return True + def ensure_installed(self, spec): + pass @pytest.fixture() diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 57a03a5db9..f8f4b5de82 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -11,6 +11,7 @@ import datetime import functools import json import os +import shutil import pytest @@ -909,3 +910,51 @@ def test_database_works_with_empty_dir(tmpdir): db.query() # Check that reading an empty directory didn't create a new index.json assert not os.path.exists(db._index_path) + + +def test_reindex_removed_prefix_is_not_installed(mutable_database, mock_store, capfd): + """When a prefix of a dependency is removed and the database is reindexed, + the spec should still be added through the dependent, but should be listed as + not installed.""" + + # Remove libelf from the filesystem + prefix = mutable_database.query_one('libelf').prefix + assert prefix.startswith(str(mock_store)) + shutil.rmtree(prefix) + + # Reindex should pick up libelf as a dependency of libdwarf + spack.store.store.reindex() + + # Reindexing should warn about libelf not being found on the filesystem + err = capfd.readouterr()[1] + assert 'this directory does not contain an installation of the spec' in err + + # And we should still have libelf in the database, but not installed. + assert not mutable_database.query_one('libelf', installed=True) + assert mutable_database.query_one('libelf', installed=False) + + +def test_reindex_when_all_prefixes_are_removed(mutable_database, mock_store): + # Remove all non-external installations from the filesystem + for spec in spack.store.db.query_local(): + if not spec.external: + assert spec.prefix.startswith(str(mock_store)) + shutil.rmtree(spec.prefix) + + # Make sure we have some explicitly installed specs + num = len(mutable_database.query_local(installed=True, explicit=True)) + assert num > 0 + + # Reindex uses the current index to repopulate itself + spack.store.store.reindex() + + # Make sure all explicit specs are still there, but are now uninstalled. + specs = mutable_database.query_local(installed=False, explicit=True) + assert len(specs) == num + + # And make sure they can be removed from the database (covers the case where + # `ref_count == 0 and not installed`, which hits some obscure branches. + for s in specs: + mutable_database.remove(s) + + assert len(mutable_database.query_local(installed=False, explicit=True)) == 0 |