From a92f1e37aaf1fcdf78f6fe0f225dbd426e063302 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Tue, 2 May 2023 10:28:27 +0200
Subject: [PATCH] Disable module file generation by default (#37258)

* Disable module generation by default (#35564)

a) It's used by site administrators, so it's niche
b) If it's used by site administrators, they likely need to modify the config anyhow, so the default config only serves as an example to get started
c) it's too arbitrary to enable tcl, but disable lmod

* Remove leftover from old module file schema

* Warn if module file config is detected and generation is disabled

---------

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
---
 etc/spack/defaults/modules.yaml               |  5 +--
 .../spack/hooks/module_file_generation.py     | 15 ++-----
 lib/spack/spack/modules/__init__.py           |  9 ++++-
 lib/spack/spack/modules/common.py             | 39 +++++++++++++++++++
 4 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/etc/spack/defaults/modules.yaml b/etc/spack/defaults/modules.yaml
index d53e05b2c8..75ec366117 100644
--- a/etc/spack/defaults/modules.yaml
+++ b/etc/spack/defaults/modules.yaml
@@ -40,9 +40,8 @@ modules:
     roots:
      tcl:    $spack/share/spack/modules
      lmod:   $spack/share/spack/lmod
-    # What type of modules to use
-    enable:
-      - tcl
+    # What type of modules to use ("tcl" and/or "lmod")
+    enable: []
 
     tcl:
       all:
diff --git a/lib/spack/spack/hooks/module_file_generation.py b/lib/spack/spack/hooks/module_file_generation.py
index e5e83cc9c8..dc86c43205 100644
--- a/lib/spack/spack/hooks/module_file_generation.py
+++ b/lib/spack/spack/hooks/module_file_generation.py
@@ -3,31 +3,24 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import llnl.util.tty as tty
+from llnl.util import tty
 
 import spack.config
 import spack.modules
-import spack.modules.common
 
 
 def _for_each_enabled(spec, method_name, explicit=None):
     """Calls a method for each enabled module"""
+    spack.modules.ensure_modules_are_enabled_or_warn()
     set_names = set(spack.config.get("modules", {}).keys())
-    # If we have old-style modules enabled, we put those in the default set
-    old_default_enabled = spack.config.get("modules:enable")
-    if old_default_enabled:
-        set_names.add("default")
     for name in set_names:
         enabled = spack.config.get("modules:%s:enable" % name)
-        if name == "default":
-            # combine enabled modules from default and old format
-            enabled = spack.config.merge_yaml(old_default_enabled, enabled)
         if not enabled:
             tty.debug("NO MODULE WRITTEN: list of enabled module files is empty")
             continue
 
-        for type in enabled:
-            generator = spack.modules.module_types[type](spec, name, explicit)
+        for module_type in enabled:
+            generator = spack.modules.module_types[module_type](spec, name, explicit)
             try:
                 getattr(generator, method_name)()
             except RuntimeError as e:
diff --git a/lib/spack/spack/modules/__init__.py b/lib/spack/spack/modules/__init__.py
index 4c56929a05..e9049a43bb 100644
--- a/lib/spack/spack/modules/__init__.py
+++ b/lib/spack/spack/modules/__init__.py
@@ -9,10 +9,15 @@
 
 from __future__ import absolute_import
 
-from .common import disable_modules
+from .common import disable_modules, ensure_modules_are_enabled_or_warn
 from .lmod import LmodModulefileWriter
 from .tcl import TclModulefileWriter
 
-__all__ = ["TclModulefileWriter", "LmodModulefileWriter", "disable_modules"]
+__all__ = [
+    "TclModulefileWriter",
+    "LmodModulefileWriter",
+    "disable_modules",
+    "ensure_modules_are_enabled_or_warn",
+]
 
 module_types = {"tcl": TclModulefileWriter, "lmod": LmodModulefileWriter}
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index b5e084bc0c..7dbe6d8988 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -33,7 +33,9 @@
 import datetime
 import inspect
 import os.path
+import pathlib
 import re
+import warnings
 from typing import Optional
 
 import llnl.util.filesystem
@@ -801,6 +803,43 @@ def verbose(self):
         return self.conf.verbose
 
 
+def ensure_modules_are_enabled_or_warn():
+    """Ensures that, if a custom configuration file is found with custom configuration for the
+    default tcl module set, then tcl module file generation is enabled. Otherwise, a warning
+    is emitted.
+    """
+
+    # TODO (v0.21 - Remove this function)
+    # Check if TCL module generation is enabled, return early if it is
+    enabled = spack.config.get("modules:default:enable", [])
+    if "tcl" in enabled:
+        return
+
+    # Check if we have custom TCL module sections
+    for scope in spack.config.config.file_scopes:
+        # Skip default configuration
+        if scope.name.startswith("default"):
+            continue
+
+        data = spack.config.get("modules:default:tcl", scope=scope.name)
+        if data:
+            config_file = pathlib.Path(scope.path)
+            if not scope.name.startswith("env"):
+                config_file = config_file / "modules.yaml"
+            break
+    else:
+        return
+
+    # If we are here we have a custom "modules" section in "config_file"
+    msg = (
+        f"detected custom TCL modules configuration in {config_file}, while TCL module file "
+        f"generation for the default module set is disabled. "
+        f"In Spack v0.20 module file generation has been disabled by default. To enable "
+        f"it run:\n\n\t$ spack config add 'modules:default:enable:[tcl]'\n"
+    )
+    warnings.warn(msg)
+
+
 class BaseModuleFileWriter(object):
     def __init__(self, spec, module_set_name, explicit=None):
         self.spec = spec