modules: use new module function instead of get_module_cmd (#8570)

Use new `module` function instead of `get_module_cmd`

Previously, Spack relied on either examining the bash `module()` function or using the `which` command to find the underlying executable for modules. More complicated module systems do not allow for the sort of simple analysis we were doing (see #6451).

Spack now uses the `module` function directly and copies environment changes from the resulting subprocess back into Spack. This should provide a future-proof implementation for changes to the logic underlying the module system on various HPC systems.

lib/spack/spack/operating_systems/cnl.py

@@ -9,7 +9,7 @@
 import llnl.util.multiproc as mp
 
 from spack.architecture import OperatingSystem
-from spack.util.module_cmd import get_module_cmd
+from spack.util.module_cmd import module
 
 
 class Cnl(OperatingSystem):
@@ -29,8 +29,7 @@ def __str__(self):
         return self.name + str(self.version)
 
     def _detect_crayos_version(self):
-        modulecmd = get_module_cmd()
-        output = modulecmd("avail", "PrgEnv-", output=str, error=str)
+        output = module("avail", "PrgEnv-")
         matches = re.findall(r'PrgEnv-\w+/(\d+).\d+.\d+', output)
         major_versions = set(matches)
         latest_version = max(major_versions)
@@ -58,10 +57,7 @@ def find_compiler(self, cmp_cls, *paths):
             if not cmp_cls.PrgEnv_compiler:
                 tty.die('Must supply PrgEnv_compiler with PrgEnv')
 
-            modulecmd = get_module_cmd()
-
-            output = modulecmd(
-                'avail', cmp_cls.PrgEnv_compiler, output=str, error=str)
+            output = module('avail', cmp_cls.PrgEnv_compiler)
             version_regex = r'(%s)/([\d\.]+[\d])' % cmp_cls.PrgEnv_compiler
             matches = re.findall(version_regex, output)
             for name, version in matches:

lib/spack/spack/operating_systems/cray_frontend.py

@@ -6,7 +6,7 @@
 import os
 
 from spack.operating_systems.linux_distro import LinuxDistro
-from spack.util.module_cmd import get_module_cmd
+from spack.util.module_cmd import module
 
 
 class CrayFrontend(LinuxDistro):
@@ -41,10 +41,7 @@ def find_compilers(self, *paths):
             # into the PATH environment variable (i.e. the following modules:
             # 'intel', 'cce', 'gcc', etc.) will also be unloaded since they are
             # specified as prerequisites in the PrgEnv-* modulefiles.
-            modulecmd = get_module_cmd()
-            exec(compile(
-                modulecmd('unload', prg_env, output=str, error=os.devnull),
-                '<string>', 'exec'))
+            module('unload', prg_env)
 
         # Call the overridden method.
         clist = super(CrayFrontend, self).find_compilers(*paths)

lib/spack/spack/platforms/cray.py

@@ -11,7 +11,7 @@
 from spack.architecture import Platform, Target, NoPlatformError
 from spack.operating_systems.cray_frontend import CrayFrontend
 from spack.operating_systems.cnl import Cnl
-from spack.util.module_cmd import get_module_cmd, unload_module
+from spack.util.module_cmd import module
 
 
 def _get_modules_in_modulecmd_output(output):
@@ -90,8 +90,8 @@ def setup_platform_environment(cls, pkg, env):
         # Unload these modules to prevent any silent linking or unnecessary
         # I/O profiling in the case of darshan.
         modules_to_unload = ["cray-mpich", "darshan", "cray-libsci", "altd"]
-        for module in modules_to_unload:
-            unload_module(module)
+        for mod in modules_to_unload:
+            module('unload', mod)
 
         env.set('CRAYPE_LINK_TYPE', 'dynamic')
         cray_wrapper_names = os.path.join(build_env_path, 'cray')
@@ -127,8 +127,7 @@ def _default_target_from_env(self):
     def _avail_targets(self):
         '''Return a list of available CrayPE CPU targets.'''
         if getattr(self, '_craype_targets', None) is None:
-            module = get_module_cmd()
-            output = module('avail', '-t', 'craype-', output=str, error=str)
+            output = module('avail', '-t', 'craype-')
             craype_modules = _get_modules_in_modulecmd_output(output)
             self._craype_targets = targets = []
             _fill_craype_targets_from_modules(targets, craype_modules)

lib/spack/spack/test/build_environment.py

@@ -205,7 +205,7 @@ def _set_wrong_cc(x):
     assert paths.index(spack_path) < paths.index(module_path)
 
 
-def test_package_inheritance_module_setup(config, mock_packages):
+def test_package_inheritance_module_setup(config, mock_packages, working_env):
     s = spack.spec.Spec('multimodule-inheritance')
     s.concretize()
     pkg = s.package
@@ -217,8 +217,6 @@ def test_package_inheritance_module_setup(config, mock_packages):
     assert pkg.use_module_variable() == 'test_module_variable'
     assert os.environ['TEST_MODULE_VAR'] == 'test_module_variable'
 
-    os.environ.pop('TEST_MODULE_VAR')
-
 
 def test_set_build_environment_variables(
         config, mock_packages, working_env, monkeypatch,

lib/spack/spack/test/conftest.py

@@ -142,7 +142,11 @@ def remove_whatever_it_is(path):
 def working_env():
     saved_env = os.environ.copy()
     yield
-    os.environ = saved_env
+    # os.environ = saved_env doesn't work
+    # it causes module_parsing::test_module_function to fail
+    # when it's run after any test using this fixutre
+    os.environ.clear()
+    os.environ.update(saved_env)
 
 
 @pytest.fixture(scope='function', autouse=True)

lib/spack/spack/test/flag_handlers.py

@@ -17,7 +17,8 @@
 def temp_env():
     old_env = os.environ.copy()
     yield
-    os.environ = old_env
+    os.environ.clear()
+    os.environ.update(old_env)
 
 
 def add_o3_to_build_system_cflags(pkg, name, flags):

lib/spack/spack/test/module_parsing.py

@@ -4,60 +4,75 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import pytest
-import subprocess
 import os
+import spack
+
 from spack.util.module_cmd import (
+    module,
     get_path_from_module,
-    get_path_from_module_contents,
     get_path_arg_from_module_line,
-    get_module_cmd_from_bash,
-    get_module_cmd,
-    ModuleError)
+    get_path_from_module_contents
+)
+
+test_module_lines = ['prepend-path LD_LIBRARY_PATH /path/to/lib',
+                     'setenv MOD_DIR /path/to',
+                     'setenv LDFLAGS -Wl,-rpath/path/to/lib',
+                     'setenv LDFLAGS -L/path/to/lib',
+                     'prepend-path PATH /path/to/bin']
 
 
-env = os.environ.copy()
-env['LC_ALL'] = 'C'
-typeset_func = subprocess.Popen('module avail',
-                                env=env,
-                                stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE,
-                                shell=True)
-typeset_func.wait()
-typeset = typeset_func.stderr.read()
-MODULE_NOT_DEFINED = b'not found' in typeset
+@pytest.fixture
+def module_function_test_mode():
+    old_mode = spack.util.module_cmd._test_mode
+    spack.util.module_cmd._test_mode = True
+
+    yield
+
+    spack.util.module_cmd._test_mode = old_mode
 
 
 @pytest.fixture
-def save_env():
-    old_path = os.environ.get('PATH', None)
-    old_bash_func = os.environ.get('BASH_FUNC_module()', None)
+def save_module_func():
+    old_func = spack.util.module_cmd.module
 
     yield
 
-    if old_path:
-        os.environ['PATH'] = old_path
-    if old_bash_func:
-        os.environ['BASH_FUNC_module()'] = old_bash_func
+    spack.util.module_cmd.module = old_func
 
 
-def test_get_path_from_module(save_env):
-    lines = ['prepend-path LD_LIBRARY_PATH /path/to/lib',
-             'prepend-path CRAY_LD_LIBRARY_PATH /path/to/lib',
-             'setenv MOD_DIR /path/to',
-             'setenv LDFLAGS -Wl,-rpath/path/to/lib',
-             'setenv LDFLAGS -L/path/to/lib',
-             'prepend-path PATH /path/to/bin']
+def test_module_function_change_env(tmpdir, working_env,
+                                    module_function_test_mode):
+    src_file = str(tmpdir.join('src_me'))
+    with open(src_file, 'w') as f:
+        f.write('export TEST_MODULE_ENV_VAR=TEST_SUCCESS\n')
 
-    for line in lines:
-        module_func = '() { eval `echo ' + line + ' bash filler`\n}'
-        os.environ['BASH_FUNC_module()'] = module_func
-        path = get_path_from_module('mod')
-        assert path == '/path/to'
+    os.environ['NOT_AFFECTED'] = "NOT_AFFECTED"
+    module('load', src_file)
+
+    assert os.environ['TEST_MODULE_ENV_VAR'] == 'TEST_SUCCESS'
+    assert os.environ['NOT_AFFECTED'] == "NOT_AFFECTED"
 
-    os.environ['BASH_FUNC_module()'] = '() { eval $(echo fill bash $*)\n}'
-    path = get_path_from_module('mod')
 
-    assert path is None
+def test_module_function_no_change(tmpdir, module_function_test_mode):
+    src_file = str(tmpdir.join('src_me'))
+    with open(src_file, 'w') as f:
+        f.write('echo TEST_MODULE_FUNCTION_PRINT')
+
+    old_env = os.environ.copy()
+    text = module('show', src_file)
+
+    assert text == 'TEST_MODULE_FUNCTION_PRINT\n'
+    assert os.environ == old_env
+
+
+def test_get_path_from_module_faked(save_module_func):
+    for line in test_module_lines:
+        def fake_module(*args):
+            return line
+        spack.util.module_cmd.module = fake_module
+
+        path = get_path_from_module('mod')
+        assert path == '/path/to'
 
 
 def test_get_path_from_module_contents():
@@ -106,62 +121,3 @@ def test_get_argument_from_module_line():
     for bl in bad_lines:
         with pytest.raises(ValueError):
             get_path_arg_from_module_line(bl)
-
-
-@pytest.mark.skipif(MODULE_NOT_DEFINED, reason='Depends on defined module fn')
-def test_get_module_cmd_from_bash_using_modules():
-    module_list_proc = subprocess.Popen(['module list'],
-                                        stdout=subprocess.PIPE,
-                                        stderr=subprocess.STDOUT,
-                                        executable='/bin/bash',
-                                        shell=True)
-    module_list_proc.wait()
-    module_list = module_list_proc.stdout.read()
-
-    module_cmd = get_module_cmd_from_bash()
-    module_cmd_list = module_cmd('list', output=str, error=str)
-
-    # Lmod command reprints some env variables on every invocation.
-    # Test containment to avoid false failures on lmod systems.
-    assert module_list in module_cmd_list
-
-
-def test_get_module_cmd_from_bash_ticks(save_env):
-    os.environ['BASH_FUNC_module()'] = '() { eval `echo bash $*`\n}'
-
-    module_cmd = get_module_cmd()
-    module_cmd_list = module_cmd('list', output=str, error=str)
-
-    assert module_cmd_list == 'python list\n'
-
-
-def test_get_module_cmd_from_bash_parens(save_env):
-    os.environ['BASH_FUNC_module()'] = '() { eval $(echo fill sh $*)\n}'
-
-    module_cmd = get_module_cmd()
-    module_cmd_list = module_cmd('list', output=str, error=str)
-
-    assert module_cmd_list == 'fill python list\n'
-
-
-def test_get_module_cmd_fails(save_env):
-    os.environ.pop('BASH_FUNC_module()')
-    os.environ.pop('PATH')
-    with pytest.raises(ModuleError):
-        module_cmd = get_module_cmd(b'--norc')
-        module_cmd()  # Here to avoid Flake F841 on previous line
-
-
-def test_get_module_cmd_from_which(tmpdir, save_env):
-    f = tmpdir.mkdir('bin').join('modulecmd')
-    f.write('#!/bin/bash\n'
-            'echo $*')
-    f.chmod(0o770)
-
-    os.environ['PATH'] = str(tmpdir.join('bin')) + ':' + os.environ['PATH']
-    os.environ.pop('BASH_FUNC_module()')
-
-    module_cmd = get_module_cmd(b'--norc')
-    module_cmd_list = module_cmd('list', output=str, error=str)
-
-    assert module_cmd_list == 'python list\n'