From d9fbdfbee930c9bae5c4c007bfb6a844ffdb0744 Mon Sep 17 00:00:00 2001 From: Xavier Delaruelle Date: Wed, 19 Jul 2023 17:57:37 +0200 Subject: 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 | 6 ++ lib/spack/spack/test/modules/lmod.py | 12 ++++ lib/spack/spack/test/modules/tcl.py | 80 +++++++++++++--------- share/spack/templates/modules/modulefile.tcl | 22 +++--- .../packages/module-long-help/package.py | 19 +++++ 5 files changed, 95 insertions(+), 44 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/module-long-help/package.py diff --git a/lib/spack/spack/tengine.py b/lib/spack/spack/tengine.py index 6a77e6becf..6f2e3fb699 100644 --- a/lib/spack/spack/tengine.py +++ b/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 diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py index 6e4222f7cd..dba40130b5 100644 --- a/lib/spack/spack/test/modules/lmod.py +++ b/lib/spack/spack/test/modules/lmod.py @@ -232,6 +232,18 @@ class TestLmod: ) 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") diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index f255bc42b8..c3157daea5 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -29,7 +29,7 @@ class TestTcl: 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 @@ class TestTcl: 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 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 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]) == 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 @@ class TestTcl: 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 - 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 "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 + 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 @pytest.mark.regression("11355") def test_manpath_setup(self, modulefile_content, module_configuration): @@ -150,12 +150,12 @@ class TestTcl: # 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 @@ class TestTcl: [ 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 'append-path --delim ":" MANPATH ""' in x]) == 0 + 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 @pytest.mark.regression("29578") def test_setenv_raw_value(self, modulefile_content, module_configuration): @@ -189,7 +189,7 @@ class TestTcl: 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 @@ class TestTcl: help_msg = ( "proc ModulesHelp { } {" - ' puts stderr "Name : mpileaks"' - ' puts stderr "Version: 2.3"' - ' puts stderr "Target : core2"' - ' puts stderr ""' - ' puts stderr "Mpileaks is a mock package that passes audits"' + " puts stderr {Name : mpileaks}" + " puts stderr {Version: 2.3}" + " puts stderr {Target : core2}" + " puts stderr {}" + " puts stderr {Mpileaks is a mock package that passes audits}" "}" ) assert help_msg in "".join(content) @@ -212,9 +212,23 @@ class TestTcl: help_msg = ( "proc ModulesHelp { } {" - ' puts stderr "Name : libdwarf"' - ' puts stderr "Version: 20130729"' - ' puts stderr "Target : core2"' + " puts stderr {Name : libdwarf}" + " puts stderr {Version: 20130729}" + " 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 @@ class TestTcl: 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 @@ class TestTcl: 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") diff --git a/share/spack/templates/modules/modulefile.tcl b/share/spack/templates/modules/modulefile.tcl index 52d987da61..746fea2f31 100644 --- a/share/spack/templates/modules/modulefile.tcl +++ b/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 "Version: {{ spec.version }}" - puts stderr "Target : {{ spec.target }}" + puts stderr {{ '{' }}Name : {{ spec.name }}{{ '}' }} + puts stderr {{ '{' }}Version: {{ spec.version }}{{ '}' }} + puts stderr {{ '{' }}Target : {{ spec.target }}{{ '}' }} {% if long_description %} - puts stderr "" -{{ long_description| textwrap(72)| quote()| prepend_to_line(' puts stderr ')| join() }} + puts stderr {} +{{ 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 %} diff --git a/var/spack/repos/builtin.mock/packages/module-long-help/package.py b/var/spack/repos/builtin.mock/packages/module-long-help/package.py new file mode 100644 index 0000000000..43a0da412a --- /dev/null +++ b/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") -- cgit v1.2.3-70-g09d2