modules: use curly braces to enclose value in Tcl modulefile (#38375)

Use curly braces instead of quotes to enclose value or text in Tcl
modulefile. Within curly braces Tcl special characters like [, ] or $
are treated verbatim whereas they are evaluated within quotes.

Curly braces is Tcl recommended way to enclose verbatim content [1].

Note: if curly braces charaters are used within content, they must be
balanced. This point has been checked against current repository and no
unbalanced curly braces has been spotted.

Fixes #24243

[1] https://wiki.tcl-lang.org/page/Tcl+Minimal+Escaping+Style

lib/spack/spack/tengine.py

@@ -100,9 +100,15 @@ def quote(text):
     return ['"{0}"'.format(line) for line in text]
 
 
+def curly_quote(text):
+    """Encloses each line of text in curly braces"""
+    return ["{{{0}}}".format(line) for line in text]
+
+
 def _set_filters(env):
     """Sets custom filters to the template engine environment"""
     env.filters["textwrap"] = textwrap.wrap
     env.filters["prepend_to_line"] = prepend_to_line
     env.filters["join"] = "\n".join
     env.filters["quote"] = quote
+    env.filters["curly_quote"] = curly_quote

lib/spack/spack/test/modules/lmod.py

@@ -232,6 +232,18 @@ def test_help_message(self, modulefile_content, module_configuration):
         )
         assert help_msg in "".join(content)
 
+        content = modulefile_content("module-long-help target=core2")
+
+        help_msg = (
+            "help([[Name   : module-long-help]])"
+            "help([[Version: 1.0]])"
+            "help([[Target : core2]])"
+            "help()"
+            "help([[Package to test long description message generated in modulefile."
+            "Message too long is wrapped over multiple lines.]])"
+        )
+        assert help_msg in "".join(content)
+
     def test_exclude(self, modulefile_content, module_configuration):
         """Tests excluding the generation of selected modules."""
         module_configuration("exclude")

lib/spack/spack/test/modules/tcl.py

@@ -29,7 +29,7 @@ def test_simple_case(self, modulefile_content, module_configuration):
         module_configuration("autoload_direct")
         content = modulefile_content(mpich_spec_string)
 
-        assert 'module-whatis "mpich @3.0.4"' in content
+        assert "module-whatis {mpich @3.0.4}" in content
 
     def test_autoload_direct(self, modulefile_content, module_configuration):
         """Tests the automatic loading of direct dependencies."""
@@ -112,16 +112,16 @@ def test_alter_environment(self, modulefile_content, module_configuration):
         content = modulefile_content("mpileaks platform=test target=x86_64")
 
         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]) == 1
+        assert len([x for x in content if "setenv FOO {foo}" in x]) == 1
-        assert len([x for x in content if 'setenv OMPI_MCA_mpi_leave_pinned "1"' in x]) == 1
+        assert len([x for x in content if "setenv OMPI_MCA_mpi_leave_pinned {1}" in x]) == 1
-        assert len([x for x in content if 'setenv OMPI_MCA_MPI_LEAVE_PINNED "1"' in x]) == 0
+        assert len([x for x in content if "setenv OMPI_MCA_MPI_LEAVE_PINNED {1}" in x]) == 0
         assert len([x for x in content if "unsetenv BAR" in x]) == 1
         assert len([x for x in content if "setenv MPILEAKS_ROOT" in x]) == 1
 
         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 'setenv FOO "foo"' in x]) == 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
         assert len([x for x in content if "depends-on foo/bar" in x]) == 1
         assert len([x for x in content if "module load foo/bar" in x]) == 1
@@ -133,14 +133,14 @@ def test_prepend_path_separator(self, modulefile_content, module_configuration):
         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 "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 "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 "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 "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 "prepend-path --delim {;} SEMICOLON {bar}" in x]) == 1
-        assert len([x for x in content if 'remove-path --delim ";" SEMICOLON "bar"' in x]) == 1
+        assert len([x for x in content if "remove-path --delim {;} SEMICOLON {bar}" in x]) == 1
-        assert len([x for x in content if 'append-path --delim " " SPACE "qux"' in x]) == 1
+        assert len([x for x in content if "append-path --delim { } SPACE {qux}" in x]) == 1
-        assert len([x for x in content if 'remove-path --delim " " SPACE "qux"' in x]) == 1
+        assert len([x for x in content if "remove-path --delim { } SPACE {qux}" in x]) == 1
 
     @pytest.mark.regression("11355")
     def test_manpath_setup(self, modulefile_content, module_configuration):
@@ -150,12 +150,12 @@ def test_manpath_setup(self, modulefile_content, module_configuration):
 
         # no manpath set by module
         content = modulefile_content("mpileaks")
