modules: allow autoload: run, like in environment views (#42743)

3 files changed, 30 insertions, 35 deletions

diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst
index 242817565c..fad1e414fc 100644
--- a/lib/spack/docs/module_file_support.rst
+++ b/lib/spack/docs/module_file_support.rst
@@ -273,9 +273,21 @@ builtin support through the ``depends_on`` function, the latter simply uses a ``
 statement. Both module systems (at least in newer versions) do reference counting, so that if a
 module is loaded by two different modules, it will only be unloaded after the others are.
 
-The ``autoload`` key accepts the values ``none``, ``direct``, and ``all``. To disable it, use
-``none``, and to enable, it's best to stick to ``direct``, which only autoloads the direct link and
-run type dependencies, relying on recursive autoloading to load the rest.
+The ``autoload`` key accepts the values:
+
+  * ``none``: no autoloading
+  * ``run``: autoload direct *run* type dependencies
+  * ``direct``: autoload direct *link and run* type dependencies
+  * ``all``: autoload all dependencies
+
+In case of ``run`` and ``direct``, a ``module load`` triggers a recursive load.
+
+The ``direct`` option is most correct: there are cases where pure link dependencies need to set
+variables for themselves, or need to have variables of their own dependencies set.
+
+In practice however, ``run`` is often sufficient, and may make ``module load`` snappier.
+
+The ``all`` option is discouraged and seldomly used.
 
 A common complaint about autoloading is the large number of modules that are visible to the user.
 Spack has a solution for this as well: ``hide_implicits: true``. This ensures that only those
@@ -297,11 +309,11 @@ Environment Modules requires version 4.7 or higher.
             tcl:
               hide_implicits: true
               all:
-                autoload: direct
+                autoload: direct # or `run`
             lmod:
               hide_implicits: true
               all:
-                autoload: direct
+                autoload: direct # or `run`
 
 .. _anonymous_specs:
 
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index a0b9870f12..20d75f61c1 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -121,43 +121,26 @@ def update_dictionary_extending_lists(target, update):
             target[key] = update[key]
 
 
-def dependencies(spec, request="all"):
-    """Returns the list of dependent specs for a given spec, according to the
-    request passed as parameter.
+def dependencies(spec: spack.spec.Spec, request: str = "all") -> List[spack.spec.Spec]:
+    """Returns the list of dependent specs for a given spec.
 
     Args:
         spec: spec to be analyzed
-        request: either 'none', 'direct' or 'all'
+        request: one of "none", "run", "direct", "all"
 
     Returns:
-        list of dependencies
-
-        The return list will be empty if request is 'none', will contain
-        the direct dependencies if request is 'direct', or the entire DAG
-        if request is 'all'.
+        list of requested dependencies
     """
-    if request not in ("none", "direct", "all"):
-        message = "Wrong value for argument 'request' : "
-        message += "should be one of ('none', 'direct', 'all')"
-        raise tty.error(message + " [current value is '%s']" % request)
-
     if request == "none":
         return []
-
-    if request == "direct":
-        return spec.dependencies(deptype=("link", "run"))
-
-    # FIXME : during module file creation nodes seem to be visited multiple
-    # FIXME : times even if cover='nodes' is given. This work around permits
-    # FIXME : to get a unique list of spec anyhow. Do we miss a merge
-    # FIXME : step among nodes that refer to the same package?
-    seen = set()
-    seen_add = seen.add
-    deps = sorted(
-        spec.traverse(order="post", cover="nodes", deptype=("link", "run"), root=False),
-        reverse=True,
-    )
-    return [d for d in deps if not (d in seen or seen_add(d))]
+    elif request == "run":
+        return spec.dependencies(deptype=dt.RUN)
+    elif request == "direct":
+        return spec.dependencies(deptype=dt.RUN | dt.LINK)
+    elif request == "all":
+        return list(spec.traverse(order="topo", deptype=dt.LINK | dt.RUN, root=False))
+
+    raise ValueError(f'request "{request}" is not one of "none", "direct", "run", "all"')
 
 
 def merge_config_rules(configuration, spec):
diff --git a/lib/spack/spack/schema/modules.py b/lib/spack/spack/schema/modules.py
index 02c8f757f2..15d933404c 100644
--- a/lib/spack/spack/schema/modules.py
+++ b/lib/spack/spack/schema/modules.py
@@ -34,7 +34,7 @@ array_of_strings = {"type": "array", "default": [], "items": {"type": "string"}}
 
 dictionary_of_strings = {"type": "object", "patternProperties": {r"\w[\w-]*": {"type": "string"}}}
 
-dependency_selection = {"type": "string", "enum": ["none", "direct", "all"]}
+dependency_selection = {"type": "string", "enum": ["none", "run", "direct", "all"]}
 
 module_file_configuration = {
     "type": "object",