From 443d702971be3ce7fa203348cb3eae16b55e0909 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 10 Jul 2018 10:54:55 +0200 Subject: spack load exits with 1 if module does not exist or is not installed fixes #2215 fixes #2570 fixes #6676 fixes #7281 closes #3827 This PR reverts the use of `spack module loads` in favor of `spack module find` when loading module files via Spack. After this PR `spack load` will accept a single spec at a time, and will be able to interpret correctly the `--dependencies` option. --- lib/spack/spack/cmd/common/modules.py | 35 ++++++++++++++++++++++++------ lib/spack/spack/test/cmd/tcl.py | 40 +++++++++++++++++++++++++++++++++++ share/spack/setup-env.sh | 8 +++++++ 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/lib/spack/spack/cmd/common/modules.py b/lib/spack/spack/cmd/common/modules.py index 964d89d668..11ee9cdb5e 100644 --- a/lib/spack/spack/cmd/common/modules.py +++ b/lib/spack/spack/cmd/common/modules.py @@ -71,7 +71,9 @@ def setup_parser(subparser): help='display full path to module file', action='store_true' ) - arguments.add_common_arguments(find_parser, ['constraint']) + arguments.add_common_arguments( + find_parser, ['constraint', 'recurse_dependencies'] + ) rm_parser = sp.add_parser('rm', help='remove module files') arguments.add_common_arguments( @@ -180,17 +182,36 @@ def find(module_type, specs, args): spec = one_spec_or_raise(specs) # Check if the module file is present - writer = spack.modules.module_types[module_type](spec) - if not os.path.isfile(writer.layout.filename): + def module_exists(spec): + writer = spack.modules.module_types[module_type](spec) + return os.path.isfile(writer.layout.filename) + + if not module_exists(spec): msg = 'Even though {1} is installed, ' msg += 'no {0} module has been generated for it.' tty.die(msg.format(module_type, spec)) + # Check if we want to recurse and load all dependencies. In that case + # modify the list of specs adding all the dependencies in post order + if args.recurse_dependencies: + specs = [ + item for item in spec.traverse(order='post', cover='nodes') + if module_exists(item) + ] + # ... and if it is print its use name or full-path if requested - if args.full_path: - print(writer.layout.filename) - else: - print(writer.layout.use_name) + def module_str(specs): + modules = [] + for x in specs: + writer = spack.modules.module_types[module_type](x) + if args.full_path: + modules.append(writer.layout.filename) + else: + modules.append(writer.layout.use_name) + + return ' '.join(modules) + + print(module_str(specs)) def rm(module_type, specs, args): diff --git a/lib/spack/spack/test/cmd/tcl.py b/lib/spack/spack/test/cmd/tcl.py index ba3e322b49..d5deffce6b 100644 --- a/lib/spack/spack/test/cmd/tcl.py +++ b/lib/spack/spack/test/cmd/tcl.py @@ -93,3 +93,43 @@ def test_remove_and_add_tcl(database, parser): def test_find(database, cli_args): """Tests 'spack tcl find' under a few common scenarios.""" tcl(*(['find'] + cli_args)) + + +@pytest.mark.db +@pytest.mark.usefixtures('database') +@pytest.mark.regression('2215') +def test_find_fails_on_multiple_matches(): + # As we installed multiple versions of mpileaks, the command will + # fail because of multiple matches + out = tcl('find', 'mpileaks', fail_on_error=False) + assert tcl.returncode == 1 + assert 'matches multiple packages' in out + + # Passing multiple packages from the command line also results in the + # same failure + out = tcl('find', 'mpileaks ^mpich', 'libelf', fail_on_error=False) + assert tcl.returncode == 1 + assert 'matches multiple packages' in out + + +@pytest.mark.db +@pytest.mark.usefixtures('database') +@pytest.mark.regression('2570') +def test_find_fails_on_non_existing_packages(): + # Another way the command might fail is if the package does not exist + out = tcl('find', 'doesnotexist', fail_on_error=False) + assert tcl.returncode == 1 + assert 'matches no package' in out + + +@pytest.mark.db +@pytest.mark.usefixtures('database') +def test_find_recursive(): + # If we call find without options it should return only one module + out = tcl('find', 'mpileaks ^zmpi') + assert len(out.split()) == 1 + + # If instead we call it with the recursive option the length should + # be greater + out = tcl('find', '-r', 'mpileaks ^zmpi') + assert len(out.split()) > 1 diff --git a/share/spack/setup-env.sh b/share/spack/setup-env.sh index 5774e8f762..6e5ab2b897 100755 --- a/share/spack/setup-env.sh +++ b/share/spack/setup-env.sh @@ -123,18 +123,26 @@ function spack { "use") if _sp_full_spec=$(command spack $_sp_flags dotkit find $_sp_subcommand_args "${_sp_spec[@]}"); then use $_sp_module_args $_sp_full_spec + else + $(exit 1) fi ;; "unuse") if _sp_full_spec=$(command spack $_sp_flags dotkit find $_sp_subcommand_args "${_sp_spec[@]}"); then unuse $_sp_module_args $_sp_full_spec + else + $(exit 1) fi ;; "load") if _sp_full_spec=$(command spack $_sp_flags tcl find $_sp_subcommand_args "${_sp_spec[@]}"); then module load $_sp_module_args $_sp_full_spec + else + $(exit 1) fi ;; "unload") if _sp_full_spec=$(command spack $_sp_flags tcl find $_sp_subcommand_args "${_sp_spec[@]}"); then module unload $_sp_module_args $_sp_full_spec + else + $(exit 1) fi ;; esac ;; -- cgit v1.2.3-60-g2f50