Correctly parse module spec on depends-on & co

NEWS.rst

@@ -76,6 +76,10 @@ Modules 5.7.0 (not yet released)
 * Fix superseding definition of ``source`` Tcl command when
   :mconfig:`source_cache` configuration option is enabled to support
   ``-encoding`` option. (fix issue #627)
+* Correctly parse the module specifications passed as argument on
+  :mfcmd:`depends-on`, mfcmd:`always-load` and :mfcmd:`prereq-all` modulefile
+  commands. It especially fixes module specification containing the definition
+  of variants. (fixes issue #626)
 
 
 .. _5.6 release notes:

tcl/mfcmd.tcl

@@ -1152,6 +1152,10 @@ proc parsePrereqCommandArgs {cmd args} {
    } elseif {[set mispopt [lsearch -inline -glob $prereq_list --*]] ne {}} {
       knerror "Misplaced option '$mispopt'"
    }
+
+   # parse module version specification
+   set prereq_list [parseModuleSpecification 0 0 0 0 {*}$prereq_list]
+
    return [list $tag_list $modulepath_list $optional $opt_list $prereq_list]
 }
 
@@ -1162,9 +1166,6 @@ proc prereqAnyModfileCmd {tryload auto args} {
    set currentModule [currentState modulename]
    set curmodnamevr [currentState modulenamevr]
 
-   # parse module version specification
-   set args [parseModuleSpecification 0 0 0 0 {*}$args]
-
    # register prereq list (sets of optional prereq are registered as list)
    # unless record inhibited for current iterp context
    if {[currentState inhibit_req_record] != [currentState evalid]} {

tcl/modscan.tcl

@@ -138,27 +138,27 @@ proc provide-sc {args} {
 
 proc prereq-sc {args} {
    lassign [parsePrereqCommandArgs prereq {*}$args] tag_list modulepath_list\
-      optional opt_list args
+      optional opt_list modspec_list
 
-   foreach modspec [parseModuleSpecification 0 0 0 0 {*}$args] {
+   foreach modspec $modspec_list {
       recordScanModuleElt $modspec prereq prereq-any depends-on-any require
    }
 }
 
 proc prereq-all-sc {args} {
    lassign [parsePrereqCommandArgs prereq-all {*}$args] tag_list\
-      modulepath_list optional opt_list args
+      modulepath_list optional opt_list modspec_list
 
-   foreach modspec [parseModuleSpecification 0 0 0 0 {*}$args] {
+   foreach modspec $modspec_list {
       recordScanModuleElt $modspec prereq-all depends-on require
    }
 }
 
 proc always-load-sc {args} {
    lassign [parsePrereqCommandArgs always-load {*}$args] tag_list\
-      modulepath_list optional opt_list args
+      modulepath_list optional opt_list modspec_list
 
-   foreach modspec [parseModuleSpecification 0 0 0 0 {*}$args] {
+   foreach modspec $modspec_list {
       recordScanModuleElt $modspec always-load require
    }
 }

testsuite/modulefiles.4/bar/1

@@ -71,3 +71,10 @@ if {[info exists env(TESTSUITE_PROVIDE)]} {
         }
     }
 }
+if {[info exists env(TESTSUITE_LCOMPAT)]} {
+    switch -- $env(TESTSUITE_LCOMPAT) {
+        depon_spec1 - allo_spec1 - prall_spec1 {
+            variant foo val1 val2 val3
+        }
+    }
+}

testsuite/modulefiles.4/lcompat/1.12

@@ -10,5 +10,14 @@ if {[info exists env(TESTSUITE_LCOMPAT)]} {
         haveDynamicMPATH2 {
             haveDynamicMPATH foo bar
         }
+        depon_spec1 {
+            depends-on bar/1 foo=val1
+        }
+        allo_spec1 {
+            always-load bar/1 foo=val1
+        }
+        prall_spec1 {
+            prereq-all bar/1 foo=val1
+        }
     }
 }

testsuite/modules.50-cmds/515-depends-on.exp

@@ -227,6 +227,23 @@ set tserr "$modlin $mpre $modlin
 testouterr_cmd_re sh "whatis $mod" OK $tserr
 
 
+setenv_var TESTSUITE_LCOMPAT depon_spec1
+setenv_var MODULES_ADVANCED_VERSION_SPEC 1
+set mp $modpath.4
+setenv_path_var MODULEPATH $mp
+set mod lcompat/1.12
+
+set ans [list]
+lappend ans [list set __MODULES_LMVARIANT bar/1&foo|val1|0|0]
+lappend ans [list set __MODULES_LMPREREQ $mod&bar/1\ foo=val1]
+lappend ans [list set _LMFILES_ $mp/bar/1:$mp/$mod]
+lappend ans [list set LOADEDMODULES bar/1:$mod]
+lappend ans [list set __MODULES_LMTAG bar/1&auto-loaded]
+set tserr [msg_top_load $mod {} bar/1{foo=val1} {}]
+testouterr_cmd sh "load --auto $mod" $ans $tserr
+
+
+
 #
 #  Cleanup
 #

testsuite/modules.50-cmds/516-prereq-all.exp

@@ -227,6 +227,22 @@ set tserr "$modlin $mpre $modlin
 testouterr_cmd_re sh "whatis $mod" OK $tserr
 
 
+setenv_var TESTSUITE_LCOMPAT prall_spec1
+setenv_var MODULES_ADVANCED_VERSION_SPEC 1
+set mp $modpath.4
+setenv_path_var MODULEPATH $mp
+set mod lcompat/1.12
+
+set ans [list]
+lappend ans [list set __MODULES_LMVARIANT bar/1&foo|val1|0|0]
+lappend ans [list set __MODULES_LMPREREQ $mod&bar/1\ foo=val1]
+lappend ans [list set _LMFILES_ $mp/bar/1:$mp/$mod]
+lappend ans [list set LOADEDMODULES bar/1:$mod]
+lappend ans [list set __MODULES_LMTAG bar/1&auto-loaded]
+set tserr [msg_top_load $mod {} bar/1{foo=val1} {}]
+testouterr_cmd sh "load --auto $mod" $ans $tserr
+
+
 #
 #  Cleanup
 #

testsuite/modules.50-cmds/517-always-load.exp

@@ -281,6 +281,22 @@ set tserr "$modlin $mpre $modlin
 testouterr_cmd_re sh "whatis $mod" OK $tserr
 
 
+setenv_var TESTSUITE_LCOMPAT allo_spec1
+setenv_var MODULES_ADVANCED_VERSION_SPEC 1
+set mp $modpath.4
+setenv_path_var MODULEPATH $mp
+set mod lcompat/1.12
+
+set ans [list]
+lappend ans [list set __MODULES_LMVARIANT bar/1&foo|val1|0|0]
+lappend ans [list set __MODULES_LMPREREQ $mod&bar/1\ foo=val1]
+lappend ans [list set _LMFILES_ $mp/bar/1:$mp/$mod]
+lappend ans [list set LOADEDMODULES bar/1:$mod]
+lappend ans [list set __MODULES_LMTAG bar/1&keep-loaded&auto-loaded]
+set tserr [msg_top_load $mod {} bar/1{foo=val1} {}]
+testouterr_cmd sh "load --auto $mod" $ans $tserr
+
+
 #
 #  Cleanup
 #