modules tcl: simplify env modification block in template (#36334)

Simplify environment modification block in modulefile Tcl template by
always setting a path delimiter to the prepend-path, append-path and
remove-path commands.

Remove --delim option to the setenv command as this command does not
allow such option.

Update test_prepend_path_separator test to explicitly check the 6
path-like commands that should be present in generated modulefile.

4 files changed, 27 insertions, 24 deletions

diff --git a/lib/spack/spack/test/data/modules/tcl/module_path_separator.yaml b/lib/spack/spack/test/data/modules/tcl/module_path_separator.yaml
new file mode 100644
index 0000000000..683a2ffffd
--- /dev/null
+++ b/lib/spack/spack/test/data/modules/tcl/module_path_separator.yaml
@@ -0,0 +1,5 @@
+enable:
+  - tcl
+tcl:
+  all:
+    autoload: none
diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py
index 6e172acbf2..f002285ce0 100644
--- a/lib/spack/spack/test/modules/lmod.py
+++ b/lib/spack/spack/test/modules/lmod.py
@@ -2,7 +2,7 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import re
+
 import sys
 
 import pytest
@@ -128,21 +128,22 @@ class TestLmod(object):
 
         content = modulefile_content("libdwarf platform=test target=core2")
 
-        assert len([x for x in content if x.startswith('prepend-path("CMAKE_PREFIX_PATH"')]) == 0
+        assert len([x for x in content if x.startswith('prepend_path("CMAKE_PREFIX_PATH"')]) == 0
         assert len([x for x in content if 'setenv("FOO", "foo")' in x]) == 0
         assert len([x for x in content if 'unsetenv("BAR")' in x]) == 0
 
     def test_prepend_path_separator(self, modulefile_content, module_configuration):
-        """Tests modifications to run-time environment."""
+        """Tests that we can use custom delimiters to manipulate path lists."""
 
         module_configuration("module_path_separator")
         content = modulefile_content("module-path-separator")
 
-        for line in content:
-            if re.match(r'[a-z]+_path\("COLON"', line):
-                assert line.endswith('"foo", ":")')
-            elif re.match(r'[a-z]+_path\("SEMICOLON"', line):
-                assert line.endswith('"bar", ";")')
+        assert len([x for x in content if 'append_path("COLON", "foo", ":")' in x]) == 1
+        assert len([x for x in content if 'prepend_path("COLON", "foo", ":")' in x]) == 1
+        assert len([x for x in content if 'remove_path("COLON", "foo", ":")' in x]) == 1
+        assert len([x for x in content if 'append_path("SEMICOLON", "bar", ";")' in x]) == 1
+        assert len([x for x in content if 'prepend_path("SEMICOLON", "bar", ";")' in x]) == 1
+        assert len([x for x in content if 'remove_path("SEMICOLON", "bar", ";")' in x]) == 1
 
     @pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
     def test_exclude(self, modulefile_content, module_configuration, config_name):
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index 95c241f4a1..f289e455e5 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -108,6 +108,19 @@ class TestTcl(object):
         assert len([x for x in content if "module load foo/bar" in x]) == 1
         assert len([x for x in content if "setenv LIBDWARF_ROOT" in x]) == 1
 
+    def test_prepend_path_separator(self, modulefile_content, module_configuration):
+        """Tests that we can use custom delimiters to manipulate path lists."""
+
+        module_configuration("module_path_separator")
+        content = modulefile_content("module-path-separator")
+
+        assert len([x for x in content if 'append-path --delim ":" COLON "foo"' in x]) == 1
+        assert len([x for x in content if 'prepend-path --delim ":" COLON "foo"' in x]) == 1
+        assert len([x for x in content if 'remove-path --delim ":" COLON "foo"' in x]) == 1
+        assert len([x for x in content if 'append-path --delim ";" SEMICOLON "bar"' in x]) == 1
+        assert len([x for x in content if 'prepend-path --delim ";" SEMICOLON "bar"' in x]) == 1
+        assert len([x for x in content if 'remove-path --delim ";" SEMICOLON "bar"' in x]) == 1
+
     @pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
     def test_exclude(self, modulefile_content, module_configuration, config_name):
         """Tests excluding the generation of selected modules."""
diff --git a/share/spack/templates/modules/modulefile.tcl b/share/spack/templates/modules/modulefile.tcl
index de8d9e29bb..be53856ba7 100644
--- a/share/spack/templates/modules/modulefile.tcl
+++ b/share/spack/templates/modules/modulefile.tcl
@@ -41,8 +41,6 @@ conflict {{ name }}
 
 {% block environment %}
 {% for command_name, cmd in environment_modifications %}
-{% if cmd.separator != ':' %}
-{# A non-standard separator is required #}
 {% if command_name == 'PrependPath' %}
 prepend-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
 {% elif command_name == 'AppendPath' %}
@@ -50,25 +48,11 @@ append-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
 {% elif command_name == 'RemovePath' %}
 remove-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
 {% elif command_name == 'SetEnv' %}
-setenv --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
-{% elif command_name == 'UnsetEnv' %}
-unsetenv {{ cmd.name }}
-{% endif %}
-{% else %}
-{# We are using the usual separator #}
-{% if command_name == 'PrependPath' %}
-prepend-path {{ cmd.name }} "{{ cmd.value }}"
-{% elif command_name == 'AppendPath' %}
-append-path {{ cmd.name }} "{{ cmd.value }}"
-{% elif command_name == 'RemovePath' %}
-remove-path {{ cmd.name }} "{{ cmd.value }}"
-{% elif command_name == 'SetEnv' %}
 setenv {{ cmd.name }} "{{ cmd.value }}"
 {% elif command_name == 'UnsetEnv' %}
 unsetenv {{ cmd.name }}
 {% endif %}
 {#  #}
-{% endif %}
 {% endfor %}
 {% endblock %}