Preserve genuine path duplicates in source-sh/sh-to-mod output

NEWS.rst

@@ -137,6 +137,21 @@ Modules 5.7.0 (not yet released)
   :instopt:`--with-domainname` and :instopt:`--with-domainname-opts` options.
   When :mconfig:`domainname` is changed with :subcmd:`config` sub-command, it
   sets the :envvar:`MODULES_DOMAINNAME` environment variable. (fix issue #645)
+* Update the :mfcmd:`source-sh` modulefile command and :subcmd:`sh-to-mod`
+  sub-command mechanism to produce a more consistent value for a path-like
+  environment variable change that requires both a prepended and an appended
+  part expressed with a different delimiter character: a :mfcmd:`setenv`
+  command is now generated instead in this situation.
+* Update the :mfcmd:`source-sh` modulefile command and :subcmd:`sh-to-mod`
+  sub-command mechanism to no longer de-duplicate a path entry found several
+  times in the entries it prepends or in the entries it appends, and to add
+  the ``--duplicates`` option on a generated :mfcmd:`prepend-path` or
+  :mfcmd:`append-path` command when one of the path entries it adds is
+  already found in the variable value prior script evaluation, is found
+  several times in the entries it adds or is found in both the entries it
+  prepends and the entries it appends, so this entry is not silently
+  absorbed by these commands' default de-duplication behavior nor relocated
+  by the :mconfig:`path_entry_reorder` configuration option.
 
 
 .. _5.6 release notes:

doc/source/design/source-shell-script-in-modulefile.rst

@@ -183,17 +183,26 @@ Specification
 
     - to separate each output kind and then be able to split them for separate analysis
 
-- De-duplication of path entries is applied for changes on path-like environment variables
+- Path entries found in the newly prepended entries or in the newly appended entries for a variable are never de-duplicated: every occurrence is kept and passed on the corresponding ``prepend-path`` or ``append-path`` command
 
-  - If the same path entry appears several times in the newly prepended entries for a variable, the first occurrence of this entry is kept others are dropped
-  - If the same path entry appears several times in the newly appended entries for a variable, the first occurrence of this entry is kept others are dropped
-  - De-duplication is not applied for path entries:
+- The ``--duplicates`` option is added to a ``prepend-path`` or ``append-path`` command when one of the entries it adds:
 
-    - appearing in both the new prepended entries and newly appended entries
-    - appearing in newly prepended entries or newly appended entries and in entries defined prior script evaluation
+  - is already found in the variable value prior script evaluation
+  - is found several times in the newly prepended entries or in the newly appended entries
+  - is found in both the newly prepended entries and the newly appended entries
+
+  - Such an entry is genuinely meant to end up at this specific position in the resulting value
+  - Without this option, ``prepend-path`` and ``append-path``'s default de-duplication behavior would silently absorb the entry, and the ``path_entry_reorder`` configuration option could relocate it, which would not correctly reproduce the change made by the script
 
 - An environment variable equaling to the path separator character (``:``) prior script evaluation is considered as undefined prior script evaluation to avoid misleading analysis
 
+- Path-like variable change is recorded as a ``setenv`` command instead of ``prepend-path`` and ``append-path`` commands when the newly prepended and newly appended parts would each need a different delimiter character to be expressed
+
+  - Prior and after variable values are compared to find a common part between them: what is found before this common part is expressed as a ``prepend-path`` command and what is found after it as an ``append-path`` command
+  - Each of these two commands may need its own delimiter character, determined from the character found in the after value right before (for the prepended part) or right after (for the appended part) the common part, when this character is not the variable path separator
+  - When the delimiter character determined for the prepended part differs from the one determined for the appended part, describing the change as a ``prepend-path`` and an ``append-path`` command would not correctly reproduce the resulting value, as each command would independently split and de-duplicate the full variable value using its own delimiter
+  - A ``setenv`` command directly setting the after value is generated instead in this situation, to always accurately reflect the change made by the script
+
 - Environment variables made for Modules private use are filtered-out from the environment changes produced
 
   - ``LOADEDMODULES``, ``_LMFILES_`` and any variable prefixed by ``__MODULES_`` are concerned

tcl/mfcmd.tcl

@@ -1676,51 +1676,76 @@ proc sh-to-mod {elt_ignored_list args} {
    foreach name $diff {
       if {$name ni $ignvarlist && ![string equal -length 10 $name\
          __MODULES_]} {
+         set idx [string first $varbef($name) $varaft($name)]
+         set doprepend [expr {$idx > 0}]
+         if {$doprepend} {
+            # check from the end to get the largest chunk to prepend
+            set idx [string last $varbef($name) $varaft($name)]
+            # get delimiter from char found between new and existing value
+            set predelim [string index $varaft($name) $idx-1]
+            set delim $predelim
+         }
+         set appdelim_idx [expr {$idx + [string length $varbef($name)]}]
+         set doappend [expr {$appdelim_idx < [string length $varaft($name)]}]
+         if {$doappend} {
+            set appdelim [string index $varaft($name) $appdelim_idx]
+            set delim $appdelim
+         }
          # new value is totally different (also consider a bare ':' as a
          # totally different value to avoid erroneous matches)
-         if {$varbef($name) eq $pathsep || [set idx [string first\
-            $varbef($name) $varaft($name)]] == -1} {
+         # if content must both be prepended and appended but each side uses a
+         # different delimiter character, consider new value totally different
+         if {$varbef($name) eq $pathsep || $idx == -1 || ($doprepend &&\
+            $doappend && $predelim ne $appdelim)} {
             lappend modcontent [list setenv $name $varaft($name)]
          } else {
+            set alllist [list]
+            # de-dup pre-existing list to correctly determine if new dup added
+            lappendNoDup alllist {*}[split $varbef($name) $delim]
+            set addmodcmd [list]
             # content should be prepended
-            if {$idx > 0} {
-               set modcmd [list prepend-path]
-               # check from the end to get the largest chunk to prepend
-               set idx [string last $varbef($name) $varaft($name)]
-               # get delimiter from char found between new and existing value
-               set delim [string index $varaft($name) $idx-1]
-               if {$delim ne $pathsep} {
-                  lappend modcmd -d $delim
-               }
-               lappend modcmd $name
-               # split value and remove duplicate entries
-               set vallist [list]
-               lappendNoDup vallist {*}[split [string range $varaft($name) 0\
-                  $idx-2] $delim]
+            if {$doprepend} {
+               set prelist [split [string range $varaft($name) 0 $idx-2]\
+                  $delim]
                # an empty element is added
-               if {![llength $vallist]} {
-                  lappend vallist {}
+               if {![llength $prelist]} {
+                  lappend prelist {}
                }
-               lappend modcontent [list {*}$modcmd {*}$vallist]
+               lappend alllist {*}$prelist
+               lappend addmodcmd prepend-path $prelist
             }
             # content should be appended
-            if {($idx + [string length $varbef($name)]) < [string length\
-               $varaft($name)]} {
-               set modcmd [list append-path]
-               set delim [string index $varaft($name) $idx+[string length\
-                  $varbef($name)]]
-               if {$delim ne $pathsep} {
-                  lappend modcmd -d $delim
+            if {$doappend} {
+               set applist [split [string range $varaft($name) [expr\
+                  {$appdelim_idx + 1}] end] $delim]
+               # an empty element is added
+               if {![llength $applist]} {
+                  lappend applist {}
                }
-               lappend modcmd $name
-               set vallist [list]
-               lappendNoDup vallist {*}[split [string range $varaft($name)\
-                  [expr {$idx + [string length $varbef($name)] + 1}] end]\
-                  $delim]
-               if {![llength $vallist]} {
-                  lappend vallist {}
+               lappend alllist {*}$applist
+               lappend addmodcmd append-path $applist
+            }
+
+            # if a directory is found several times in prepended or appended
+            # list, or across both list, or already in pre-existing value:
+            # should not be de-duplicated by prepend-path/append-path's
+            # default behavior, nor relocated by the path_entry_reorder
+            # configuration option. a plain membership test (rather than
+            # merging pre-existing value into alllist) is required here, as
+            # merging would let lappendNoDup silently skip an entry already
+            # in alllist, hiding the very overlap this check looks for
+            set isdup [isDupInList $alllist]
+
+            foreach {cmd addlist} $addmodcmd {
+               set addcmd [list $cmd]
+               if {$isdup} {
+                  lappend addcmd --duplicates
+               }
+               if {$delim ne $pathsep} {
+                  lappend addcmd -d $delim
                }
-               lappend modcontent [list {*}$modcmd {*}$vallist]
+               lappend addcmd $name {*}$addlist
+               lappend modcontent $addcmd
             }
          }
       }

tcl/util.tcl

@@ -322,6 +322,13 @@ proc getDiffBetweenList {list1 list2} {
    return [list $res1 $res2]
 }
 
+# returns if at least one element in list is present several times
+proc isDupInList {arg_list} {
+   ##nagelfar ignore #2 Unknown variable
+   lappendNoDup uniq_list {*}$arg_list
+   return [expr {[llength $arg_list] != [llength $uniq_list]}]
+}
+
 # return intersection of all lists: elements present in every list
 proc getIntersectBetweenList {args} {
    foreach lst $args {