From 9c84f4d26f3e39fb592c2a7be73aa6f67678dfde Mon Sep 17 00:00:00 2001
From: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Date: Sun, 16 Feb 2025 14:35:55 +0100
Subject: [PATCH] Fix interp variable reset between evaluations

Fix interpreter variable reset between multiple modulefile or modulerc
evaluations when variable initial value is an invalid list.

Fixes #570

Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
---
 NEWS.rst                                            |  2 ++
 tcl/interp.tcl.in                                   |  3 ++-
 testsuite/example/siteconfig.tcl-1                  |  7 +++++++
 testsuite/modulefiles.3/interp/.modulerc            |  6 ++++++
 testsuite/modulefiles.3/interp/1                    |  6 ++++++
 testsuite/modules.50-cmds/560-siteconfig-interp.exp | 13 +++++++++++++
 6 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/NEWS.rst b/NEWS.rst
index b47ce9e6b..30866fb5f 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -34,6 +34,8 @@ Modules 5.6.0 (not yet released)
 * Doc: describe the :ref:`developer-certificate-of-origin` process now applied
   on contributions.
 * Doc: add :ref:`add-new-maintainer` guide
+* Fix interpreter variable reset between multiple modulefile or modulerc
+  evaluations when variable initial value is an invalid list. (fix issue #570)
 
 .. _Security policy: https://github.com/envmodules/modules/blob/main/SECURITY.md
 .. _Modules chat room: https://matrix.to/#/#modules:matrix.org
diff --git a/tcl/interp.tcl.in b/tcl/interp.tcl.in
index 317f9c0e6..e9e8f13c8 100644
--- a/tcl/interp.tcl.in
+++ b/tcl/interp.tcl.in
@@ -847,7 +847,8 @@ proc resetInterpState {itrp fresh dumpVarsVN dumpArrayVarsVN untrackVarsVN\
          } elseif {![info exists dumpArrayVars($var)]} {
             if {$dumpVars($var) ne [$itrp eval set ::$var]} {
                reportDebug "restoring on $itrp var $var"
-               if {[llength $dumpVars($var)] > 1} {
+               if {[string is list $dumpVars($var)] && [llength\
+                  $dumpVars($var)] > 1} {
                   # restore value as list
                   $itrp eval set ::$var [list $dumpVars($var)]
                } else {
diff --git a/testsuite/example/siteconfig.tcl-1 b/testsuite/example/siteconfig.tcl-1
index dbcb5e6d8..809d3d5d1 100644
--- a/testsuite/example/siteconfig.tcl-1
+++ b/testsuite/example/siteconfig.tcl-1
@@ -671,5 +671,12 @@ if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_GETMODULEFILEFROMLOADINGMODULE)
     report [string length [getModulefileFromLoadingModule foo]]
 }
 
+if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST)]} {
+    switch -- $env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST) {
+        var    {set modulefile_extra_vars [list myvar {"foo"; "bar"}]}
+        rc_var {set modulerc_extra_vars [list myvar {"foo"; "bar"}]}
+    }
+}
+
 
 }
diff --git a/testsuite/modulefiles.3/interp/.modulerc b/testsuite/modulefiles.3/interp/.modulerc
index 1fba6b3ff..e3c203087 100644
--- a/testsuite/modulefiles.3/interp/.modulerc
+++ b/testsuite/modulefiles.3/interp/.modulerc
@@ -47,3 +47,9 @@ if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_REGULAR)]} {
         mix - override {module-version interp/1 [mycmd] $myvar}
     }
 }
+
+if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST)]} {
+    switch -- $env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST) {
+        rc_var {set myvar value}
+    }
+}
diff --git a/testsuite/modulefiles.3/interp/1 b/testsuite/modulefiles.3/interp/1
index 27294e5e3..ee79ccab4 100644
--- a/testsuite/modulefiles.3/interp/1
+++ b/testsuite/modulefiles.3/interp/1
@@ -58,3 +58,9 @@ if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_REGULAR)]} {
         }
     }
 }
+
+if {[info exists env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST)]} {
+    switch -- $env(TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST) {
+        var {set myvar value}
+    }
+}
diff --git a/testsuite/modules.50-cmds/560-siteconfig-interp.exp b/testsuite/modules.50-cmds/560-siteconfig-interp.exp
index dc22eb2a7..bf9c1ab1f 100644
--- a/testsuite/modules.50-cmds/560-siteconfig-interp.exp
+++ b/testsuite/modules.50-cmds/560-siteconfig-interp.exp
@@ -125,6 +125,19 @@ if {[cmpversion $tclsh_version 8.5] > 0} {
 unsetenv_var TESTSUITE_ENABLE_SITECONFIG_EXTRA_UNK
 
 
+# variable value not a list
+setenv_var TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST rc_var
+set ans [list]
+lappend ans [list set _LMFILES_ $mp/interp/1:$mp/foo/1.0]
+lappend ans [list set LOADEDMODULES interp/1:foo/1.0]
+testouterr_cmd sh {load interp/1 foo/1.0} $ans {}
+
+setenv_var TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST var
+testouterr_cmd sh {load interp/1 foo/1.0} $ans {}
+
+unsetenv_var TESTSUITE_ENABLE_SITECONFIG_EXTRA_VALNOTALIST
+
+
 # extra cmds and vars that supersede existing cmds or variables
 setenv_var TESTSUITE_ENABLE_SITECONFIG_EXTRA_SUPERSEDE rc_cmd
 set tserr "interp/0\ninterp/1(myproc1:myproc2)\ninterp/2"