Bugfix: spack find -x in environments (#46798)

This addresses part [1] of #46345

#44713 introduced a bug where all non-spec query parameters like date
ranges, -x, etc. were ignored when an env was active.

This fixes that issue and adds tests for it.

---------

Co-authored-by: Harmen Stoppels <me@harmenstoppels.nl>

lib/spack/spack/cmd/__init__.py

@@ -545,6 +545,18 @@ def __init__(self, name):
         super().__init__("{0} is not a permissible Spack command name.".format(name))
 
 
+class MultipleSpecsMatch(Exception):
+    """Raised when multiple specs match a constraint, in a context where
+    this is not allowed.
+    """
+
+
+class NoSpecMatches(Exception):
+    """Raised when no spec matches a constraint, in a context where
+    this is not allowed.
+    """
+
+
 ########################################
 # argparse types for argument validation
 ########################################

lib/spack/spack/cmd/find.py

@@ -222,11 +222,9 @@ def decorator(spec, fmt):
 def display_env(env, args, decorator, results):
     """Display extra find output when running in an environment.
 
-    Find in an environment outputs 2 or 3 sections:
+    In an environment, `spack find` outputs a preliminary section
-
+    showing the root specs of the environment (this is in addition
-    1. Root specs
+    to the section listing out specs matching the query parameters).
-    2. Concretized roots (if asked for with -c)
-    3. Installed specs
 
     """
     tty.msg("In environment %s" % env.name)
@@ -299,6 +297,56 @@ def root_decorator(spec, string):
         print()
 
 
+def _find_query(args, env):
+    q_args = query_arguments(args)
+    concretized_but_not_installed = list()
+    if env:
+        all_env_specs = env.all_specs()
+        if args.constraint:
+            init_specs = cmd.parse_specs(args.constraint)
+            env_specs = env.all_matching_specs(*init_specs)
+        else:
+            env_specs = all_env_specs
+
+        spec_hashes = set(x.dag_hash() for x in env_specs)
+        specs_meeting_q_args = set(spack.store.STORE.db.query(hashes=spec_hashes, **q_args))
+
+        results = list()
+        with spack.store.STORE.db.read_transaction():
+            for spec in env_specs:
+                if not spec.installed:
+                    concretized_but_not_installed.append(spec)
+                if spec in specs_meeting_q_args:
+                    results.append(spec)
+    else:
+        results = args.specs(**q_args)
+
+    # use groups by default except with format.
+    if args.groups is None:
+        args.groups = not args.format
+
+    # Exit early with an error code if no package matches the constraint
+    if concretized_but_not_installed and args.show_concretized:
+        pass
+    elif results:
+        pass
+    elif args.constraint:
+        raise cmd.NoSpecMatches()
+
+    # If tags have been specified on the command line, filter by tags
+    if args.tags:
+        packages_with_tags = spack.repo.PATH.packages_with_tags(*args.tags)
+        results = [x for x in results if x.name in packages_with_tags]
+        concretized_but_not_installed = [
+            x for x in concretized_but_not_installed if x.name in packages_with_tags
+        ]
+
+    if args.loaded:
+        results = cmd.filter_loaded_specs(results)
+
+    return results, concretized_but_not_installed
+
+
 def find(parser, args):
     env = ev.active_environment()
 
@@ -307,34 +355,12 @@ def find(parser, args):
     if not env and args.show_concretized:
         tty.die("-c / --show-concretized requires an active environment")
 
-    if env:
+    try:
-        if args.constraint:
+        results, concretized_but_not_installed = _find_query(args, env)
-            init_specs = spack.cmd.parse_specs(args.constraint)
+    except cmd.NoSpecMatches:
-            results = env.all_matching_specs(*init_specs)
+        # Note: this uses args.constraint vs. args.constraint_specs because
-        else:
+        # the latter only exists if you call args.specs()
-            results = env.all_specs()
+        tty.die(f"No package matches the query: {' '.join(args.constraint)}")
-    else:
-        q_args = query_arguments(args)
-        results = args.specs(**q_args)
-
-    decorator = make_env_decorator(env) if env else lambda s, f: f
-
-    # use groups by default except with format.
-    if args.groups is None:
-        args.groups = not args.format
-
-    # Exit early with an error code if no package matches the constraint
-    if not results and args.constraint:
-        constraint_str = " ".join(str(s) for s in args.constraint_specs)
-        tty.die(f"No package matches the query: {constraint_str}")
-
-    # If tags have been specified on the command line, filter by tags
-    if args.tags:
-        packages_with_tags = spack.repo.PATH.packages_with_tags(*args.tags)
-        results = [x for x in results if x.name in packages_with_tags]
-
-    if args.loaded:
-        results = spack.cmd.filter_loaded_specs(results)
 
     if args.install_status or args.show_concretized:
         status_fn = spack.spec.Spec.install_status
@@ -345,14 +371,16 @@ def find(parser, args):
     if args.json:
         cmd.display_specs_as_json(results, deps=args.deps)
     else:
+        decorator = make_env_decorator(env) if env else lambda s, f: f
+
         if not args.format:
             if env:
                 display_env(env, args, decorator, results)
 
         if not args.only_roots:
-            display_results = results
+            display_results = list(results)
-            if not args.show_concretized:
+            if args.show_concretized:
-                display_results = list(x for x in results if x.installed)
+                display_results += concretized_but_not_installed
             cmd.display_specs(
                 display_results, args, decorator=decorator, all_headers=True, status_fn=status_fn
             )
@@ -370,13 +398,9 @@ def find(parser, args):
                     concretized_suffix += " (show with `spack find -c`)"
 
             pkg_type = "loaded" if args.loaded else "installed"
-            spack.cmd.print_how_many_pkgs(
+            cmd.print_how_many_pkgs(results, pkg_type, suffix=installed_suffix)
-                list(x for x in results if x.installed), pkg_type, suffix=installed_suffix
-            )
 
             if env:
-                spack.cmd.print_how_many_pkgs(
+                cmd.print_how_many_pkgs(
-                    list(x for x in results if not x.installed),
+                    concretized_but_not_installed, "concretized", suffix=concretized_suffix
-                    "concretized",
-                    suffix=concretized_suffix,
                 )

lib/spack/spack/cmd/modules/__init__.py

@@ -19,6 +19,7 @@
 import spack.modules
 import spack.modules.common
 import spack.repo
+from spack.cmd import MultipleSpecsMatch, NoSpecMatches
 from spack.cmd.common import arguments
 
 description = "manipulate module files"
@@ -91,18 +92,6 @@ def add_loads_arguments(subparser):
     arguments.add_common_arguments(subparser, ["recurse_dependencies"])
 
 
-class MultipleSpecsMatch(Exception):
-    """Raised when multiple specs match a constraint, in a context where
-    this is not allowed.
-    """
-
-
-class NoSpecMatches(Exception):
-    """Raised when no spec matches a constraint, in a context where
-    this is not allowed.
-    """
-
-
 def one_spec_or_raise(specs):
     """Ensures exactly one spec has been selected, or raises the appropriate
     exception.

lib/spack/spack/test/cmd/find.py

@@ -14,10 +14,13 @@
 import spack.cmd as cmd
 import spack.cmd.find
 import spack.environment as ev
+import spack.repo
 import spack.store
 import spack.user_environment as uenv
 from spack.main import SpackCommand
 from spack.spec import Spec
+from spack.test.conftest import create_test_repo
+from spack.test.utilities import SpackCommandArgs
 from spack.util.pattern import Bunch
 
 find = SpackCommand("find")
@@ -453,3 +456,140 @@ def test_environment_with_version_range_in_compiler_doesnt_fail(tmp_path):
     with test_environment:
         output = find()
     assert "zlib%gcc@12.1.0" in output
+
+
+_pkga = (
+    "a0",
+    """\
+class A0(Package):
+    version("1.2")
+    version("1.1")
+
+    depends_on("b0")
+    depends_on("c0")
+""",
+)
+
+
+_pkgb = (
+    "b0",
+    """\
+class B0(Package):
+    version("1.2")
+    version("1.1")
+""",
+)
+
+
+_pkgc = (
+    "c0",
+    """\
+class C0(Package):
+    version("1.2")
+    version("1.1")
+
+    tags = ["tag0", "tag1"]
+""",
+)
+
+
+_pkgd = (
+    "d0",
+    """\
+class D0(Package):
+    version("1.2")
+    version("1.1")
+
+    depends_on("c0")
+    depends_on("e0")
+""",
+)
+
+
+_pkge = (
+    "e0",
+    """\
+class E0(Package):
+    tags = ["tag1", "tag2"]
+
+    version("1.2")
+    version("1.1")
+""",
+)
+
+
+@pytest.fixture
+def _create_test_repo(tmpdir, mutable_config):
+    r"""
+      a0  d0
+     / \ / \
+    b0  c0  e0
+    """
+    yield create_test_repo(tmpdir, [_pkga, _pkgb, _pkgc, _pkgd, _pkge])
+
+
+@pytest.fixture
+def test_repo(_create_test_repo, monkeypatch, mock_stage):
+    with spack.repo.use_repositories(_create_test_repo) as mock_repo_path:
+        yield mock_repo_path
+
+
+def test_find_concretized_not_installed(
+    mutable_mock_env_path, install_mockery, mock_fetch, test_repo, mock_archive
+):
+    """Test queries against installs of specs against fake repo.
+
+    Given A, B, C, D, E, create an environment and install A.
+    Add and concretize (but do not install) D.
+    Test a few queries after force uninstalling a dependency of A (but not
+    A itself).
+    """
+    add = SpackCommand("add")
+    concretize = SpackCommand("concretize")
+    uninstall = SpackCommand("uninstall")
+
+    def _query(_e, *args):
+        return spack.cmd.find._find_query(SpackCommandArgs("find")(*args), _e)
+
+    def _nresults(_qresult):
+        return len(_qresult[0]), len(_qresult[1])
+
+    env("create", "test")
+    with ev.read("test") as e:
+        install("--fake", "--add", "a0")
+
+        assert _nresults(_query(e)) == (3, 0)
+        assert _nresults(_query(e, "--explicit")) == (1, 0)
+
+        add("d0")
+        concretize("--reuse")
+
+        # At this point d0 should use existing c0, but d/e
+        # are not installed in the env
+
+        # --explicit, --deprecated, --start-date, etc. are all
+        # filters on records, and therefore don't apply to
+        # concretized-but-not-installed results
+        assert _nresults(_query(e, "--explicit")) == (1, 2)
+
+        assert _nresults(_query(e)) == (3, 2)
+        assert _nresults(_query(e, "-c", "d0")) == (0, 1)
+
+        uninstall("-f", "-y", "b0")
+
+        # b0 is now missing (it is not installed, but has an
+        # installed parent)
+
+        assert _nresults(_query(e)) == (2, 3)
+        # b0 is "double-counted" here: it meets the --missing
+        # criteria, and also now qualifies as a
+        # concretized-but-not-installed spec
+        assert _nresults(_query(e, "--missing")) == (3, 3)
+        assert _nresults(_query(e, "--only-missing")) == (1, 3)
+
+        # Tags are not attached to install records, so they
+        # can modify the concretized-but-not-installed results
+
+        assert _nresults(_query(e, "--tag=tag0")) == (1, 0)
+        assert _nresults(_query(e, "--tag=tag1")) == (1, 1)
+        assert _nresults(_query(e, "--tag=tag2")) == (0, 1)

lib/spack/spack/test/utilities.py

@@ -0,0 +1,32 @@
+# Copyright 2013-2024 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+"""Non-fixture utilities for test code. Must be imported.
+"""
+from spack.main import make_argument_parser
+
+
+class SpackCommandArgs:
+    """Use this to get an Args object like what is passed into
+    a command.
+
+    Useful for emulating args in unit tests that want to check
+    helper functions in Spack commands. Ensures that you get all
+    the default arg values established by the parser.
+
+    Example usage::
+
+        install_args = SpackCommandArgs("install")("-v", "mpich")
+    """
+
+    def __init__(self, command_name):
+        self.parser = make_argument_parser()
+        self.command_name = command_name
+
+    def __call__(self, *argv, **kwargs):
+        self.parser.add_command(self.command_name)
+        prepend = kwargs["global_args"] if "global_args" in kwargs else []
+        args, unknown = self.parser.parse_known_args(prepend + [self.command_name] + list(argv))
+        return args