From 1659beb220bde672ff50a47af934e8ba3b7323e5 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 9 Mar 2021 01:47:00 +0100 Subject: [PATCH] Fix `spack graph` when deptypes are filtered (#22121) --- lib/spack/spack/graph.py | 13 +++++----- lib/spack/spack/test/directory_layout.py | 2 +- lib/spack/spack/test/graph.py | 9 +++++++ .../both-link-and-build-dep-a/package.py | 24 +++++++++++++++++++ .../both-link-and-build-dep-b/package.py | 23 ++++++++++++++++++ .../both-link-and-build-dep-c/package.py | 21 ++++++++++++++++ 6 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/both-link-and-build-dep-a/package.py create mode 100644 var/spack/repos/builtin.mock/packages/both-link-and-build-dep-b/package.py create mode 100644 var/spack/repos/builtin.mock/packages/both-link-and-build-dep-c/package.py diff --git a/lib/spack/spack/graph.py b/lib/spack/spack/graph.py index 2ddc10ac7ac..87ad51e030a 100644 --- a/lib/spack/spack/graph.py +++ b/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])] diff --git a/lib/spack/spack/test/directory_layout.py b/lib/spack/spack/test/directory_layout.py index 04e42549d60..73446714ba3 100644 --- a/lib/spack/spack/test/directory_layout.py +++ b/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() diff --git a/lib/spack/spack/test/graph.py b/lib/spack/spack/test/graph.py index 9242cd2717e..76c95247288 100644 --- a/lib/spack/spack/test/graph.py +++ b/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'] diff --git a/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-a/package.py b/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-a/package.py new file mode 100644 index 00000000000..11efe3efc00 --- /dev/null +++ b/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') diff --git a/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-b/package.py b/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-b/package.py new file mode 100644 index 00000000000..855ca1fc060 --- /dev/null +++ b/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') diff --git a/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-c/package.py b/var/spack/repos/builtin.mock/packages/both-link-and-build-dep-c/package.py new file mode 100644 index 00000000000..a1507e8f349 --- /dev/null +++ b/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')