diff options
author | Massimiliano Culpo <massimiliano.culpo@gmail.com> | 2024-10-21 18:03:57 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-21 18:03:57 +0200 |
commit | 4187c57250fbfd50aaa5bb8d08b11daf42cbb83b (patch) | |
tree | a773a82003e1b1a0af396e143cd5365003cefa9f /lib | |
parent | 590be9bba11aff7004cce8c9aed56452ac1a481e (diff) | |
download | spack-4187c57250fbfd50aaa5bb8d08b11daf42cbb83b.tar.gz spack-4187c57250fbfd50aaa5bb8d08b11daf42cbb83b.tar.bz2 spack-4187c57250fbfd50aaa5bb8d08b11daf42cbb83b.tar.xz spack-4187c57250fbfd50aaa5bb8d08b11daf42cbb83b.zip |
Fix broken `spack find -u` (#47102)
fixes #47101
The bug was introduced in #33495, where `spack find was not updated,
and wasn't caught by unit tests.
Now a Database can accept a custom predicate to select the installation
records. A unit test is added to prevent regressions. The weird convention
of having `any` as a default value has been replaced by the more commonly
used `None`.
Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/cmd/find.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/cmd/modules/__init__.py | 5 | ||||
-rw-r--r-- | lib/spack/spack/database.py | 22 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/find.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/test/database.py | 17 |
5 files changed, 37 insertions, 17 deletions
diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py index 2f25683c5e..afa830fb8e 100644 --- a/lib/spack/spack/cmd/find.py +++ b/lib/spack/spack/cmd/find.py @@ -174,9 +174,9 @@ def query_arguments(args): if (args.missing or args.only_missing) and not args.only_deprecated: installed.append(InstallStatuses.MISSING) - known = any + predicate_fn = None if args.unknown: - known = False + predicate_fn = lambda x: not spack.repo.PATH.exists(x.spec.name) explicit = any if args.explicit: @@ -184,7 +184,7 @@ def query_arguments(args): if args.implicit: explicit = False - q_args = {"installed": installed, "known": known, "explicit": explicit} + q_args = {"installed": installed, "predicate_fn": predicate_fn, "explicit": explicit} install_tree = args.install_tree upstreams = spack.config.get("upstreams", {}) diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index b11063714f..754813addc 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -378,7 +378,10 @@ callbacks = {"refresh": refresh, "rm": rm, "find": find, "loads": loads} def modules_cmd(parser, args, module_type, callbacks=callbacks): # Qualifiers to be used when querying the db for specs constraint_qualifiers = { - "refresh": {"installed": True, "known": lambda x: not spack.repo.PATH.exists(x)} + "refresh": { + "installed": True, + "predicate_fn": lambda x: spack.repo.PATH.exists(x.spec.name), + } } query_args = constraint_qualifiers.get(args.subparser_name, {}) diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index 907b73c5db..c1ed06d241 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -299,12 +299,9 @@ _QUERY_DOCSTRING = """ database. If it is a spec, we'll evaluate ``spec.satisfies(query_spec)`` - known (bool or None): Specs that are "known" are those - for which Spack can locate a ``package.py`` file -- i.e., - Spack "knows" how to install them. Specs that are unknown may - represent packages that existed in a previous version of - Spack, but have since either changed their name or - been removed + predicate_fn: optional predicate taking an InstallRecord as argument, and returning + whether that record is selected for the query. It can be used to craft criteria + that need some data for selection not provided by the Database itself. installed (bool or InstallStatus or typing.Iterable or None): if ``True``, includes only installed @@ -604,6 +601,9 @@ class FailureTracker: return self.dir / f"{spec.name}-{spec.dag_hash()}" +SelectType = Callable[[InstallRecord], bool] + + class Database: #: Fields written for each install record record_fields: Tuple[str, ...] = DEFAULT_INSTALL_RECORD_FIELDS @@ -1526,7 +1526,7 @@ class Database: def _query( self, query_spec=any, - known=any, + predicate_fn: Optional[SelectType] = None, installed=True, explicit=any, start_date=None, @@ -1534,7 +1534,7 @@ class Database: hashes=None, in_buildcache=any, origin=None, - ): + ) -> List["spack.spec.Spec"]: """Run a query on the database.""" # TODO: Specs are a lot like queries. Should there be a @@ -1580,7 +1580,7 @@ class Database: if explicit is not any and rec.explicit != explicit: continue - if known is not any and known(rec.spec.name): + if predicate_fn is not None and not predicate_fn(rec): continue if start_date or end_date: @@ -1665,14 +1665,14 @@ class Database: query.__doc__ = "" query.__doc__ += _QUERY_DOCSTRING - def query_one(self, query_spec, known=any, installed=True): + def query_one(self, query_spec, predicate_fn=None, installed=True): """Query for exactly one spec that matches the query spec. Raises an assertion error if more than one spec matches the query. Returns None if no installed package matches. """ - concrete_specs = self.query(query_spec, known=known, installed=installed) + concrete_specs = self.query(query_spec, predicate_fn=predicate_fn, installed=installed) assert len(concrete_specs) <= 1 return concrete_specs[0] if concrete_specs else None diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py index fa8299f2ab..a29b56708f 100644 --- a/lib/spack/spack/test/cmd/find.py +++ b/lib/spack/spack/test/cmd/find.py @@ -70,10 +70,10 @@ def test_query_arguments(): q_args = query_arguments(args) assert "installed" in q_args - assert "known" in q_args + assert "predicate_fn" in q_args assert "explicit" in q_args assert q_args["installed"] == ["installed"] - assert q_args["known"] is any + assert q_args["predicate_fn"] is None assert q_args["explicit"] is any assert "start_date" in q_args assert "end_date" not in q_args diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 7140a50287..3049e54398 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -1181,3 +1181,20 @@ def test_reindex_with_upstreams(tmp_path, monkeypatch, mock_packages, config): assert not reindexed_local_store.db.query_local("callpath") assert reindexed_local_store.db.query("callpath") == [callpath] assert reindexed_local_store.db.query_local("mpileaks") == [mpileaks] + + +@pytest.mark.regression("47101") +def test_query_with_predicate_fn(database): + all_specs = database.query() + + # Name starts with a string + specs = database.query(predicate_fn=lambda x: x.spec.name.startswith("mpil")) + assert specs and all(x.name.startswith("mpil") for x in specs) + assert len(specs) < len(all_specs) + + # Recipe is currently known/unknown + specs = database.query(predicate_fn=lambda x: spack.repo.PATH.exists(x.spec.name)) + assert specs == all_specs + + specs = database.query(predicate_fn=lambda x: not spack.repo.PATH.exists(x.spec.name)) + assert not specs |