Fix spack graph when deptypes are filtered (#22121)

lib/spack/spack/graph.py

@@ -62,17 +62,16 @@ def topological_sort(spec, reverse=False, deptype='all'):
     """
     deptype = canonical_deptype(deptype)
 
-    if not reverse:
-        parents = lambda s: s.dependents()
-        children = lambda s: s.dependencies()
-    else:
-        parents = lambda s: s.dependencies()
-        children = lambda s: s.dependents()
-
     # Work on a copy so this is nondestructive.
     spec = spec.copy(deps=deptype)
     nodes = spec.index(deptype=deptype)
 
+    parents = lambda s: [p for p in s.dependents() if p.name in nodes]
+    children = lambda s: s.dependencies()
+
+    if reverse:
+        parents, children = children, parents
+
     topo_order = []
     par = dict((name, parents(nodes[name])) for name in nodes.keys())
     remaining = [name for name in nodes.keys() if not parents(nodes[name])]

lib/spack/spack/test/directory_layout.py

@@ -120,7 +120,7 @@ def test_read_and_write_spec(temporary_store, config, mock_packages):
         # TODO: fix this when we can concretize more loosely based on
         # TODO: what is installed. We currently omit these to
         # TODO: increase reuse of build dependencies.
-        stored_deptypes = ('link', 'run')
+        stored_deptypes = spack.hash_types.full_hash
         expected = spec.copy(deps=stored_deptypes)
         expected._mark_concrete()
 

lib/spack/spack/test/graph.py

@@ -122,3 +122,12 @@ def test_ascii_graph_mpileaks(mock_packages):
 |/
 o  libelf
 '''
+
+
+def test_topo_sort_filtered(mock_packages):
+    """Test topo sort gives correct order when filtering link deps."""
+    s = Spec('both-link-and-build-dep-a').normalized()
+
+    topo = topological_sort(s, deptype=('link',))
+
+    assert topo == ['both-link-and-build-dep-a', 'both-link-and-build-dep-c']

var/spack/repos/builtin.mock/packages/both-link-and-build-dep-a/package.py

@@ -0,0 +1,24 @@
+# Copyright 2013-2021 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)
+
+from spack import *
+
+
+class BothLinkAndBuildDepA(Package):
+    """
+        Structure where c occurs as a build dep down the line and as a direct
+        link dep. Useful for testing situations where you copy the parent spec
+        just with link deps, and you want to make sure b is not part of that.
+        a <--build-- b <-link-- c
+        a <--link--- c
+    """
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    depends_on('both-link-and-build-dep-b', type='build')
+    depends_on('both-link-and-build-dep-c', type='link')

var/spack/repos/builtin.mock/packages/both-link-and-build-dep-b/package.py

@@ -0,0 +1,23 @@
+# Copyright 2013-2021 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)
+
+from spack import *
+
+
+class BothLinkAndBuildDepB(Package):
+    """
+        Structure where c occurs as a build dep down the line and as a direct
+        link dep. Useful for testing situations where you copy the parent spec
+        just with link deps, and you want to make sure b is not part of that.
+        a <--build-- b <-link-- c
+        a <--link--- c
+    """
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    depends_on('both-link-and-build-dep-c', type='link')

var/spack/repos/builtin.mock/packages/both-link-and-build-dep-c/package.py

@@ -0,0 +1,21 @@
+# Copyright 2013-2021 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)
+
+from spack import *
+
+
+class BothLinkAndBuildDepC(Package):
+    """
+        Structure where c occurs as a build dep down the line and as a direct
+        link dep. Useful for testing situations where you copy the parent spec
+        just with link deps, and you want to make sure b is not part of that.
+        a <--build-- b <-link-- c
+        a <--link--- c
+    """
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')