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

4 files changed, 115 insertions, 30 deletions

diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py
index 4f1e640f6c..fbf93e9b2b 100644
--- a/lib/spack/spack/cmd/modules/__init__.py
+++ b/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))
 
 
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 434d09c125..2e0090a449 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -312,20 +312,45 @@ class UpstreamModuleIndex(object):
         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:
diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py
index ac8b48a9e8..6f2ffcbf85 100644
--- a/lib/spack/spack/test/cmd/module.py
+++ b/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
 
diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py
index 604c19cca8..fdaf898daf 100644
--- a/lib/spack/spack/test/modules/common.py
+++ b/lib/spack/spack/test/modules/common.py
@@ -10,8 +10,7 @@ import collections
 
 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 @@ 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):