Skip to content

Commit 473c3cb

Browse files
xdelaruellexdelaruelle
committed
Further simplify sh-to-mod prepend/append path command generation
Merge the two prepend/append branches of the environment variable diff loop in the sh-to-mod procedure into a single flow: both branches now share one directory list, seeded with the de-duplicated prior value then extended with the raw (non-de-duplicated) entries to prepend and to append, and one pass decides whether --duplicates is needed on the resulting commands. This relies on a new helper procedure checking whether a list holds a repeated element. Seeding with the de-duplicated prior value, rather than merging it in afterwards, avoids a repeated entry already found in the prior value on its own from masking a genuine new duplicate. No behavior change. Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
1 parent 0e3470e commit 473c3cb

2 files changed

Lines changed: 66 additions & 102 deletions

File tree

‎tcl/mfcmd.tcl‎

Lines changed: 59 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,54 +1650,6 @@ proc execShAndGetEnv {elt_ignored_list shell script args} {
16501650
}
16511651
}
16521652
1653-
# split the range of a variable value to prepend or append into a
1654-
# directory list for sh-to-mod: a directory found several times in this
1655-
# list is not de-duplicated, as it is genuinely meant to be added at each
1656-
# of these positions in the resulting value. Whether de-duplication would
1657-
# have dropped an entry is also reported, to guide the --duplicates
1658-
# option decision made in mkPathAddModCmd
1659-
proc splitPathAddList {rangeval delim} {
1660-
set dirlist [split $rangeval $delim]
1661-
set diruniq [list]
1662-
lappendNoDup diruniq {*}$dirlist
1663-
set dupbylen [expr {[llength $dirlist] != [llength $diruniq]}]
1664-
# an empty element is added
1665-
if {![llength $dirlist]} {
1666-
lappend dirlist {}
1667-
}
1668-
return [list $dirlist $dupbylen]
1669-
}
1670-
1671-
# build the prepend-path or append-path modulefile command adding dirlist
1672-
# to variable name for sh-to-mod. --duplicates is set on this command
1673-
# when one of its entries is genuinely meant to end up at this specific
1674-
# position in the resulting value, so it is not silently absorbed by
1675-
# prepend-path/append-path's default dedup behavior, nor relocated by the
1676-
# path_entry_reorder configuration option: this is the case when this
1677-
# entry is already found in befval, was found several times in dirlist
1678-
# prior de-duplication (dupbylen), or is also found in the list of
1679-
# entries added on the other side of the value (dupacross)
1680-
proc mkPathAddModCmd {cmdname dirlist dupbylen dupacross delim pathsep\
1681-
befval name} {
1682-
set dupopt [list]
1683-
if {$dupacross || $dupbylen} {
1684-
set dupopt [list --duplicates]
1685-
} else {
1686-
foreach dir $dirlist {
1687-
if {$dir in [split $befval $delim]} {
1688-
set dupopt [list --duplicates]
1689-
break
1690-
}
1691-
}
1692-
}
1693-
set modcmd [list $cmdname {*}$dupopt]
1694-
if {$delim ne $pathsep} {
1695-
lappend modcmd -d $delim
1696-
}
1697-
lappend modcmd $name
1698-
return [list {*}$modcmd {*}$dirlist]
1699-
}
1700-
17011653
# execute script with args through shell and convert environment changes into
17021654
# corresponding modulefile commands
17031655
proc sh-to-mod {elt_ignored_list args} {
@@ -1724,71 +1676,76 @@ proc sh-to-mod {elt_ignored_list args} {
17241676
foreach name $diff {
17251677
if {$name ni $ignvarlist && ![string equal -length 10 $name\
17261678
__MODULES_]} {
1679+
set idx [string first $varbef($name) $varaft($name)]
1680+
set doprepend [expr {$idx > 0}]
1681+
if {$doprepend} {
1682+
# check from the end to get the largest chunk to prepend
1683+
set idx [string last $varbef($name) $varaft($name)]
1684+
# get delimiter from char found between new and existing value
1685+
set predelim [string index $varaft($name) $idx-1]
1686+
set delim $predelim
1687+
}
1688+
set appdelim_idx [expr {$idx + [string length $varbef($name)]}]
1689+
set doappend [expr {$appdelim_idx < [string length $varaft($name)]}]
1690+
if {$doappend} {
1691+
set appdelim [string index $varaft($name) $appdelim_idx]
1692+
set delim $appdelim
1693+
}
17271694
# new value is totally different (also consider a bare ':' as a
17281695
# totally different value to avoid erroneous matches)
1729-
if {$varbef($name) eq $pathsep || [set idx [string first\
1730-
$varbef($name) $varaft($name)]] == -1} {
1696+
# if content must both be prepended and appended but each side uses a
1697+
# different delimiter character, consider new value totally different
1698+
if {$varbef($name) eq $pathsep || $idx == -1 || ($doprepend &&\
1699+
$doappend && $predelim ne $appdelim)} {
17311700
lappend modcontent [list setenv $name $varaft($name)]
17321701
} else {
1733-
set doprepend [expr {$idx > 0}]
1702+
set alllist [list]
1703+
# de-dup pre-existing list to correctly determine if new dup added
1704+
lappendNoDup alllist {*}[split $varbef($name) $delim]
1705+
set addmodcmd [list]
1706+
# content should be prepended
17341707
if {$doprepend} {
1735-
# check from the end to get the largest chunk to prepend
1736-
set idx [string last $varbef($name) $varaft($name)]
1737-
# get delimiter from char found between new and existing value
1738-
set predelim [string index $varaft($name) $idx-1]
1708+
set prelist [split [string range $varaft($name) 0 $idx-2]\
1709+
$delim]
1710+
# an empty element is added
1711+
if {![llength $prelist]} {
1712+
lappend prelist {}
1713+
}
1714+
lappend alllist {*}$prelist
1715+
lappend addmodcmd prepend-path $prelist
17391716
}
1740-
set doappend [expr {($idx + [string length $varbef($name)]) <\
1741-
[string length $varaft($name)]}]
1717+
# content should be appended
17421718
if {$doappend} {
1743-
set appdelim [string index $varaft($name) $idx+[string\
1744-
length $varbef($name)]]
1745-
}
1746-
1747-
# if content must both be prepended and appended but each side
1748-
# uses a different delimiter character, the change cannot be
1749-
# expressed as a coherent pair of path-manipulation commands,
1750-
# since each delimiter implies a different, incompatible split
1751-
# of the value: fall back to a plain setenv to correctly
1752-
# capture the new value
1753-
if {$doprepend && $doappend && $predelim ne $appdelim} {
1754-
lappend modcontent [list setenv $name $varaft($name)]
1755-
} else {
1756-
# content should be prepended
1757-
if {$doprepend} {
1758-
lassign [splitPathAddList [string range $varaft($name)\
1759-
0 $idx-2] $predelim] prelist predupbylen
1760-
}
1761-
# content should be appended
1762-
if {$doappend} {
1763-
lassign [splitPathAddList [string range $varaft($name)\
1764-
[expr {$idx + [string length $varbef($name)] + 1}]\
1765-
end] $appdelim] applist appdupbylen
1719+
set applist [split [string range $varaft($name) [expr\
1720+
{$appdelim_idx + 1}] end] $delim]
1721+
# an empty element is added
1722+
if {![llength $applist]} {
1723+
lappend applist {}
17661724
}
1725+
lappend alllist {*}$applist
1726+
lappend addmodcmd append-path $applist
1727+
}
17671728
1768-
# a directory found in both the prepended and appended
1769-
# entries is also genuinely meant to end up at each of
1770-
# these positions in the resulting value: this is passed
1771-
# on to mkPathAddModCmd along with each side's own
1772-
# dupbylen, to decide whether --duplicates should be set
1773-
set dupacross 0
1774-
if {$doprepend && $doappend} {
1775-
foreach dir $prelist {
1776-
if {$dir in $applist} {
1777-
set dupacross 1
1778-
break
1779-
}
1780-
}
1781-
}
1782-
if {$doprepend} {
1783-
lappend modcontent [mkPathAddModCmd prepend-path\
1784-
$prelist $predupbylen $dupacross $predelim $pathsep\
1785-
$varbef($name) $name]
1729+
# if a directory is found several times in prepended or appended
1730+
# list, or across both list, or already in pre-existing value:
1731+
# should not be de-duplicated by prepend-path/append-path's
1732+
# default behavior, nor relocated by the path_entry_reorder
1733+
# configuration option. a plain membership test (rather than
1734+
# merging pre-existing value into alllist) is required here, as
1735+
# merging would let lappendNoDup silently skip an entry already
1736+
# in alllist, hiding the very overlap this check looks for
1737+
set isdup [isDupInList $alllist]
1738+
1739+
foreach {cmd addlist} $addmodcmd {
1740+
set addcmd [list $cmd]
1741+
if {$isdup} {
1742+
lappend addcmd --duplicates
17861743
}
1787-
if {$doappend} {
1788-
lappend modcontent [mkPathAddModCmd append-path\
1789-
$applist $appdupbylen $dupacross $appdelim $pathsep\
1790-
$varbef($name) $name]
1744+
if {$delim ne $pathsep} {
1745+
lappend addcmd -d $delim
17911746
}
1747+
lappend addcmd $name {*}$addlist
1748+
lappend modcontent $addcmd
17921749
}
17931750
}
17941751
}

‎tcl/util.tcl‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,13 @@ proc getDiffBetweenList {list1 list2} {
322322
return [list $res1 $res2]
323323
}
324324

325+
# returns if at least one element in list is present several times
326+
proc isDupInList {arg_list} {
327+
##nagelfar ignore #2 Unknown variable
328+
lappendNoDup uniq_list {*}$arg_list
329+
return [expr {[llength $arg_list] != [llength $uniq_list]}]
330+
}
331+
325332
# return intersection of all lists: elements present in every list
326333
proc getIntersectBetweenList {args} {
327334
foreach lst $args {

0 commit comments

Comments
 (0)