Skip to content

Commit f4ea413

Browse files
dschodscho
authored
pacman-helper: inline a vercmp Bash function into the patched repo-add (#714)
When `pacman-helper.sh quick_add` is run from the `pacman-packages` action of [`git-for-windows/git-for-windows-automation`](https://github.com/git-for-windows/git-for-windows-automation/blob/release/.github/actions/pacman-packages/action.yml), the SDK that the action sets up is the `build-installers` flavor, which intentionally ships only a small subset of `/usr/bin`. `vercmp.exe` is not in that subset, so every `repo-add` invocation prints two lines of noise of the form: ``` D:/a/_temp/build-extra/repo-add: line 254: vercmp: command not found D:/a/_temp/build-extra/repo-add: line 254: ((: > 0 : arithmetic syntax error: operand expected (error token is "> 0 ") ``` (see e.g. [run 27414389819](https://github.com/git-for-windows/git-for-windows-automation/actions/runs/27414389819/job/81026080873), which produced 64 such pairs before the unrelated `db3` leak bit it; that one is already fixed on `main` as 887f246). The errors themselves are benign because we invoke `repo-add` without `--prevent-downgrade`, so the suppressed "newer version already present" warning has no functional consequence, but I'd rather not pollute release logs with them. Adding `vercmp.exe` (or any of the other plausible interpreters that came up while exploring this: `awk.exe`, `perl`, `expr.exe`) to `build-installers` would defeat the point of that minimal SDK. Instead, this extends the existing sed-based patching of `/usr/bin/repo-add` to also inject a small Bash function called `vercmp`. The patched copy is what `pacman-helper.sh` invokes, and `repo-add` is itself a Bash script, so the injected function is in scope at the call site with no PATH manipulation, no `export -f` gymnastics across process boundaries, and `pacman-helper.sh` itself stays POSIX `sh`. The algorithm is essentially the same shape as the prior-art `version_compare` in `git-extra/git-update-git-for-windows`, with one fix (strict equality returns `0`, which `vercmp`'s contract requires but the prior art got wrong). It splits each version string on runs of non-alphanumeric characters and walks the resulting arrays segment by segment: two numeric segments compare numerically, a numeric vs. alphanumeric segment lets the numeric side win (matching pacman's behavior of treating pre-release suffixes as older), and otherwise it falls back to lexical comparison. When one string runs out of segments first, the longer string wins unless its extra segment starts with a letter, in which case it is treated as a pre-release. All nine test cases from `git-update-git-for-windows --test-version-compare` pass, plus the strict-equality case and the exact strings from the failing run: ``` OK vercmp 2.32.0.windows.1 2.32.1.windows.1 -> -1 OK vercmp 2.32.1.windows.1 2.32.0.windows.1 -> 1 OK vercmp 2.32.1.vfs.0.0 2.32.0.windows.1 -> 1 OK vercmp 2.32.1.vfs.0.0 2.32.0.vfs.0.0 -> 1 OK vercmp 2.32.0.vfs.0.1 2.32.0.vfs.0.2 -> -1 OK vercmp 2.32.0-rc0.windows.1 2.31.1.windows.1 -> 1 OK vercmp 2.32.0-rc2.windows.1 2.32.0.windows.1 -> -1 OK vercmp 2.34.0.rc1.windows.1 2.33.1.windows.1 -> 1 OK vercmp 2.34.0.rc2.windows.1 2.34.0.windows.1 -> -1 OK vercmp 2.55.0.1-1 2.55.0.1-1 -> 0 OK vercmp 2.54.0.1-1 2.55.0.rc0.windows.1-1 -> -1 OK vercmp 2.55.0.rc0.windows.1-1 2.54.0.1-1 -> 1 OK vercmp 2.55.0.windows.1-1 2.55.0.windows.1-2 -> -1 OK vercmp 2.55.0.windows.1-2 2.55.0.windows.1-1 -> 1 OK vercmp 2.54 2.54.1 -> -1 OK vercmp 2.55.0.rc0 2.55.0 -> -1 ``` ### Known limitation Within a mixed-alphanumeric segment such as `rc10` vs. `rc2` the lexical fallback gets the order wrong (would say `rc10` < `rc2`), but Git for Windows has never published an RC past single digits, so a full rpm-style sub-tokenization stays out of scope. ### Scope `repo-remove` next door is patched the same way but does not call `vercmp` (it only takes package names, not versions), so its patching block is intentionally left alone.
2 parents 015df0e + e9e4255 commit f4ea413

1 file changed

Lines changed: 42 additions & 2 deletions

File tree

‎pacman-helper.sh‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,48 @@ call_gpg () {
7979
repo_add () {
8080
if test ! -s "$this_script_dir/repo-add"
8181
then
82-
# Make sure that GPGKEY is used unquoted
83-
sed 's/"\(\${\?GPGKEY}\?\)"/\1/g' </usr/bin/repo-add >"$this_script_dir/repo-add"
82+
# The `build-installers` SDK that is used by the
83+
# `pacman-packages` action of
84+
# `git-for-windows/git-for-windows-automation` does not ship
85+
# `vercmp.exe`. Inject a small Bash function of the same name
86+
# (`repo-add` is a Bash script, so it picks it up at the call
87+
# site without any PATH manipulation), and make sure that
88+
# `GPGKEY` is used unquoted.
89+
{
90+
sed '1q' </usr/bin/repo-add &&
91+
cat <<\VERCMP_EOF &&
92+
vercmp () {
93+
[[ "$1" = "$2" ]] && { echo 0; return; }
94+
local -a sa=( ${1//[^0-9A-Za-z]/ } )
95+
local -a sb=( ${2//[^0-9A-Za-z]/ } )
96+
local n=${#sa[@]}
97+
(( ${#sb[@]} < n )) && n=${#sb[@]}
98+
local i
99+
for (( i = 0; i < n; i++ )); do
100+
[[ "${sa[i]}" = "${sb[i]}" ]] && continue
101+
if [[ "${sa[i]}" =~ ^[0-9]+$ && "${sb[i]}" =~ ^[0-9]+$ ]]; then
102+
(( 10#${sa[i]} > 10#${sb[i]} )) && echo 1 || echo -1
103+
return
104+
fi
105+
[[ "${sa[i]}" =~ ^[0-9]+$ ]] && { echo 1; return; }
106+
[[ "${sb[i]}" =~ ^[0-9]+$ ]] && { echo -1; return; }
107+
[[ "${sa[i]}" > "${sb[i]}" ]] && echo 1 || echo -1
108+
return
109+
done
110+
(( ${#sa[@]} == ${#sb[@]} )) && { echo 0; return; }
111+
local extra sign
112+
if (( ${#sa[@]} > ${#sb[@]} )); then
113+
extra=${sa[n]}
114+
sign=1
115+
else
116+
extra=${sb[n]}
117+
sign=-1
118+
fi
119+
[[ "$extra" =~ ^[A-Za-z] ]] && echo $(( -sign )) || echo $sign
120+
}
121+
VERCMP_EOF
122+
sed '1d; s/"\(\${\?GPGKEY}\?\)"/\1/g' </usr/bin/repo-add
123+
} >"$this_script_dir/repo-add"
84124
fi &&
85125
"$this_script_dir/repo-add" "$@"
86126
}

0 commit comments

Comments
 (0)