Spec.__getitem__: restrict to direct deps + transitive runtime deps (#49016)

With this change spec["pkg"] searches only direct dependencies and transitive link/run
dependencies, ordered by depth. This avoids situations where we pick up unwanted 
deps of build/test deps. 

To reach those, you need to do spec["build_dep"]["pkg"] explicitly.

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

lib/spack/spack/spec.py

@@ -3751,18 +3751,16 @@ def __getitem__(self, name: str):
             csv = query_parameters.pop().strip()
             query_parameters = re.split(r"\s*,\s*", csv)
 
-        order = lambda: itertools.chain(
+        # Consider all direct dependencies and transitive runtime dependencies
-            self.traverse_edges(deptype=dt.LINK, order="breadth", cover="edges"),
+        order = itertools.chain(
-            self.edges_to_dependencies(depflag=dt.BUILD | dt.RUN | dt.TEST),
+            self.edges_to_dependencies(depflag=dt.ALL),
-            self.traverse_edges(deptype=dt.ALL, order="breadth", cover="edges"),
+            self.traverse_edges(deptype=dt.LINK | dt.RUN, order="breadth", cover="edges"),
         )
 
-        # Consider runtime dependencies and direct build/test deps before transitive dependencies,
-        # and prefer matches closest to the root.
         try:
-            edge = next((e for e in order() if e.spec.name == name or name in e.virtuals))
+            edge = next((e for e in order if e.spec.name == name or name in e.virtuals))
-        except StopIteration:
+        except StopIteration as e:
-            raise KeyError(f"No spec with name {name} in {self}")
+            raise KeyError(f"No spec with name {name} in {self}") from e
 
         if self._concrete:
             return SpecBuildInterface(

lib/spack/spack/test/spec_dag.py

@@ -585,8 +585,9 @@ def test_construct_spec_with_deptypes(self):
         assert s["b"].edges_to_dependencies(name="c")[0].depflag == dt.BUILD
         assert s["d"].edges_to_dependencies(name="e")[0].depflag == dt.BUILD | dt.LINK
         assert s["e"].edges_to_dependencies(name="f")[0].depflag == dt.RUN
-
+        # The subscript follows link/run transitive deps or direct build/test deps, therefore
-        assert s["c"].edges_from_dependents(name="b")[0].depflag == dt.BUILD
+        # we need an extra step to get to "c"
+        assert s["b"]["c"].edges_from_dependents(name="b")[0].depflag == dt.BUILD
         assert s["e"].edges_from_dependents(name="d")[0].depflag == dt.BUILD | dt.LINK
         assert s["f"].edges_from_dependents(name="e")[0].depflag == dt.RUN
 
@@ -898,8 +899,9 @@ def test_adding_same_deptype_with_the_same_name_raises(
 
 @pytest.mark.regression("33499")
 def test_indexing_prefers_direct_or_transitive_link_deps():
-    # Test whether spec indexing prefers direct/transitive link type deps over deps of
+    """Tests whether spec indexing prefers direct/transitive link/run type deps over deps of
-    # build/run/test deps, and whether it does fall back to a full dag search.
+    build/test deps.
+    """
     root = Spec("root")
 
     # Use a and z to since we typically traverse by edges sorted alphabetically.
@@ -912,7 +914,7 @@ def test_indexing_prefers_direct_or_transitive_link_deps():
     z3_flavor_1 = Spec("z3 +through_a1")
     z3_flavor_2 = Spec("z3 +through_z1")
 
-    root.add_dependency_edge(a1, depflag=dt.BUILD | dt.RUN | dt.TEST, virtuals=())
+    root.add_dependency_edge(a1, depflag=dt.BUILD | dt.TEST, virtuals=())
 
     # unique package as a dep of a build/run/test type dep.
     a1.add_dependency_edge(a2, depflag=dt.ALL, virtuals=())
@@ -927,8 +929,9 @@ def test_indexing_prefers_direct_or_transitive_link_deps():
     assert "through_z1" in root["z3"].variants
     assert "through_a1" in a1["z3"].variants
 
-    # Ensure that the full DAG is still searched
+    # Ensure that only the runtime sub-DAG can be searched
-    assert root["a2"]
+    with pytest.raises(KeyError):
+        root["a2"]
 
 
 def test_getitem_sticks_to_subdag():

lib/spack/spack/test/spec_semantics.py

@@ -700,9 +700,9 @@ def test_unsatisfiable_multi_value_variant(self, default_mock_concretization):
     def test_copy_satisfies_transitive(self):
         spec = spack.concretize.concretize_one("dttop")
         copy = spec.copy()
-        for s in spec.traverse():
+        for s, t in zip(spec.traverse(), copy.traverse()):
-            assert s.satisfies(copy[s.name])
+            assert s.satisfies(t)
-            assert copy[s.name].satisfies(s)
+            assert t.satisfies(s)
 
     def test_intersects_virtual(self):
         assert Spec("mpich").intersects(Spec("mpi"))