Bugfix: allow missing modules if they are blacklisted (#13540)

`spack module loads` and `spack module find` previously failed if any upstream modules were missing.  This prevented it from being used with upstreams (or, really, any spack instance) that blacklisted modules.

This PR makes module finding is now more lenient (especially for blacklisted modules).

- `spack module find` now does not report an error if the spec is blacklisted
  - instead, it prints a single warning if any modules will be omitted from the loads file
  - It comments the missing modules out of the loads file so the user can see what's missing
  - Debug messages are also printed so users can check this with `spack -d...`

- also added tests for new functionality

lib/spack/spack/cmd/modules/__init__.py

@@ -111,6 +111,14 @@ def one_spec_or_raise(specs):
     return specs[0]
 
 
+_missing_modules_warning = (
+    "Modules have been omitted for one or more specs, either"
+    " because they were blacklisted or because the spec is"
+    " associated with a package that is installed upstream and"
+    " that installation has not generated a module file. Rerun"
+    " this command with debug output enabled for more details.")
+
+
 def loads(module_type, specs, args, out=sys.stdout):
     """Prompt the list of modules associated with a list of specs"""
 
@@ -131,7 +139,9 @@ def loads(module_type, specs, args, out=sys.stdout):
             )
 
     modules = list(
-        (spec, spack.modules.common.get_module(module_type, spec, False))
+        (spec,
+         spack.modules.common.get_module(
+             module_type, spec, get_full_path=False, required=False))
         for spec in specs)
 
     module_commands = {
@@ -145,15 +155,24 @@ def loads(module_type, specs, args, out=sys.stdout):
     }
 
     exclude_set = set(args.exclude)
-    prompt_template = '{comment}{exclude}{command}{prefix}{name}'
+    load_template = '{comment}{exclude}{command}{prefix}{name}'
     for spec, mod in modules:
-        d['exclude'] = '## ' if spec.name in exclude_set else ''
-        d['comment'] = '' if not args.shell else '# {0}\n'.format(
-            spec.format())
-        d['name'] = mod
-        out.write(prompt_template.format(**d))
+        if not mod:
+            module_output_for_spec = (
+                '## blacklisted or missing from upstream: {0}'.format(
+                    spec.format()))
+        else:
+            d['exclude'] = '## ' if spec.name in exclude_set else ''
+            d['comment'] = '' if not args.shell else '# {0}\n'.format(
+                spec.format())
+            d['name'] = mod
+            module_output_for_spec = load_template.format(**d)
+        out.write(module_output_for_spec)
         out.write('\n')
 
+    if not all(mod for _, mod in modules):
+        tty.warn(_missing_modules_warning)
+
 
 def find(module_type, specs, args):
     """Retrieve paths or use names of module files"""
@@ -161,18 +180,27 @@ def find(module_type, specs, args):
     single_spec = one_spec_or_raise(specs)
 
     if args.recurse_dependencies:
