Refresh modules that put content to stdout/prestdout

Mark modulefile qualified for refresh evaluation when puts command is used to send content to stdout or prestdout channels. Such loaded modules where wrongly skipped during refresh unless if they declared another volatile environment change. Choice is made to also refresh modules that put content to stdout/prestdout to keep the behavior of Modules 3.2 Fixes #488
cea-hpc · May 6, 2023 · 4782306 · 4782306
1 parent b55c2fe
commit 4782306
Show file tree

Hide file tree

Showing 13 changed files with 52 additions and 12 deletions.
diff --git a/NEWS.rst b/NEWS.rst
@@ -121,6 +121,9 @@ Modules 5.3.0 (not yet released)
 * Requirement and incompatibility extra specifiers accept module specification
   as value.
 * Doc: add :ref:`Extra specifier` description in :ref:`module(1)` man page.
+* Mark loaded modules as qualified for refresh evaluation when they send
+  content to ``stdout`` or ``prestdout`` channels with :mfcmd:`puts`
+  modulefile command. (fix issue #488)
 
 
 .. _5.2 release notes:

diff --git a/doc/source/changes.rst b/doc/source/changes.rst
@@ -639,9 +639,9 @@ Module Sub-Commands
 :subcmd:`refresh`
 
  Sub-command evaluates all loaded modulefiles and re-apply the non-persistent
- environment changes they define (i.e., shell aliases and functions). With
- this change the :subcmd:`refresh` sub-command is restored to the behavior it
- had on Modules version 3.2.
+ environment changes they define (i.e., shell aliases, shell functions and
+ command put on stdout channel). With this change the :subcmd:`refresh`
+ sub-command is restored to the behavior it had on Modules version 3.2.
 
 :subcmd:`restore`, :subcmd:`source`
 

diff --git a/doc/source/module.rst b/doc/source/module.rst
@@ -1808,11 +1808,11 @@ Module Sub-Commands
  have already been set by the currently loaded modules.
 
  Loaded modules are evaluated in ``refresh`` mode following their load order.
- In this evaluation mode only the :mfcmd:`complete`, :mfcmd:`set-alias` and
- :mfcmd:`set-function` modulefile commands will produce environment changes.
- Other modulefile commands that produce environment changes (like
- :mfcmd:`setenv` or :mfcmd:`append-path`) are ignored during a ``refresh``
- evaluation as their changes should already be applied.
+ In this evaluation mode only the :mfcmd:`complete`, :mfcmd:`set-alias`,
+ :mfcmd:`set-function` and :mfcmd:`puts` modulefile commands will produce
+ environment changes. Other modulefile commands that produce environment
+ changes (like :mfcmd:`setenv` or :mfcmd:`append-path`) are ignored during a
+ ``refresh`` evaluation as their changes should already be applied.
 
  Only the loaded modules defining non-persistent environment changes are
  evaluated in ``refresh`` mode. Such loaded modules are listed in the

diff --git a/tcl/mfinterp.tcl.in b/tcl/mfinterp.tcl.in
@@ -1596,8 +1596,11 @@ proc putsModfileCmd {itrp args} {
       knerror {wrong # args: should be "puts ?-nonewline? ?channelId? string"}
    # defer puts if it targets stdout (see renderSettings)
    } elseif {[info exists deferPuts]} {
+      # current module is qualified for refresh evaluation
+      lappendState -nodup refresh_qualified [currentState modulename]
       lappend ::g_stdoutPuts {*}$deferPuts
    } elseif {[info exists deferPrePuts]} {
+      lappendState -nodup refresh_qualified [currentState modulename]
       lappend ::g_prestdoutPuts {*}$deferPrePuts
    # if it targets stderr call report, which knows what channel to use
    } elseif {[info exists reportArgs]} {

diff --git a/testsuite/modulefiles.3/refresh/1.0 b/testsuite/modulefiles.3/refresh/1.0
@@ -55,5 +55,6 @@ set-alias almode "echo [module-info mode]"
 
 if {[module-info mode] in {load refresh}} {
     # puts to the result channel should be effective in refresh mode
+    puts prestdout "echo pre [module-info name]"
     puts "echo [module-info name]"
 }
diff --git a/testsuite/modulefiles.3/refresh/2.0 b/testsuite/modulefiles.3/refresh/2.0
@@ -28,5 +28,6 @@ if {[info exists env(TESTSUITE_REFRESH)]} {
 }
 
 if {[module-info mode] in {load refresh}} {
+    puts prestdout "echo pre [module-info name]"
     puts "echo [module-info name]"
 }
diff --git a/testsuite/modules.50-cmds/078-refresh.exp b/testsuite/modules.50-cmds/078-refresh.exp
@@ -92,6 +92,7 @@ setenv_var FOO value
 setenv_path_var BAR value othervalue
 
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
 lappend ans [list alias altags {echo loaded}]
 lappend ans [list alias alspec {echo refresh/1.0}]
 lappend ans [list alias alcmd {echo refresh}]
@@ -107,6 +108,7 @@ testouterr_cmd sh {refresh} $ans {}
 setenv_var __MODULES_LMTAG refresh/1.0&bar
 
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
 lappend ans [list alias altags {echo bar loaded}]
 lappend ans [list alias alspec {echo refresh/1.0}]
 lappend ans [list alias alcmd {echo refresh}]
@@ -126,6 +128,7 @@ setenv_var MODULES_ADVANCED_VERSION_SPEC 1
 setenv_var __MODULES_LMVARIANT refresh/2.0&foo|1|1|0
 
 set ans [list]
+lappend ans [list out {echo pre refresh/2.0}]
 lappend ans [list alias altags {echo bar loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alfoo {echo foo2}]
@@ -139,6 +142,8 @@ setenv_var __MODULES_LMREFRESH refresh/1.0:refresh/2.0
 setenv_var __MODULES_LMTAG refresh/2.0&bar
 
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
+lappend ans [list out {echo pre refresh/2.0}]
 lappend ans [list alias altags {echo bar loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alisloaded2 {echo is-loaded ok}]
@@ -161,6 +166,8 @@ testouterr_cmd sh {refresh -v} $ans $tserr2
 setenv_var __MODULES_LMTAG refresh/2.0&bar&hidden-loaded
 set tserr1 [msg_refresh refresh/1.0]
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
+lappend ans [list out {echo pre refresh/2.0}]
 lappend ans [list alias altags {echo bar hidden-loaded loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alisloaded2 {echo is-loaded ok}]
@@ -179,6 +186,8 @@ testouterr_cmd sh {refresh -v} $ans $tserr2
 # verbose output and hidden-loaded module
 setenv_var __MODULES_LMTAG refresh/2.0&bar&hidden-loaded&auto-loaded
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
+lappend ans [list out {echo pre refresh/2.0}]
 lappend ans [list alias altags {echo auto-loaded bar hidden-loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alisloaded2 {echo is-loaded ok}]
@@ -230,6 +239,7 @@ setenv_var __MODULES_LMTAG refresh/2.0&bar
 # break during evaluation
 setenv_var TESTSUITE_REFRESH break1
 set ans1 [list]
+lappend ans1 [list out {echo pre refresh/2.0}]
 lappend ans1 [list alias altags {echo bar loaded}]
 lappend ans1 [list alias alspec {echo refresh/2.0}]
 lappend ans1 [list alias alfoo {echo foo2}]
@@ -251,6 +261,7 @@ setenv_var __MODULES_LMTAG refresh/2.0&bar
 setenv_var TESTSUITE_REFRESH break2
 
 set ans2 [list]
+lappend ans2 [list out {echo pre refresh/1.0}]
 lappend ans2 [list alias alisloaded2 {echo is-loaded ok}]
 lappend ans2 [list alias alspec {echo refresh/1.0}]
 lappend ans2 [list alias altags {echo loaded}]
@@ -306,6 +317,7 @@ testouterr_cmd sh {refresh} $ans2 $tserr
 # continue command
 setenv_var TESTSUITE_REFRESH continue1
 set ans [list]
+lappend ans [list out {echo pre refresh/2.0}]
 lappend ans [list alias altags {echo bar loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alisloaded2 {echo is-loaded ok}]
@@ -319,6 +331,7 @@ testouterr_cmd sh {refresh} $ans {}
 
 setenv_var TESTSUITE_REFRESH continue2
 set ans [list]
+lappend ans [list out {echo pre refresh/1.0}]
 lappend ans [list alias altags {echo bar loaded}]
 lappend ans [list alias alspec {echo refresh/2.0}]
 lappend ans [list alias alisloaded2 {echo is-loaded ok}]

diff --git a/testsuite/modules.50-cmds/083-info-mode.exp b/testsuite/modules.50-cmds/083-info-mode.exp
@@ -42,16 +42,19 @@ set modulefilere2 "$modpathre/$module2"
 #  The _LMFILES_ and the LOADEDMODULES will be affected
 #
 
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list load]
 
 lappend ansrm [list unset _LMFILES_]
 lappend ansrm [list unset LOADEDMODULES]
+lappend ansrm [list unset __MODULES_LMREFRESH]
 lappend ansrm [list unload]
 
 lappend anssw [list set _LMFILES_ $modulefile2]
 lappend anssw [list set LOADEDMODULES $module2]
+lappend anssw [list set __MODULES_LMREFRESH $module2]
 lappend anssw [list unload]
 lappend anssw [list load]
 
@@ -71,6 +74,7 @@ testouterr_cmd_re "sh" "display $module" [join $ansout "\n"] "$modlin\n$modulefi
 
 # Set up the environment for test that require the module to be loaded
 setenv_loaded_module $module $modulefile
+setenv_var __MODULES_LMREFRESH $module
 
 # test unloading
 # different behavior than C-version: on Tcl-version mode is set to 'unload'
@@ -84,13 +88,12 @@ test_cmd "sh" "unload $module" $ansrm
 testouterr_cmd "sh" "switch $module $module2" $anssw ""
 
 # test refresh
-setenv_var __MODULES_LMREFRESH $module
 set ansout [list refresh [shell_ok sh 1]]
 testouterr_cmd sh {refresh} [join $ansout \n] {}
-unsetenv_var __MODULES_LMREFRESH
 
 # Clean up the just changed environment
 unsetenv_loaded_module
+unsetenv_var __MODULES_LMREFRESH
 
 # test help
 set ansout [list "help" [shell_ok sh 1]]

diff --git a/testsuite/modules.50-cmds/084-info-mode-exp.exp b/testsuite/modules.50-cmds/084-info-mode-exp.exp
@@ -44,17 +44,20 @@ set modulefilere2 "$modpathre/$module2"
 #  The _LMFILES_ and the LOADEDMODULES will be affected
 #
 
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list load]
 
 lappend ansrm [list unset _LMFILES_]
 lappend ansrm [list unset LOADEDMODULES]
+lappend ansrm [list unset __MODULES_LMREFRESH]
 lappend ansrm [list unload]
 lappend ansrm [list remove]
 
 lappend anssw [list set _LMFILES_ $modulefile2]
 lappend anssw [list set LOADEDMODULES $module2]
+lappend anssw [list set __MODULES_LMREFRESH $module2]
 lappend anssw [list unload]
 lappend anssw [list remove]
 lappend anssw [list switch]
@@ -78,6 +81,7 @@ testouterr_cmd_re "sh" "display $module" [join $ansout "\n"] "$modlin\n$modulefi
 
 # Set up the environment for test that require the module to be loaded
 setenv_loaded_module $module $modulefile
+setenv_var __MODULES_LMREFRESH $module
 
 # test unloading
 # different behavior than C-version: on Tcl-version mode is set to 'unload'
@@ -93,13 +97,12 @@ test_cmd "sh" "unload $module" $ansrm
 testouterr_cmd "sh" "switch $module $module2" $anssw ""
 
 # test refresh
-setenv_var __MODULES_LMREFRESH $module
 set ansout [list refresh nonpersist [shell_ok sh 1]]
 testouterr_cmd sh {refresh} [join $ansout \n] {}
-unsetenv_var __MODULES_LMREFRESH
 
 # Clean up the just changed environment
 unsetenv_loaded_module
+unsetenv_var __MODULES_LMREFRESH
 
 # test help
 set ansout [list "help" [shell_ok sh 1]]

diff --git a/testsuite/modules.50-cmds/085-info-flags.exp b/testsuite/modules.50-cmds/085-info-flags.exp
@@ -43,16 +43,19 @@ set modulefilere2 "$modpathre/$module2"
 #  The _LMFILES_ and the LOADEDMODULES will be affected
 #
 
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list 0]
 
 lappend ansrm [list unset _LMFILES_]
 lappend ansrm [list unset LOADEDMODULES]
+lappend ansrm [list unset __MODULES_LMREFRESH]
 lappend ansrm [list 0]
 
 lappend anssw [list set _LMFILES_ $modulefile2]
 lappend anssw [list set LOADEDMODULES $module2]
+lappend anssw [list set __MODULES_LMREFRESH $module2]
 lappend anssw [list 0]
 lappend anssw [list 0]
 
@@ -75,6 +78,7 @@ testouterr_cmd_re "sh" "display $module" [join $ansout "\n"] "$modlin\n$modulefi
 
 # Set up the environment for test that require the module to be loaded
 setenv_loaded_module $module $modulefile
+setenv_var __MODULES_LMREFRESH $module
 
 # test unloading
 test_cmd "sh" "unload $module" $ansrm
@@ -84,6 +88,7 @@ test_cmd "sh" "switch $module $module2" $anssw
 
 # Clean up the just changed environment
 unsetenv_loaded_module
+unsetenv_var __MODULES_LMREFRESH
 
 # test help
 testouterr_cmd_re "sh" "help $module" [join $ansout "\n"] "$modlin\n$lin_help\n\n0\n$modlin"

diff --git a/testsuite/modules.50-cmds/310-puts.exp b/testsuite/modules.50-cmds/310-puts.exp
@@ -31,6 +31,7 @@ set module "puts/1"
 set modulefile "$modpath/$module"
 set modulefilere "$modpathre/$module"
 
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list set testsuite "ok"]
@@ -63,6 +64,7 @@ set modulefile "$modpath/$module"
 set modulefilere "$modpathre/$module"
 
 set ans [list]
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list set testsuite ok]
@@ -215,6 +217,7 @@ set module "puts/8"
 set modulefile "$modpath/$module"
 
 set ans [list]
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile]
 lappend ans [list set LOADEDMODULES $module]
 lappend ans [list set testsuite ok]
@@ -235,6 +238,7 @@ if {[file exists $fname]} {
 
 set ans [list]
 lappend ans [list set __MODULES_LMPREREQ puts/9&$module]
+lappend ans [list set __MODULES_LMREFRESH $module]
 lappend ans [list set _LMFILES_ $modulefile:$modpath/puts/9]
 lappend ans [list set LOADEDMODULES $module:puts/9]
 lappend ans [list set testsuite ok]

diff --git a/testsuite/modules.50-cmds/311-puts-prestdout.exp b/testsuite/modules.50-cmds/311-puts-prestdout.exp
@@ -37,6 +37,7 @@ lappend ans [list out textpreout1]
 if {$install_advversspec eq {y}} {
     lappend ans [list set __MODULES_LMALTNAME $mod&as|puts/default&as|puts/latest]
 }
+lappend ans [list set __MODULES_LMREFRESH $mod]
 lappend ans [list set _LMFILES_ $modfile]
 lappend ans [list set LOADEDMODULES $mod]
 lappend ans [list set testsuite ok]
@@ -69,6 +70,7 @@ lappend ans [list out textpreout1]
 if {$install_advversspec eq {y}} {
     lappend ans [list set __MODULES_LMALTNAME $mod&as|puts/default&as|puts/latest]
 }
+lappend ans [list set __MODULES_LMREFRESH $mod]
 lappend ans [list set _LMFILES_ $modfile]
 lappend ans [list set LOADEDMODULES $mod]
 lappend ans [list set testsuite ok]
@@ -105,6 +107,7 @@ lappend ans [list out textpreout1textpreout2]
 if {$install_advversspec eq {y}} {
     lappend ans [list set __MODULES_LMALTNAME $mod&as|puts/default&as|puts/latest]
 }
+lappend ans [list set __MODULES_LMREFRESH $mod]
 lappend ans [list set _LMFILES_ $modfile]
 lappend ans [list set LOADEDMODULES $mod]
 lappend ans [list set testsuite ok]

diff --git a/testsuite/modules.70-maint/230-verbosity.exp b/testsuite/modules.70-maint/230-verbosity.exp
@@ -99,6 +99,7 @@ foreach verbosity [list silent concise normal verbose verbose2] {
 
     # usage of puts command in modulefile toward stdout and stderr
     set ans [list]
+    lappend ans [list set __MODULES_LMREFRESH puts/1]
     lappend ans [list set _LMFILES_ $modpath/puts/1]
     lappend ans [list set LOADEDMODULES puts/1]
     lappend ans [list set testsuite "ok"]