Disable module load, unload, ... invocations via wrapper

This change disables module commands that modify the environment via a
wrapper when running a custom build script. This avoids preprocessing
the custom build script for module statements which may produce an
invalid bash script. This also allows for module commands that do not
alter the environment to be executed as we only disable a subset of
subcommands in the wrapper.

Fixes #249

benchcab/data/environment_modules_wrapper.bash

@@ -0,0 +1,17 @@
+#!/bin/bash
+
+# Wrapper around the module (environment modules) command which disables
+# commands that modify the current environment.
+module() {
+    args=("$@")
+    for arg in "${args[@]}"; do
+        case $arg in
+            add|load|rm|unload|swap|switch|use|unuse|purge)
+                echo "command disabled: module ""${args[*]}" 1>&2
+                return 1
+                ;;
+        esac
+    done
+    _module_raw "${args[@]}" 2>&1
+    return $?
+}

benchcab/model.py

@@ -4,15 +4,12 @@
 """Contains functions and data structures relating to CABLE models."""
 
 import os
-import shlex
-import shutil
-import stat
 from pathlib import Path
 from typing import Optional
 
 from benchcab import internal
 from benchcab.environment_modules import EnvironmentModules, EnvironmentModulesInterface
-from benchcab.utils import get_logger
+from benchcab.utils import get_logger, get_package_data_path
 from benchcab.utils.fs import chdir, copy2, rename
 from benchcab.utils.repo import GitRepo, LocalRepo, Repo
 from benchcab.utils.subprocess import SubprocessWrapper, SubprocessWrapperInterface
@@ -99,22 +96,13 @@ def custom_build(self, modules: list[str]):
             )
             raise FileNotFoundError(msg)
 
-        tmp_script_path = build_script_path.parent / "tmp-build.sh"
-
-        self.logger.debug(f"Copying {build_script_path} to {tmp_script_path}")
-        shutil.copy(build_script_path, tmp_script_path)
-
-        self.logger.debug(f"chmod +x {tmp_script_path}")
-        tmp_script_path.chmod(tmp_script_path.stat().st_mode | stat.S_IEXEC)
-
-        self.logger.debug(
-            f"Modifying {tmp_script_path.name}: remove lines that call environment modules"
-        )
-
-        remove_module_lines(tmp_script_path)
-
         with chdir(build_script_path.parent), self.modules_handler.load(modules):
-            self.subprocess_handler.run_cmd(f"./{tmp_script_path.name}")
+            modules_wrapper_path = get_package_data_path(
+                Path("environment_modules_wrapper.bash")
+            )
+            self.subprocess_handler.run_cmd(
+                f"source {modules_wrapper_path}; ./{build_script_path.name}"
+            )
 
     def pre_build(self, mpi=False):
         """Runs CABLE pre-build steps."""
@@ -170,14 +158,3 @@ def post_build(self, mpi=False):
             tmp_dir / exe,
             path_to_repo / self.src_dir / "offline" / exe,
         )
-
-
-def remove_module_lines(file_path: Path) -> None:
-    """Remove lines from `file_path` that call the environment modules package."""
-    with file_path.open("r", encoding="utf-8") as file:
-        contents = file.read()
-    with file_path.open("w", encoding="utf-8") as file:
-        for line in contents.splitlines(True):
-            cmds = shlex.split(line, comments=True)
-            if "module" not in cmds:
-                file.write(line)

benchcab/utils/__init__.py

@@ -32,6 +32,23 @@ def get_installed_root() -> Path:
     return Path(resources.files("benchcab"))
 
 
+def get_package_data_path(resource: Path) -> Path:
+    """Return the absolute path to a given resource in the package data directory.
+
+    Parameters
+    ----------
+    resource: Path
+        Path to the resource relative to the package data directory.
+
+    Returns
+    -------
+    Path
+        Absolute path to the resource.
+
+    """
+    return Path(sys.modules["benchcab"].__file__).parent / "data" / resource
+
+
 def load_package_data(filename: str) -> Union[str, dict]:
     """Load data out of the installed package data directory.
 

tests/test_model.py

@@ -11,7 +11,7 @@
 import pytest
 
 from benchcab import internal
-from benchcab.model import Model, remove_module_lines
+from benchcab.model import Model
 from benchcab.utils.repo import Repo
 
 
@@ -293,14 +293,6 @@ def modules(self):
         """Return a list of modules for testing."""
         return ["foo", "bar"]
 
-    def test_build_command_execution(
-        self, model, mock_subprocess_handler, build_script, modules
-    ):
-        """Success case: execute the build command for a custom build script."""
-        model.build_script = str(build_script)
-        model.custom_build(modules)
-        assert "./tmp-build.sh" in mock_subprocess_handler.commands
-
     def test_modules_loaded_at_runtime(
         self, model, mock_environment_modules_handler, build_script, modules
     ):
@@ -326,54 +318,3 @@ def test_file_not_found_exception(self, model, build_script, modules):
             "option in config.yaml?",
         ):
             model.custom_build(modules)
-
-
-class TestRemoveModuleLines:
-    """Tests for `remove_module_lines()`."""
-
-    def test_module_lines_removed_from_shell_script(self):
-        """Success case: test 'module' lines are removed from mock shell script."""
-        file_path = Path("test-build.sh")
-        with file_path.open("w", encoding="utf-8") as file:
-            file.write(
-                """#!/bin/bash
-module add bar
-module purge
-
-host_gadi()
-{
-   . /etc/bashrc
-   module purge
-   module add intel-compiler/2019.5.281
-   module add netcdf/4.6.3
-   module load foo
-   modules
-   echo foo && module load
-   echo foo # module load
-   # module load foo
-
-   if [[ $1 = 'mpi' ]]; then
-      module add intel-mpi/2019.5.281
-   fi
-}
-"""
-            )
-
-        remove_module_lines(file_path)
-
-        with file_path.open("r", encoding="utf-8") as file:
-            assert file.read() == (
-                """#!/bin/bash
-
-host_gadi()
-{
-   . /etc/bashrc
-   modules
-   echo foo # module load
-   # module load foo
-
-   if [[ $1 = 'mpi' ]]; then
-   fi
-}
-"""
-            )