diff --git a/lib/spack/spack/cmd/dependencies.py b/lib/spack/spack/cmd/dependencies.py index e65e050bfa..7f390341ef 100644 --- a/lib/spack/spack/cmd/dependencies.py +++ b/lib/spack/spack/cmd/dependencies.py @@ -9,6 +9,7 @@ import spack.cmd import spack.cmd.common.arguments as arguments import spack.environment as ev +import spack.package import spack.repo import spack.store @@ -52,22 +53,15 @@ def dependencies(parser, args): else: spec = specs[0] - - if not spec.virtual: - packages = [spec.package] - else: - packages = [ - spack.repo.get(s.name) - for s in spack.repo.path.providers_for(spec)] - - dependencies = set() - for pkg in packages: - possible = pkg.possible_dependencies( - args.transitive, args.expand_virtuals, deptype=args.deptype) - dependencies.update(possible) + dependencies = spack.package.possible_dependencies( + spec, + transitive=args.transitive, + expand_virtuals=args.expand_virtuals, + deptype=args.deptype + ) if spec.name in dependencies: - dependencies.remove(spec.name) + del dependencies[spec.name] if dependencies: colify(sorted(dependencies)) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index de0f9b3568..9156d7d0dd 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -605,11 +605,10 @@ def possible_dependencies( """ deptype = spack.dependency.canonical_deptype(deptype) - if visited is None: - visited = {cls.name: set()} + visited = {} if visited is None else visited + missing = {} if missing is None else missing - if missing is None: - missing = {cls.name: set()} + visited.setdefault(cls.name, set()) for name, conditions in cls.dependencies.items(): # check whether this dependency could be of the type asked for @@ -624,6 +623,7 @@ def possible_dependencies( providers = spack.repo.path.providers_for(name) dep_names = [spec.name for spec in providers] else: + visited.setdefault(cls.name, set()).add(name) visited.setdefault(name, set()) continue else: @@ -2157,13 +2157,20 @@ def possible_dependencies(*pkg_or_spec, **kwargs): packages = [] for pos in pkg_or_spec: if isinstance(pos, PackageMeta): - pkg = pos - elif isinstance(pos, spack.spec.Spec): - pkg = pos.package - else: - pkg = spack.spec.Spec(pos).package + packages.append(pos) + continue - packages.append(pkg) + if not isinstance(pos, spack.spec.Spec): + pos = spack.spec.Spec(pos) + + if spack.repo.path.is_virtual(pos.name): + packages.extend( + p.package_class + for p in spack.repo.path.providers_for(pos.name) + ) + continue + else: + packages.append(pos.package_class) visited = {} for pkg in packages: diff --git a/lib/spack/spack/test/cmd/dependencies.py b/lib/spack/spack/test/cmd/dependencies.py index fc47069181..05d0556936 100644 --- a/lib/spack/spack/test/cmd/dependencies.py +++ b/lib/spack/spack/test/cmd/dependencies.py @@ -17,7 +17,7 @@ mpi_deps = ['fake'] -def test_immediate_dependencies(mock_packages): +def test_direct_dependencies(mock_packages): out = dependencies('mpileaks') actual = set(re.split(r'\s+', out.strip())) expected = set(['callpath'] + mpis) @@ -47,7 +47,7 @@ def test_transitive_dependencies_with_deptypes(mock_packages): @pytest.mark.db -def test_immediate_installed_dependencies(mock_packages, database): +def test_direct_installed_dependencies(mock_packages, database): with color_when(False): out = dependencies('--installed', 'mpileaks^mpich') diff --git a/lib/spack/spack/test/package_class.py b/lib/spack/spack/test/package_class.py index b3351ffb49..d540ac663e 100644 --- a/lib/spack/spack/test/package_class.py +++ b/lib/spack/spack/test/package_class.py @@ -11,12 +11,17 @@ """ import pytest +import spack.package import spack.repo -@pytest.fixture -def mpileaks_possible_deps(mock_packages): - mpi_names = [spec.name for spec in spack.repo.path.providers_for('mpi')] +@pytest.fixture(scope="module") +def mpi_names(mock_repo_path): + return [spec.name for spec in mock_repo_path.providers_for('mpi')] + + +@pytest.fixture() +def mpileaks_possible_deps(mock_packages, mpi_names): possible = { 'callpath': set(['dyninst'] + mpi_names), 'dyninst': set(['libdwarf', 'libelf']), @@ -34,47 +39,72 @@ def mpileaks_possible_deps(mock_packages): def test_possible_dependencies(mock_packages, mpileaks_possible_deps): mpileaks = spack.repo.get('mpileaks') - assert (mpileaks.possible_dependencies(expand_virtuals=True) == - mpileaks_possible_deps) + assert mpileaks_possible_deps == ( + mpileaks.possible_dependencies(expand_virtuals=True)) - assert mpileaks.possible_dependencies(expand_virtuals=False) == { - 'callpath': set(['dyninst']), + assert { + 'callpath': set(['dyninst', 'mpi']), 'dyninst': set(['libdwarf', 'libelf']), 'libdwarf': set(['libelf']), 'libelf': set(), 'mpi': set(), - 'mpileaks': set(['callpath']), - } + 'mpileaks': set(['callpath', 'mpi']), + } == mpileaks.possible_dependencies(expand_virtuals=False) + + +def test_possible_direct_dependencies(mock_packages, mpileaks_possible_deps): + mpileaks = spack.repo.get('mpileaks') + deps = mpileaks.possible_dependencies(transitive=False, + expand_virtuals=False) + + assert { + 'callpath': set(), + 'mpi': set(), + 'mpileaks': set(['callpath', 'mpi']), + } == deps + + +def test_possible_dependencies_virtual(mock_packages, mpi_names): + expected = dict( + (name, set(spack.repo.get(name).dependencies)) + for name in mpi_names + ) + + # only one mock MPI has a dependency + expected['fake'] = set() + + assert expected == spack.package.possible_dependencies( + "mpi", transitive=False) def test_possible_dependencies_missing(mock_packages): md = spack.repo.get("missing-dependency") missing = {} md.possible_dependencies(transitive=True, missing=missing) - assert missing["missing-dependency"] == set([ + assert set([ "this-is-a-missing-dependency" - ]) + ]) == missing["missing-dependency"] def test_possible_dependencies_with_deptypes(mock_packages): dtbuild1 = spack.repo.get('dtbuild1') - assert dtbuild1.possible_dependencies(deptype=('link', 'run')) == { + assert { 'dtbuild1': set(['dtrun2', 'dtlink2']), 'dtlink2': set(), 'dtrun2': set(), - } + } == dtbuild1.possible_dependencies(deptype=('link', 'run')) - assert dtbuild1.possible_dependencies(deptype=('build')) == { + assert { 'dtbuild1': set(['dtbuild2', 'dtlink2']), 'dtbuild2': set(), 'dtlink2': set(), - } + } == dtbuild1.possible_dependencies(deptype=('build')) - assert dtbuild1.possible_dependencies(deptype=('link')) == { + assert { 'dtbuild1': set(['dtlink2']), 'dtlink2': set(), - } + } == dtbuild1.possible_dependencies(deptype=('link')) def test_possible_dependencies_with_multiple_classes( @@ -88,4 +118,4 @@ def test_possible_dependencies_with_multiple_classes( 'dt-diamond-bottom': set(), }) - assert spack.package.possible_dependencies(*pkgs) == expected + assert expected == spack.package.possible_dependencies(*pkgs)