From 6b619daef36b2d780bb6b3e32ec3d89a4f011faf Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 15 Jan 2019 11:23:08 -0600 Subject: specs: better lookup by hash; allow references to missing dependency hashes - previously spec parsing didn't allow you to look up missing (but still known) specs by hash - This allows you to reference and potentially reinstall force-uninstalled dependencies - add testing for force uninstall and for reference by spec - cmd/install tests now use mutable_database --- lib/spack/spack/database.py | 67 +++++++++++++++++++++++++ lib/spack/spack/spec.py | 10 ++-- lib/spack/spack/test/cmd/uninstall.py | 94 +++++++++++++++++++++++++++++++---- 3 files changed, 155 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index ad6475b026..4d532d1f00 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -937,6 +937,73 @@ class Database(object): continue # TODO: conditional way to do this instead of catching exceptions + def get_by_hash_local(self, dag_hash, default=None, installed=any): + """Look up a spec in *this DB* by DAG hash, or by a DAG hash prefix. + + Arguments: + dag_hash (str): hash (or hash prefix) to look up + default (object, optional): default value to return if dag_hash is + not in the DB (default: None) + installed (bool or any, optional): if ``True``, includes only + installed specs in the search; if ``False`` only missing specs, + and if ``any``, either installed or missing (default: any) + + ``installed`` defaults to ``any`` so that we can refer to any + known hash. Note that ``query()`` and ``query_one()`` differ in + that they only return installed specs by default. + + Returns: + (list): a list of specs matching the hash or hash prefix + + """ + with self.read_transaction(): + # hash is a full hash and is in the data somewhere + if dag_hash in self._data: + rec = self._data[dag_hash] + if installed is any or rec.installed == installed: + return [rec.spec] + else: + return default + + # check if hash is a prefix of some installed (or previously + # installed) spec. + matches = [record.spec for h, record in self._data.items() + if h.startswith(dag_hash) and + (installed is any or installed == record.installed)] + if matches: + return matches + + # nothing found + return default + + def get_by_hash(self, dag_hash, default=None, installed=any): + """Look up a spec by DAG hash, or by a DAG hash prefix. + + Arguments: + dag_hash (str): hash (or hash prefix) to look up + default (object, optional): default value to return if dag_hash is + not in the DB (default: None) + installed (bool or any, optional): if ``True``, includes only + installed specs in the search; if ``False`` only missing specs, + and if ``any``, either installed or missing (default: any) + + ``installed`` defaults to ``any`` so that we can refer to any + known hash. Note that ``query()`` and ``query_one()`` differ in + that they only return installed specs by default. + + Returns: + (list): a list of specs matching the hash or hash prefix + + """ + search_path = [self] + self.upstream_dbs + for db in search_path: + spec = db.get_by_hash_local( + dag_hash, default=default, installed=installed) + if spec is not None: + return spec + + return default + def _query( self, query_spec=any, diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 3586cb2aae..c7926ad3a3 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3951,17 +3951,15 @@ class SpecParser(spack.parse.Parser): def spec_by_hash(self): self.expect(ID) - specs = spack.store.db.query() - matches = [spec for spec in specs if - spec.dag_hash()[:len(self.token.value)] == self.token.value] - + dag_hash = self.token.value + matches = spack.store.db.get_by_hash(dag_hash) if not matches: - raise NoSuchHashError(self.token.value) + raise NoSuchHashError(dag_hash) if len(matches) != 1: raise AmbiguousHashError( "Multiple packages specify hash beginning '%s'." - % self.token.value, *matches) + % dag_hash, *matches) return matches[0] diff --git a/lib/spack/spack/test/cmd/uninstall.py b/lib/spack/spack/test/cmd/uninstall.py index f331b1a548..c9a23c48cf 100644 --- a/lib/spack/spack/test/cmd/uninstall.py +++ b/lib/spack/spack/test/cmd/uninstall.py @@ -8,6 +8,7 @@ import spack.store from spack.main import SpackCommand, SpackCommandError uninstall = SpackCommand('uninstall') +install = SpackCommand('install') class MockArgs(object): @@ -21,24 +22,21 @@ class MockArgs(object): @pytest.mark.db -@pytest.mark.usefixtures('database') -def test_multiple_matches(): +def test_multiple_matches(mutable_database): """Test unable to uninstall when multiple matches.""" with pytest.raises(SpackCommandError): uninstall('-y', 'mpileaks') @pytest.mark.db -@pytest.mark.usefixtures('database') -def test_installed_dependents(): +def test_installed_dependents(mutable_database): """Test can't uninstall when ther are installed dependents.""" with pytest.raises(SpackCommandError): uninstall('-y', 'libelf') @pytest.mark.db -@pytest.mark.usefixtures('mutable_database') -def test_recursive_uninstall(): +def test_recursive_uninstall(mutable_database): """Test recursive uninstall.""" uninstall('-y', '-a', '--dependents', 'callpath') @@ -56,12 +54,11 @@ def test_recursive_uninstall(): @pytest.mark.db @pytest.mark.regression('3690') -@pytest.mark.usefixtures('mutable_database') @pytest.mark.parametrize('constraint,expected_number_of_specs', [ ('dyninst', 7), ('libelf', 5) ]) def test_uninstall_spec_with_multiple_roots( - constraint, expected_number_of_specs + constraint, expected_number_of_specs, mutable_database ): uninstall('-y', '-a', '--dependents', constraint) @@ -70,14 +67,91 @@ def test_uninstall_spec_with_multiple_roots( @pytest.mark.db -@pytest.mark.usefixtures('mutable_database') @pytest.mark.parametrize('constraint,expected_number_of_specs', [ ('dyninst', 13), ('libelf', 13) ]) def test_force_uninstall_spec_with_ref_count_not_zero( - constraint, expected_number_of_specs + constraint, expected_number_of_specs, mutable_database ): uninstall('-f', '-y', constraint) all_specs = spack.store.layout.all_specs() assert len(all_specs) == expected_number_of_specs + + +@pytest.mark.db +def test_force_uninstall_and_reinstall_by_hash(mutable_database): + """Test forced uninstall and reinstall of old specs.""" + # this is the spec to be removed + callpath_spec = spack.store.db.query_one('callpath ^mpich') + dag_hash = callpath_spec.dag_hash() + + # ensure can look up by hash and that it's a dependent of mpileaks + def validate_callpath_spec(installed): + assert installed is True or installed is False + + specs = spack.store.db.get_by_hash(dag_hash, installed=installed) + assert len(specs) == 1 and specs[0] == callpath_spec + + specs = spack.store.db.get_by_hash(dag_hash[:7], installed=installed) + assert len(specs) == 1 and specs[0] == callpath_spec + + specs = spack.store.db.get_by_hash(dag_hash, installed=any) + assert len(specs) == 1 and specs[0] == callpath_spec + + specs = spack.store.db.get_by_hash(dag_hash[:7], installed=any) + assert len(specs) == 1 and specs[0] == callpath_spec + + specs = spack.store.db.get_by_hash(dag_hash, installed=not installed) + assert specs is None + + specs = spack.store.db.get_by_hash(dag_hash[:7], + installed=not installed) + assert specs is None + + mpileaks_spec = spack.store.db.query_one('mpileaks ^mpich') + assert callpath_spec in mpileaks_spec + + spec = spack.store.db.query_one('callpath ^mpich', installed=installed) + assert spec == callpath_spec + + spec = spack.store.db.query_one('callpath ^mpich', installed=any) + assert spec == callpath_spec + + spec = spack.store.db.query_one('callpath ^mpich', + installed=not installed) + assert spec is None + + validate_callpath_spec(True) + + uninstall('-y', '-f', 'callpath ^mpich') + + # ensure that you can still look up by hash and see deps, EVEN though + # the callpath spec is missing. + validate_callpath_spec(False) + + # BUT, make sure that the removed callpath spec is not in queries + def db_specs(): + all_specs = spack.store.layout.all_specs() + return ( + all_specs, + [s for s in all_specs if s.satisfies('mpileaks')], + [s for s in all_specs if s.satisfies('callpath')], + [s for s in all_specs if s.satisfies('mpi')] + ) + all_specs, mpileaks_specs, callpath_specs, mpi_specs = db_specs() + assert len(all_specs) == 13 + assert len(mpileaks_specs) == 3 + assert len(callpath_specs) == 2 + assert len(mpi_specs) == 3 + + # Now, REINSTALL the spec and make sure everything still holds + install('--fake', '/%s' % dag_hash[:7]) + + validate_callpath_spec(True) + + all_specs, mpileaks_specs, callpath_specs, mpi_specs = db_specs() + assert len(all_specs) == 14 # back to 14 + assert len(mpileaks_specs) == 3 + assert len(callpath_specs) == 3 # back to 3 + assert len(mpi_specs) == 3 -- cgit v1.2.3-60-g2f50