-        assert len([x for x in content if 'append-path --delim ":" MANPATH ""' in x]) == 0
+        assert len([x for x in content if "append-path --delim {:} MANPATH {}" in x]) == 0
 
         # manpath set by module with prepend-path
         content = modulefile_content("module-manpath-prepend")
         assert (
-            len([x for x in content if 'prepend-path --delim ":" MANPATH "/path/to/man"' in x])
+            len([x for x in content if "prepend-path --delim {:} MANPATH {/path/to/man}" in x])
             == 1
         )
         assert (
@@ -163,24 +163,24 @@ def test_manpath_setup(self, modulefile_content, module_configuration):
                 [
                     x
                     for x in content
-                    if 'prepend-path --delim ":" MANPATH "/path/to/share/man"' in x
+                    if "prepend-path --delim {:} MANPATH {/path/to/share/man}" in x
                 ]
             )
             == 1
         )
-        assert len([x for x in content if 'append-path --delim ":" MANPATH ""' in x]) == 1
+        assert len([x for x in content if "append-path --delim {:} MANPATH {}" in x]) == 1
 
         # manpath set by module with append-path
         content = modulefile_content("module-manpath-append")
         assert (
-            len([x for x in content if 'append-path --delim ":" MANPATH "/path/to/man"' in x]) == 1
+            len([x for x in content if "append-path --delim {:} MANPATH {/path/to/man}" in x]) == 1
         )
-        assert len([x for x in content if 'append-path --delim ":" MANPATH ""' in x]) == 1
+        assert len([x for x in content if "append-path --delim {:} MANPATH {}" in x]) == 1
 
         # manpath set by module with setenv
         content = modulefile_content("module-manpath-setenv")
-        assert len([x for x in content if 'setenv MANPATH "/path/to/man"' in x]) == 1
+        assert len([x for x in content if "setenv MANPATH {/path/to/man}" in x]) == 1
-        assert len([x for x in content if 'append-path --delim ":" MANPATH ""' in x]) == 0
+        assert len([x for x in content if "append-path --delim {:} MANPATH {}" in x]) == 0
 
     @pytest.mark.regression("29578")
     def test_setenv_raw_value(self, modulefile_content, module_configuration):
@@ -189,7 +189,7 @@ def test_setenv_raw_value(self, modulefile_content, module_configuration):
         module_configuration("autoload_direct")
         content = modulefile_content("module-setenv-raw")
 
-        assert len([x for x in content if 'setenv FOO "{{name}}, {name}, {{}}, {}"' in x]) == 1
+        assert len([x for x in content if "setenv FOO {{{name}}, {name}, {{}}, {}}" in x]) == 1
 
     def test_help_message(self, modulefile_content, module_configuration):
         """Tests the generation of module help message."""
@@ -199,11 +199,11 @@ def test_help_message(self, modulefile_content, module_configuration):
 
         help_msg = (
             "proc ModulesHelp { } {"
-            '    puts stderr "Name   : mpileaks"'
+            "    puts stderr {Name   : mpileaks}"
-            '    puts stderr "Version: 2.3"'
+            "    puts stderr {Version: 2.3}"
-            '    puts stderr "Target : core2"'
+            "    puts stderr {Target : core2}"
-            '    puts stderr ""'
+            "    puts stderr {}"
-            '    puts stderr "Mpileaks is a mock package that passes audits"'
+            "    puts stderr {Mpileaks is a mock package that passes audits}"
             "}"
         )
         assert help_msg in "".join(content)
@@ -212,9 +212,23 @@ def test_help_message(self, modulefile_content, module_configuration):
 
         help_msg = (
             "proc ModulesHelp { } {"
-            '    puts stderr "Name   : libdwarf"'
+            "    puts stderr {Name   : libdwarf}"
-            '    puts stderr "Version: 20130729"'
+            "    puts stderr {Version: 20130729}"
-            '    puts stderr "Target : core2"'
+            "    puts stderr {Target : core2}"
+            "}"
+        )
+        assert help_msg in "".join(content)
+
+        content = modulefile_content("module-long-help target=core2")
+
+        help_msg = (
+            "proc ModulesHelp { } {"
+            "    puts stderr {Name   : module-long-help}"
+            "    puts stderr {Version: 1.0}"
+            "    puts stderr {Target : core2}"
+            "    puts stderr {}"
+            "    puts stderr {Package to test long description message generated in modulefile.}"
+            "    puts stderr {Message too long is wrapped over multiple lines.}"
             "}"
         )
         assert help_msg in "".join(content)
@@ -372,14 +386,14 @@ def test_setup_environment(self, modulefile_content, module_configuration):
         content = modulefile_content("mpileaks")
 
         assert len([x for x in content if "setenv FOOBAR" in x]) == 1
