only use modules.yaml from recipe (#275)

Do not use modules.yaml from the Spack installation if no modules.yaml file is provided.

Fix #274

Author: albestro

docs/recipes.md

@@ -34,7 +34,7 @@ version: 2
 * `name`: a plain text name for the environment
 * `store`: the location where the environment will be mounted.
 * `spack`: which spack and package repositories to use for installation.
-* `modules`: _optional_ enable/diasble module file generation (default `true`).
+* `modules`: (_deprecated_) _optional_ enable/disable module file generation.
 * `description`: _optional_ a string that describes the environment (default empty).
 * `version`:  _default = 1_ the version of the uenv recipe (see below)
 
@@ -440,11 +440,12 @@ The `append_path` field is the same as `prepend_path`, except it appends instead
 
 ## Modules
 
-Modules are generated for the installed compilers and packages by spack. The default module generation rules set by the version of spack specified in `config.yaml` will be used if no `modules.yaml` file is provided.
+The presence of a `modules.yaml` file in the recipe is a necessary and sufficient condition to enable module generation.
 
-To set rules for module generation, provide a `modules.yaml` file as per the [spack documentation](https://spack.readthedocs.io/en/latest/module_file_support.html).
+!!! warning
+    `config:modules` field has been deprecated. It can still be specified, but it has to be consistent with the presence of `modules.yaml` file.
 
-To disable module generation, set the field `config:modules:False` in `config.yaml`.
+Modules are generated for the installed compilers and packages by spack. Rules for module generation in `modules.yaml` file should be provided as per the [spack documentation](https://spack.readthedocs.io/en/latest/module_file_support.html).
 
 ## Custom Spack Packages
 

stackinator/builder.py

@@ -158,7 +158,7 @@ def environment_meta(self, recipe):
         meta["views"] = recipe.environment_view_meta
         meta["mount"] = str(recipe.mount)
         modules = None
-        if conf["modules"]:
+        if recipe.with_modules:
             modules = {"root": str(recipe.mount / "modules")}
         meta["modules"] = modules
         self._environment_meta = meta
@@ -227,7 +227,7 @@ def generate(self, recipe):
             f.write(
                 makefile_template.render(
                     cache=recipe.mirror,
-                    modules=recipe.config["modules"],
+                    modules=recipe.with_modules,
                     post_install_hook=recipe.post_install_hook,
                     pre_install_hook=recipe.pre_install_hook,
                     spack_version=spack_version,
@@ -463,6 +463,7 @@ def generate(self, recipe):
         with (generate_config_path / "Makefile").open("w") as f:
             f.write(
                 make_config_template.render(
+                    modules=recipe.with_modules,
                     build_path=self.path.as_posix(),
                     all_compilers=all_compilers,
                     release_compilers=all_compilers,
@@ -471,11 +472,11 @@ def generate(self, recipe):
             )
 
         # write modules/modules.yaml
-        modules_yaml = recipe.modules_yaml
-        generate_modules_path = self.path / "modules"
-        generate_modules_path.mkdir(exist_ok=True)
-        with (generate_modules_path / "modules.yaml").open("w") as f:
-            f.write(modules_yaml)
+        if recipe.with_modules:
+            generate_modules_path = self.path / "modules"
+            generate_modules_path.mkdir(exist_ok=True)
+            with (generate_modules_path / "modules.yaml").open("w") as f:
+                yaml.dump(recipe.modules, f)
 
         # write the meta data
         meta_path = store_path / "meta"

stackinator/recipe.py

@@ -63,13 +63,23 @@ def __init__(self, args):
             self.generate_compiler_specs(raw)
 
         # optional modules.yaml file
+        self.modules = None
         modules_path = self.path / "modules.yaml"
         self._logger.debug(f"opening {modules_path}")
-        if not modules_path.is_file():
-            modules_path = pathlib.Path(args.build) / "spack/etc/spack/defaults/modules.yaml"
-            self._logger.debug(f"no modules.yaml provided - using the {modules_path}")
+        if modules_path.is_file():
+            with modules_path.open() as fid:
+                self.modules = yaml.load(fid, Loader=yaml.Loader)
+                # Note: it should match MODULEPATH set by envvars and used by uenv view "modules"
+                self.modules["modules"]["default"]["roots"]["tcl"] = (pathlib.Path(self.mount) / "modules").as_posix()
 
-        self.modules = modules_path
+        # DEPRECATED field `config:modules`
+        if "modules" in self.config:
+            self._logger.warning("boolean field config.yaml:modules has been deprecated")
+
+            if self.with_modules != self.config["modules"]:
+                self._logger.error(f"config.yaml:modules:{self.config['modules']}")
+                self._logger.error(f"modules.yaml:{self.with_modules}")
+                raise RuntimeError("conflicting modules configuration detected")
 
         # optional packages.yaml file
         packages_path = self.path / "packages.yaml"
@@ -261,6 +271,10 @@ def config(self, config_path):
             schema.ConfigValidator.validate(raw)
             self._config = raw
 
+    @property
+    def with_modules(self) -> bool:
+        return self.modules is not None
+
     # In Stackinator 6 we replaced logic required to determine the
     # pre 1.0 Spack version.
     def find_spack_version(self, develop):
@@ -313,13 +327,6 @@ def environment_view_meta(self):
 
         return view_meta
 
-    @property
-    def modules_yaml(self):
-        with self.modules.open() as fid:
-            raw = yaml.load(fid, Loader=yaml.Loader)
-            raw["modules"]["default"]["roots"]["tcl"] = (pathlib.Path(self.mount) / "modules").as_posix()
-            return yaml.dump(raw)
-
     # creates the self.environments field that describes the full specifications
     # for all of the environments sets, grouped in environments, from the raw
     # environments.yaml input.

stackinator/schema/config.json

@@ -65,8 +65,7 @@
             }
         },
         "modules" : {
-            "type": "boolean",
-            "default": true
+            "type": "boolean"
         },
         "description" : {
             "oneOf": [

stackinator/templates/Makefile.generate-config

@@ -10,7 +10,7 @@ ALL_COMPILER_PREFIXES ={% for compiler in all_compilers %} $$($(SPACK_HELPER) -e
 COMPILER_PREFIXES ={% for compiler in release_compilers %} $$($(SPACK_HELPER) -e ../compilers/{{ compiler }} find --explicit --format='{prefix}' gcc llvm llvm-amdgpu nvhpc){% endfor %}
 
 
-all: $(CONFIG_DIR)/upstreams.yaml $(CONFIG_DIR)/packages.yaml $(CONFIG_DIR)/repos.yaml $(MODULE_DIR)/upstreams.yaml $(MODULE_DIR)/compilers.yaml
+all: $(CONFIG_DIR)/upstreams.yaml $(CONFIG_DIR)/packages.yaml $(CONFIG_DIR)/repos.yaml{% if modules %} $(MODULE_DIR)/upstreams.yaml $(MODULE_DIR)/compilers.yaml{% endif %}
 
 # Generate the upstream configuration that will be provided by the mounted image
 $(CONFIG_DIR)/upstreams.yaml:

unittests/test_schema.py

@@ -43,7 +43,6 @@ def test_config_yaml(yaml_path):
         assert raw["store"] == "/user-environment"
         assert raw["spack"]["commit"] is None
         assert raw["spack"]["packages"]["commit"] is None
-        assert raw["modules"] == True  # noqa: E712
         assert raw["mirror"] == {"enable": True, "key": None}
         assert raw["description"] is None
 
@@ -64,7 +63,6 @@ def test_config_yaml(yaml_path):
     schema.ConfigValidator.validate(raw)
     assert raw["spack"]["commit"] is None
     assert raw["spack"]["packages"]["commit"] is not None
-    assert raw["modules"] == True  # noqa: E712
     assert raw["mirror"] == {"enable": True, "key": None}
     assert raw["description"] is None
 
@@ -85,7 +83,6 @@ def test_config_yaml(yaml_path):
     schema.ConfigValidator.validate(raw)
     assert raw["spack"]["commit"] == "develop"
     assert raw["spack"]["packages"]["commit"] is None
-    assert raw["modules"] == True  # noqa: E712
     assert raw["mirror"] == {"enable": True, "key": None}
     assert raw["description"] is None