Update tags set with prereq on all loaded mods

xdelaruelle · xdelaruelle · commit f9071947e40d · 2022-05-28T18:26:02.000+02:00
Update tags set through prereq command on all already loaded requirements. Closes #459.
diff --git a/NEWS.rst b/NEWS.rst
@@ -24,6 +24,8 @@ Modules 5.1.1 (not yet released)
   issue #456)
 * Report a global known error when current working directory cannot be
   retrieved due to the removal of this directory. (fix issue #457)
+* Update tags set through :mfcmd:`prereq` command on all already loaded
+  requirements. (fix issue #459)
 
 
 Modules 5.1.0 (2022-04-30)
diff --git a/doc/source/design/module-tags.rst b/doc/source/design/module-tags.rst
@@ -344,8 +344,7 @@ Updating tags on already loaded modules
 
 - With ``prereq``-like commands:
 
-    - only the first loaded requirement in specified list gets its tag list
-      updated
+    - all loaded requirement in specified list get their tag list updated
     - loading requirement does not get its tag list updated (no real use case
       foreseen for cyclic dependencies)
 
diff --git a/tcl/mfinterp.tcl.in b/tcl/mfinterp.tcl.in
@@ -1722,19 +1722,20 @@ proc prereq {args} {
       loadRequirementModuleList $tag_list {*}$args
    }
 
+   set loadedmod_list {}
    foreach mod $args {
-      # get first loaded or loading mod in args list
+      # get all loaded or loading mod in args list
       if {[set loadedmod [getLoadedMatchingName $mod returnfirst]] ne {} ||\
          [set loadedmod [getLoadedMatchingName $mod returnfirst 1]] ne {}} {
-         break
+         lappend loadedmod_list $loadedmod
       }
    }
 
-   if {$loadedmod eq {}} {
+   if {[llength $loadedmod_list] == 0} {
       reportMissingPrereqError $curmodnamevr {*}$args
    } elseif {![getConf auto_handling]} {
-      # apply missing tag to first loaded module found
-      cmdModuleTag 0 0 $tag_list $loadedmod
+      # apply missing tag to all loaded module found
+      cmdModuleTag 0 0 $tag_list {*}$loadedmod_list
    }
 
    return {}
diff --git a/tcl/modeval.tcl b/tcl/modeval.tcl
@@ -618,15 +618,16 @@ proc loadRequirementModuleList {tag_list args} {
    set prereqloaded 0
 
    # calling procedure must have already parsed module specification in args
+   set loadedmod_list {}
    foreach mod $args {
-      # get first loaded or loading mod in args list
+      # get all loaded or loading mod in args list
       if {[set loadedmod [getLoadedMatchingName $mod returnfirst]] ne {} ||\
          [set loadedmod [getLoadedMatchingName $mod returnfirst 1]] ne {}} {
-         break
+         lappend loadedmod_list $loadedmod
       }
    }
 
-   if {$loadedmod eq {}} {
+   if {[llength $loadedmod_list] == 0} {
       set imax [llength $args]
       # if prereq list specified, try to load first then
       # try next if load of first module not successful
@@ -668,8 +669,8 @@ proc loadRequirementModuleList {tag_list args} {
       releaseHeldReport {*}$holdidlist
    } else {
       set prereqloaded 1
-      # apply missing tag to first loaded module found
-      cmdModuleTag 0 0 $tag_list $loadedmod
+      # apply missing tag to all loaded module found
+      cmdModuleTag 0 0 $tag_list {*}$loadedmod_list
    }
 
    return [list $ret $prereqloaded]
diff --git a/testsuite/modulefiles.3/tag/8.0 b/testsuite/modulefiles.3/tag/8.0
@@ -143,5 +143,8 @@ if {[info exists env(TESTSUITE_TAG_OPT)]} {
             variant --boolean bar
             variant --boolean foo
         }
+        prtag13 {
+            prereq --tag=foo foo/1.0 bar/1.0
+        }
     }
 }
diff --git a/testsuite/modules.50-cmds/465-tag-opt.exp b/testsuite/modules.50-cmds/465-tag-opt.exp
@@ -703,7 +703,7 @@ lappend ans [list set LOADEDMODULES foo/1.0:$mod]
 testouterr_cmd sh "load --auto $mod" $ans {}
 testouterr_cmd sh "load --no-auto $mod" $ans {}
 
-# multiple prereqs already loaded (first one updated only)
+# multiple prereqs already loaded
 setenv_var TESTSUITE_TAG_OPT prtag10
 setenv_loaded_module [list bar/1.0 foo/1.0] [list $mp/bar/1.0 $mp/foo/1.0]
 setenv_path_var __MODULES_LMEXTRATAG foo/1.0&foo
@@ -712,8 +712,8 @@ set ans [list]
 lappend ans [list set __MODULES_LMPREREQ $mod&foo/1.0|bar/1.0]
 lappend ans [list set _LMFILES_ $mp/bar/1.0:$mp/foo/1.0:$modfile]
 lappend ans [list set LOADEDMODULES bar/1.0:foo/1.0:$mod]
-lappend ans [list set __MODULES_LMEXTRATAG foo/1.0&foo&hidden-loaded]
-lappend ans [list set __MODULES_LMTAG foo/1.0&foo&hidden-loaded]
+lappend ans [list set __MODULES_LMEXTRATAG foo/1.0&foo&hidden-loaded:bar/1.0&hidden-loaded&foo]
+lappend ans [list set __MODULES_LMTAG foo/1.0&foo&hidden-loaded:bar/1.0&hidden-loaded&foo]
 testouterr_cmd sh "load --auto $mod" $ans {}
 
 # auto-loaded tag set manually on already loaded (prereq)
@@ -741,9 +741,45 @@ lappend ans [list set __MODULES_LMTAG foo/1.0&keep-loaded&foo&auto-loaded]
 set tserr [msg_top_load $mod {} foo/1.0 {}]
 testouterr_cmd sh "load --auto $mod" $ans $tserr
 
+# tag set manually on second optional requirement (prereq)
+setenv_var TESTSUITE_TAG_OPT prtag13
+setenv_loaded_module [list foo/1.0 $mod] [list $mp/foo/1.0 $modfile]
+setenv_path_var __MODULES_LMEXTRATAG foo/1.0&foo
+setenv_path_var __MODULES_LMTAG foo/1.0&foo&auto-loaded
+setenv_path_var __MODULES_LMPREREQ $mod&foo/1.0|bar/1.0
+set ans [list]
+lappend ans [list set __MODULES_LMPREREQ $mod&foo/1.0|bar/1.0]
+lappend ans [list set _LMFILES_ $mp/foo/1.0:$mp/bar/1.0:$modfile]
+lappend ans [list set LOADEDMODULES foo/1.0:bar/1.0:$mod]
+lappend ans [list set __MODULES_LMEXTRATAG foo/1.0&foo:bar/1.0&foo]
+lappend ans [list set __MODULES_LMTAG foo/1.0&foo&auto-loaded:bar/1.0&foo]
+#FIXME: tag applied by DepRe reloading module does not appear on info messages
+set tserr [msg_top_load bar/1.0 {} {} $mod]
+testouterr_cmd sh "load --auto bar/1.0" $ans $tserr
+
+# both requirement loaded, but not tagged
+unsetenv_path_var __MODULES_LMEXTRATAG
+unsetenv_path_var __MODULES_LMTAG
+unsetenv_path_var __MODULES_LMPREREQ
+setenv_loaded_module [list foo/1.0 bar/1.0] [list $mp/foo/1.0 $mp/bar/1.0]
+set ans [list]
+lappend ans [list set __MODULES_LMPREREQ $mod&foo/1.0|bar/1.0]
+lappend ans [list set _LMFILES_ $mp/foo/1.0:$mp/bar/1.0:$modfile]
+lappend ans [list set LOADEDMODULES foo/1.0:bar/1.0:$mod]
+lappend ans [list set __MODULES_LMEXTRATAG foo/1.0&foo:bar/1.0&foo]
+lappend ans [list set __MODULES_LMTAG foo/1.0&foo:bar/1.0&foo]
+set tserr {}
+#FIXME: info messages do not mention that requirements have been updated with tag
+testouterr_cmd sh "load --auto $mod" $ans $tserr
+testouterr_cmd sh "load --no-auto $mod" $ans $tserr
+set tserr [msg_tag {foo/1.0 <foo>}]\n[msg_tag {bar/1.0 <foo>}]\n[msg_load $mod]
+testouterr_cmd sh "load -v --auto $mod" $ans $tserr
+testouterr_cmd sh "load -v --no-auto $mod" $ans $tserr
+
 
 unsetenv_path_var __MODULES_LMEXTRATAG
 unsetenv_path_var __MODULES_LMTAG
+unsetenv_path_var __MODULES_LMPREREQ
 unsetenv_loaded_module
 
 

Original file line number	Diff line number	Diff line change
`@@ -1722,19 +1722,20 @@ proc prereq {args} {`
`1722`	`1722`	`loadRequirementModuleList $tag_list {*}$args`
`1723`	`1723`	`}`
`1724`	`1724`
	`1725`	`+ set loadedmod_list {}`
`1725`	`1726`	`foreach mod $args {`
`1726`		`- # get first loaded or loading mod in args list`
	`1727`	`+ # get all loaded or loading mod in args list`
`1727`	`1728`	`if {[set loadedmod [getLoadedMatchingName $mod returnfirst]] ne {} \|\|\`
`1728`	`1729`	`[set loadedmod [getLoadedMatchingName $mod returnfirst 1]] ne {}} {`
`1729`		`- break`
	`1730`	`+ lappend loadedmod_list $loadedmod`
`1730`	`1731`	`}`
`1731`	`1732`	`}`
`1732`	`1733`
`1733`		`- if {$loadedmod eq {}} {`
	`1734`	`+ if {[llength $loadedmod_list] == 0} {`
`1734`	`1735`	`reportMissingPrereqError $curmodnamevr {*}$args`
`1735`	`1736`	`} elseif {![getConf auto_handling]} {`
`1736`		`- # apply missing tag to first loaded module found`
`1737`		`- cmdModuleTag 0 0 $tag_list $loadedmod`
	`1737`	`+ # apply missing tag to all loaded module found`
	`1738`	`+ cmdModuleTag 0 0 $tag_list {*}$loadedmod_list`
`1738`	`1739`	`}`
`1739`	`1740`
`1740`	`1741`	`return {}`
Original file line number	Diff line number	Diff line change
`@@ -143,5 +143,8 @@ if {[info exists env(TESTSUITE_TAG_OPT)]} {`
`143`	`143`	`variant --boolean bar`
`144`	`144`	`variant --boolean foo`
`145`	`145`	`}`
	`146`	`+ prtag13 {`
	`147`	`+ prereq --tag=foo foo/1.0 bar/1.0`
	`148`	`+ }`
`146`	`149`	`}`
`147`	`150`	`}`