-        assert len([x for x in content if 'setenv FOOBAR "mpileaks"' in x]) == 1
+        assert len([x for x in content if "setenv FOOBAR {mpileaks}" in x]) == 1
 
         spec = spack.spec.Spec("mpileaks")
         spec.concretize()
         content = modulefile_content(str(spec["callpath"]))
 
         assert len([x for x in content if "setenv FOOBAR" in x]) == 1
-        assert len([x for x in content if 'setenv FOOBAR "callpath"' in x]) == 1
+        assert len([x for x in content if "setenv FOOBAR {callpath}" in x]) == 1
 
     def test_override_config(self, module_configuration, factory):
         """Tests overriding some sections of the configuration file."""
@@ -420,7 +434,7 @@ def test_extend_context(self, modulefile_content, module_configuration):
 
         assert 'puts stderr "sentence from package"' in content
 
-        short_description = 'module-whatis "This package updates the context for Tcl modulefiles."'
+        short_description = "module-whatis {This package updates the context for Tcl modulefiles.}"
         assert short_description in content
 
     @pytest.mark.regression("4400")

share/spack/templates/modules/modulefile.tcl

@@ -11,16 +11,16 @@
 
 {% block header %}
 {% if short_description %}
-module-whatis "{{ short_description }}"
+module-whatis {{ '{' }}{{ short_description }}{{ '}' }}
 {% endif %}
 
 proc ModulesHelp { } {
-    puts stderr "Name   : {{ spec.name }}"
+    puts stderr {{ '{' }}Name   : {{ spec.name }}{{ '}' }}
-    puts stderr "Version: {{ spec.version }}"
+    puts stderr {{ '{' }}Version: {{ spec.version }}{{ '}' }}
-    puts stderr "Target : {{ spec.target }}"
+    puts stderr {{ '{' }}Target : {{ spec.target }}{{ '}' }}
 {% if long_description %}
-    puts stderr ""
+    puts stderr {}
-{{ long_description| textwrap(72)| quote()| prepend_to_line('    puts stderr ')| join() }}
+{{ long_description| textwrap(72)| curly_quote()| prepend_to_line('    puts stderr ')| join() }}
 {% endif %}
 }
 {% endblock %}
@@ -54,13 +54,13 @@ conflict {{ name }}
 {% block environment %}
 {% for command_name, cmd in environment_modifications %}
 {% if command_name == 'PrependPath' %}
-prepend-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
+prepend-path --delim {{ '{' }}{{ cmd.separator }}{{ '}' }} {{ cmd.name }} {{ '{' }}{{ cmd.value }}{{ '}' }}
 {% elif command_name in ('AppendPath', 'AppendFlagsEnv') %}
-append-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
+append-path --delim {{ '{' }}{{ cmd.separator }}{{ '}' }} {{ cmd.name }} {{ '{' }}{{ cmd.value }}{{ '}' }}
 {% elif command_name in ('RemovePath', 'RemoveFlagsEnv') %}
-remove-path --delim "{{ cmd.separator }}" {{ cmd.name }} "{{ cmd.value }}"
+remove-path --delim {{ '{' }}{{ cmd.separator }}{{ '}' }} {{ cmd.name }} {{ '{' }}{{ cmd.value }}{{ '}' }}
 {% elif command_name == 'SetEnv' %}
-setenv {{ cmd.name }} "{{ cmd.value }}"
+setenv {{ cmd.name }} {{ '{' }}{{ cmd.value }}{{ '}' }}
 {% elif command_name == 'UnsetEnv' %}
 unsetenv {{ cmd.name }}
 {% endif %}
@@ -68,7 +68,7 @@ unsetenv {{ cmd.name }}
 {% endfor %}
 {# Make sure system man pages are enabled by appending trailing delimiter to MANPATH #}
 {% if has_manpath_modifications %}
-append-path --delim ":" MANPATH ""
+append-path --delim {{ '{' }}:{{ '}' }} MANPATH {{ '{' }}{{ '}' }}
 {% endif %}
 {% endblock %}
 

var/spack/repos/builtin.mock/packages/module-long-help/package.py

@@ -0,0 +1,19 @@
+# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class ModuleLongHelp(Package):
+    """Package to test long description message generated in modulefile.
+    Message too long is wrapped over multiple lines."""
+
+    homepage = "http://www.llnl.gov"
+    url = "http://www.llnl.gov/module-long-help-1.0.tar.gz"
+
+    version("1.0", "0123456789abcdef0123456789abcdef")
+
+    def setup_run_environment(self, env):
+        env.set("FOO", "bar")