-        specs_to_retrieve = list(
-            single_spec.traverse(order='post', cover='nodes',
+        dependency_specs_to_retrieve = list(
+            single_spec.traverse(root=False, order='post', cover='nodes',
                                  deptype=('link', 'run')))
     else:
-        specs_to_retrieve = [single_spec]
+        dependency_specs_to_retrieve = []
 
     try:
-        modules = [spack.modules.common.get_module(module_type, spec,
-                                                   args.full_path)
-                   for spec in specs_to_retrieve]
+        modules = [
+            spack.modules.common.get_module(
+                module_type, spec, args.full_path, required=False)
+            for spec in dependency_specs_to_retrieve]
+
+        modules.append(
+            spack.modules.common.get_module(
+                module_type, single_spec, args.full_path, required=True))
     except spack.modules.common.ModuleNotFoundError as e:
         tty.die(e.message)
+
+    if not all(modules):
+        tty.warn(_missing_modules_warning)
+    modules = list(x for x in modules if x)
     print(' '.join(modules))
 
 

lib/spack/spack/modules/common.py

@@ -312,20 +312,45 @@ def upstream_module(self, spec, module_type):
         module_index = self.module_indices[db_index]
         module_type_index = module_index.get(module_type, {})
         if not module_type_index:
-            raise ModuleNotFoundError(
+            tty.debug(
                 "No {0} modules associated with the Spack instance where"
                 " {1} is installed".format(module_type, spec))
+            return None
         if spec.dag_hash() in module_type_index:
             return module_type_index[spec.dag_hash()]
         else:
-            raise ModuleNotFoundError(
+            tty.debug(
                 "No module is available for upstream package {0}".format(spec))
+            return None
 
 
-def get_module(module_type, spec, get_full_path):
+def get_module(module_type, spec, get_full_path, required=True):
+    """Retrieve the module file for a given spec and module type.
+
+    Retrieve the module file for the given spec if it is available. If the
+    module is not available, this will raise an exception unless the module
+    is blacklisted or if the spec is installed upstream.
+
+    Args:
+        module_type: the type of module we want to retrieve (e.g. lmod)
+        spec: refers to the installed package that we want to retrieve a module
+            for
+        required: if the module is required but blacklisted, this function will
+            print a debug message. If a module is missing but not blacklisted,
+            then an exception is raised (regardless of whether it is required)
+        get_full_path: if ``True``, this returns the full path to the module.
+            Otherwise, this returns the module name.
+
+    Returns:
+        The module name or path. May return ``None`` if the module is not
+        available.
+    """
     if spec.package.installed_upstream:
-        module = spack.modules.common.upstream_module_index.upstream_module(
-            spec, module_type)
+        module = (spack.modules.common.upstream_module_index
+                  .upstream_module(spec, module_type))
+        if not module:
+            return None
+
         if get_full_path:
             return module.path
         else:
@@ -333,10 +358,17 @@ def get_module(module_type, spec, get_full_path):
     else:
         writer = spack.modules.module_types[module_type](spec)
         if not os.path.isfile(writer.layout.filename):
-            err_msg = "No module available for package {0} at {1}".format(
-                spec, writer.layout.filename
-            )
-            raise ModuleNotFoundError(err_msg)
+            if not writer.conf.blacklisted:
+                err_msg = "No module available for package {0} at {1}".format(
+                    spec, writer.layout.filename
+                )
+                raise ModuleNotFoundError(err_msg)
+            elif required:
+                tty.debug("The module configuration has blacklisted {0}: "
+                          "omitting it".format(spec))
+            else:
+                return None
+
         if get_full_path:
             return writer.layout.filename
         else:

lib/spack/spack/test/cmd/module.py

@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import os.path
+import re
 
 import pytest
 
@@ -144,6 +145,34 @@ def test_find_recursive():
     assert len(out.split()) > 1
 
 
+@pytest.mark.db
+def test_find_recursive_blacklisted(database, module_configuration):
+    module_configuration('blacklist')
+
+    module('lmod', 'refresh', '-y', '--delete-tree')
+    module('lmod', 'find', '-r', 'mpileaks ^mpich')
+
+
+@pytest.mark.db
+def test_loads_recursive_blacklisted(database, module_configuration):
+    module_configuration('blacklist')
+
+    module('lmod', 'refresh', '-y', '--delete-tree')
+    output = module('lmod', 'loads', '-r', 'mpileaks ^mpich')
+    lines = output.split('\n')
+
+    assert any(re.match(r'[^#]*module load.*mpileaks', l) for l in lines)
+    assert not any(re.match(r'[^#]module load.*callpath', l) for l in lines)
+    assert any(re.match(r'## blacklisted or missing.*callpath', l)
+               for l in lines)
+
+    # TODO: currently there is no way to separate stdout and stderr when
+    # invoking a SpackCommand. Supporting this requires refactoring
+    # SpackCommand, or log_output, or both.
+    # start_of_warning = spack.cmd.modules._missing_modules_warning[:10]
+    # assert start_of_warning not in output
+
+
 # Needed to make the 'module_configuration' fixture below work
 writer_cls = spack.modules.lmod.LmodModulefileWriter
 

lib/spack/spack/test/modules/common.py

@@ -10,8 +10,7 @@
 
 import spack.spec
 import spack.modules.tcl
-from spack.modules.common import (
-    UpstreamModuleIndex, ModuleNotFoundError)
+from spack.modules.common import UpstreamModuleIndex
 
 import spack.error
 
@@ -133,18 +132,15 @@ def test_upstream_module_index():
     assert m1.path == '/path/to/a'
 
     # No modules are defined for the DB associated with s2
-    with pytest.raises(ModuleNotFoundError):
-        upstream_index.upstream_module(s2, 'tcl')
+    assert not upstream_index.upstream_module(s2, 'tcl')
 
     # Modules are defined for the index associated with s1, but none are
     # defined for the requested type
-    with pytest.raises(ModuleNotFoundError):
-        upstream_index.upstream_module(s1, 'lmod')
+    assert not upstream_index.upstream_module(s1, 'lmod')
 
     # A module is registered with a DB and the associated module index has
     # modules of the specified type defined, but not for the requested spec
-    with pytest.raises(ModuleNotFoundError):
-        upstream_index.upstream_module(s3, 'tcl')
+    assert not upstream_index.upstream_module(s3, 'tcl')
 
     # The spec isn't recorded as installed in any of the DBs
     with pytest.raises(spack.error.SpackError):